optbuilder: reset annotations when building CREATE FUNCTION#105581
optbuilder: reset annotations when building CREATE FUNCTION#105581craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
DrewKimball
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @rafiss)
pkg/sql/opt/optbuilder/create_function.go line 61 at r1 (raw file):
b.qualifyDataSourceNamesInAST = false b.evalCtx.Annotations = oldEvalCtxAnn b.semaCtx.Annotations = oldSemaCtxAnn
Is this not handled by the defer here?
rafiss
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/create_function.go line 61 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
Is this not handled by the defer here?
i was thinking about it more, and i thought we should set it at the end of the loop in case, since it's possible something could use b after the loop ends, but before the function returns.
DrewKimball
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
pkg/sql/opt/optbuilder/create_function.go line 61 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
i was thinking about it more, and i thought we should set it at the end of the loop in case, since it's possible something could use
bafter the loop ends, but before the function returns.
I see, SGTM.
In 22dabb0 we started overriding the annotations for each statement in the UDF body. We should reset them to the original values, so we don't accidentally leave the old reference. Release note: None
3bfae5d to
d66290a
Compare
|
tftr! bors r+ |
|
Build succeeded: |
In 22dabb0 we started overriding the annotations for each statement in the UDF body. We should reset them to the original values, so we don't accidentally leave the old reference.
Epic: None
Release note: None