Skip to content

Use CEL PartialVars during AST pruning - #333

Merged
rodaine merged 3 commits into
bufbuild:mainfrom
TristonianJones:partial-vars-fix
Aug 7, 2026
Merged

rodaine merged 3 commits into
bufbuild:mainfrom
TristonianJones:partial-vars-fix

Conversation

@TristonianJones

Copy link
Copy Markdown
Contributor

A change in cel-go v0.30.0 revealed a latent bug in how residual expressions were computed with protovalidate-go, as error values terminate function call argument evaluation. The fix marks this as unknown and calls CEL with a PartialVars argument to clearly denote the desired, non-terminating, evaluation behavior.

Metric Before Patch After Patch Change / Improvement
Allocations 241 allocs/op 5 allocs/op ~98% reduction (48x fewer allocs)
Memory Allocated 44,059 B/op (~44 KB) 96 B/op ~99.8% reduction (>450x less memory)
Execution Time 16,856 ns/op (~16.8 µs) 404.1 ns/op (~0.4 µs) ~42x faster
Throughput 69,870 ops 2,931,400 ops ~42x throughput

A change in cel-go v0.30.0 revealed a latent bug in how residual expressions
were computed with protovalidate-go, as `error` values terminate function
call argument evaluation. The fix marks `this` as unknown and calls CEL
with a `PartialVars` argument to clearly denote the desired, non-terminating,
evaluation behavior.
@CLAassistant

CLAassistant commented Aug 5, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@TristonianJones

Copy link
Copy Markdown
Contributor Author

@miparnisari FYI, as this addresses cel-expr/cel-go#1403

@miparnisari

miparnisari commented Aug 6, 2026 •

Copy link
Copy Markdown

Thank you @TristonianJones! Is the after patch column a return to pre-0.30.0 behavior, or is that plus an actual improvement?

@rodaine

rodaine commented Aug 6, 2026

Copy link
Copy Markdown
Member

Oh sweet! Thanks for the patch, @TristonianJones. Looks like there are some formatting issues, but otherwise looks good.

@TristonianJones

Copy link
Copy Markdown
Contributor Author

Hi @rodaine, I believe I updated the formatting issue. Would you mind reapproving the workflow?

The issue in question is a regression from cel-go v0.29.2. As @miparnisari the 'before' patch here is how things regressed, and the 'after' is restoring it to its previous performance.

@rodaine
rodaine merged commit 0e5a75c into bufbuild:main Aug 7, 2026
7 checks passed
@rodaine

rodaine commented Aug 7, 2026

Copy link
Copy Markdown
Member

Thanks again!

@tstirrat15

Copy link
Copy Markdown

Any sense of when this will go out? We're keen to see it.

@pkwarren

Copy link
Copy Markdown
Member

Any sense of when this will go out? We're keen to see it.

Version 1.3.0 is now available which includes this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants