Thread local variables through table-valued function resolution - #4566
Draft
arnaud-lacurie wants to merge 2 commits into
Draft
Thread local variables through table-valued function resolution#4566arnaud-lacurie wants to merge 2 commits into
arnaud-lacurie wants to merge 2 commits into
Conversation
Two changes that only make sense together: without both, a temp function referencing GET_VARIABLE(x) would either never see the calling transaction's variables, or would see them but reuse a stale compiled function when they change. - Thread local variables from BaseVisitor down through SqlFunctionCatalog/UserDefinedFunctionCatalog to where a user-defined function actually gets (lazily) compiled. - RecordLayerInvokedRoutine's function-provider memoization key grows from just isCaseSensitive to (isCaseSensitive, snapshot of local variables), so a temp function body gets recompiled when the variables it references change, rather than reusing whatever it first compiled to. Uses a LinkedHashMap snapshot rather than an ImmutableMap because local-variable values are legitimately nullable. - AstNormalizer defers a missing GET_VARIABLE reference inside a CREATE TEMPORARY FUNCTION body (the function may be called, with the variable set, long after it's created) but still hard-fails at actual invocation time if the variable is still unset. - Permanent (schema-template) function bodies are unaffected: they compile eagerly at CREATE time against an empty variable scope, so a GET_VARIABLE reference in one always raises UNDEFINED_PARAMETER.
- DdlVisitor: the comment claimed local variables are merged ("add not
replace") into CREATE-time state; the code actually swaps the map
wholesale and restores it afterward. Corrected.
- AstNormalizer: the deferred-placeholder comment claimed resolution
happens via withExecutionContext; it actually happens via a later
normalizeFunctionBody pass. Corrected.
- Trimmed comments that only restated the line below them or duplicated
an assertion message already in the code, and a "review concern"
process reference that doesn't belong in a test comment.
arnaud-lacurie
force-pushed
the
apple/arnaud-lacurie/local-vars/4-tvf
branch
from
September 5, 2026 17:50
692bb7a to
a948e67
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two changes that only make sense together: without both, a temp
function referencing GET_VARIABLE(x) would either never see the
calling transaction's variables, or would see them but reuse a stale
compiled function when they change.
SqlFunctionCatalog/UserDefinedFunctionCatalog to where a
user-defined function actually gets (lazily) compiled.
from just isCaseSensitive to (isCaseSensitive, snapshot of local
variables), so a temp function body gets recompiled when the
variables it references change, rather than reusing whatever it
first compiled to. Uses a LinkedHashMap snapshot rather than an
ImmutableMap because local-variable values are legitimately
nullable.
CREATE TEMPORARY FUNCTION body (the function may be called, with
the variable set, long after it's created) but still hard-fails at
actual invocation time if the variable is still unset.
compile eagerly at CREATE time against an empty variable scope, so
a GET_VARIABLE reference in one always raises UNDEFINED_PARAMETER.
Stack created with GitHub Stacks CLI • Give Feedback 💬