Add transaction-scoped local variables (SET LOCAL / @var) - #4198
Closed
arnaud-lacurie wants to merge 38 commits into
Closed
Add transaction-scoped local variables (SET LOCAL / @var)#4198arnaud-lacurie wants to merge 38 commits into
arnaud-lacurie wants to merge 38 commits into
Conversation
Adds `SET LOCAL variable = value` and `@variable` reference syntax, scoped to the current FDB transaction. Variables are stored in the FDBRecordContext session layer and injected as named prepared-statement parameters at normalization time, so the plan cache sees a single plan structure per query and reuses it across different variable values.
- Replace incorrect reference-equality check in mergeNamedParams with a plain key-presence check - Remove inaccurate Javadoc claim about case normalization in Transaction.setLocalVariable - Add missing HashMap import
arnaud-lacurie
marked this pull request as draft
May 23, 2026 11:37
…ordContextTransaction AbstractMetadataOperationsFactory was delegating getSetLocalVariableConstantAction to NoOpMetadataOperationsFactory, which silently dropped SET LOCAL statements in TransactionBoundDatabase connections. Since SetLocalVariableConstantAction only touches the transaction's session layer (no catalog dependencies), it should be available in all factory contexts. Also adds a test exercising SET LOCAL through the TransactionBoundDatabase/ RecordStoreAndRecordContextTransaction path.
`@body_var` referenced directly in a temp TVF body requires the variable to exist at CREATE time, but uses a live reference to the transaction's local-variable map, so it always reflects the current value at the time the function is called.
…arams injection When a temporary table-valued function body contains a local variable ref (@x), the memoized compilation lambda was using the CREATE-time PreparedParams (empty), causing the COV placeholder to have an incompatible type that crashed plan compilation with COMPARAND_TO_COMPARISON_IS_OF_COMPLEX_TYPE. Fix: inject SELECT-time PreparedParams into the CREATE-time MutablePlanGenerationContext via a ThreadLocal set by BaseVisitor.resolveTableValuedFunction before the lambda runs. The lambda temporarily swaps in the real params for the duration of compilation then restores the originals. With the actual variable value and type available, the COV gets the right type (e.g. STRING) and the binary comparison operator (EQ_SS) is chosen correctly. Also adds variableValueCapturedInContinuation test verifying that the variable value bound at query execution time is captured in the continuation and cannot be overwritten by a subsequent SET LOCAL before EXECUTE CONTINUATION.
…pared params When a temp TVF body containing `?param` was compiled lazily at SELECT time, the ThreadLocal carried the full SELECT-time PreparedParams (which included local variable bindings merged with `?param` bindings). This replaced the CREATE-time `?param` value with the SELECT-time one, causing wrong results. Fix: thread only local variable bindings (@var) through the ThreadLocal and through PlanContext/MutablePlanGenerationContext as a separate field. In the memoized lambda, MERGE the local vars into the CREATE-time params (PreparedParams.withAdditionalNamed) rather than replacing them, so that the function's own ?param bindings are preserved. EmbeddedRelationalStatement and EmbeddedRelationalPreparedStatement both pass the local-vars-only map via PlanContext.withLocalVariables, which PlanGenerator copies into MutablePlanGenerationContext.setLocalVariables. BaseVisitor.resolveTableValuedFunction then sets TVFUNCTION_COMPILATION_PARAMS to that local-vars map (not the full PreparedParams) before triggering the lazy compilation.
1. Restore 4-arg AstNormalizer constructor (backward compat) Our branch added a 5th `deferMissingVarsInFunctionBodies` parameter, removing the 4-arg form that AstNormalizerTests.visitFullDescribeStatementThrows looked up via reflection. Restore it as a delegating overload. 2. Fix function-body cache-key normalization to use CREATE-time ?param bindings normalizeAst's temp-function loop calls normalizeFunctionBody to compute the cache-key contribution of each registered TVF. We mistakenly passed preparedStatementParameters (SELECT-time, no ?param) instead of the routine's own CREATE-time PreparedParams. This broke any test that created a TVF with ?param and then called it from a plain SELECT statement. Fix: add a private normalizeAst overload that carries localVariables (only @var bindings) from normalizeQuery. normalizeFunctionBody now receives PreparedParams.withAdditionalNamed(routine.getPreparedParams(), localVariables): - CREATE-time ?param bindings are preserved (function body semantics) - SELECT-time @var bindings are layered on top (local variable resolution) Since ?param names and @var names are enforced-disjoint at prepare time, no collision is possible. The public 8-arg normalizeAst passes Map.of() so all test call sites remain unchanged.
Remove unreachable BaseVisitor.visitVariableRef (ExpressionVisitor calls visitVariableRef directly, never via ANTLR dispatch). Restore DelegatingVisitor delegation stubs required by TypedVisitor interface. Add direct test for RecordStoreAndRecordContextTransaction.setLocalVariable and getLocalVariables delegation methods.
visitSetLocalVariable, visitVariableRef, and visitVariableRefAtom in DelegatingVisitor are interface-compliance stubs that are never called at runtime — DdlVisitor and ExpressionVisitor always override them. Annotate with @ExcludeFromJacocoGeneratedReport to suppress Teamscale test gap warnings for unreachable delegation boilerplate.
… gaps Add visitSetLocalVariableTest, visitVariableRefTest, and visitVariableRefAtomTest to DelegatingVisitorTest using the existing Mockito delegation pattern. Remove @ExcludeFromJacocoGeneratedReport annotations added in the previous approach.
Store rawUserDefinedFunctionProvider separately so toBuilder() hands back the un-memoized lambda. Previously the memoized BiFunction was passed into the builder, which then called memoize() again in the constructor, wrapping the cache in a redundant outer layer. Add a comment explaining the cache is bounded by transaction lifetime since RecordLayerInvokedRoutine is created per CREATE TEMPORARY FUNCTION and discarded when the transaction ends.
Covers syntax, transaction lifecycle, use in function bodies (permanent and temporary), independence from ?param named parameters, and continuation capture semantics.
…road diagram for SET LOCAL docs
…ained localVariables param
# Conflicts: # docs/sphinx/source/reference/sql_commands/DDL.rst
- RecordLayerInvokedRoutine.memoize: use a null-tolerant unmodifiable map snapshot instead of ImmutableMap.copyOf, which threw NPE on null values (SET LOCAL x = NULL crashed any query invoking a user-defined/table function). - EmbeddedRelationalStatement.createPlanContext: stop injecting local variables into the named-prepared-parameter map so ?name no longer resolves to a same-named local variable on plain Statements (restores @name/?name namespace independence, matching the prepared-statement path). - Add regression tests: nullLocalVariableDoesNotBreakFunctionInvocation and namedParamDoesNotResolveToLocalVariableOnPlainStatement.
Pins the behavior behind the review concern about static/permanent functions. Temporary function bodies resolve @var at call time, but permanent (schema- template) function bodies are compiled against an empty local-variable scope (eager compile in DdlVisitor at CREATE time; RoutineParser uses a fresh context and getSqlFunctionCompiler ignores localVars), so a @var reference in a permanent body surfaces as UNDEFINED_PARAMETER. If permanent-body @var support is added later, this test should be updated.
LOCAL is MySQL's synonym for SESSION scope, not transaction scope, so it misdescribed what this feature actually does. GET_VARIABLE(name) replaces the @var sigil as a dedicated grammar production (like CAST), resolved at AST-normalization time, so it keeps the same plan-cache-safety properties @var had rather than going through the generic function catalog. Also fixes a latent plan-cache bug surfaced while reworking this: the cache key only hashed a variable's name, so re-SETting a variable to a different type (including NULL <-> concrete type) between two runs of identical query text would silently reuse a plan compiled for the old type. AstNormalizer's cache-key text now folds in the variable's current resolved type, forcing a fresh compile on a type change instead.
Collaborator
Author
|
Splitting this into a stack of smaller, individually-testable PRs (per review feedback):
Along the way the syntax changed from |
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
1 participant
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.
Summary
SET TRANSACTION VARIABLE name = valueto bind a named variable scoped to the current FDB transactionGET_VARIABLE(name)to read it — a dedicated grammar production (likeCAST), resolved at normalization time rather than through the generic function catalogFDBRecordContextsession layer (same mechanism asBoundSchemaTemplate) and are not visible after commit/rollbackSETting a variable to a different type (includingNULL<-> concrete) forces a fresh compile instead of reusing a plan shaped for the old type