Skip to content

Thread local variables through table-valued function resolution - #4566

Draft
arnaud-lacurie wants to merge 2 commits into
apple/arnaud-lacurie/local-vars/3-getfrom
apple/arnaud-lacurie/local-vars/4-tvf
Draft

Thread local variables through table-valued function resolution#4566
arnaud-lacurie wants to merge 2 commits into
apple/arnaud-lacurie/local-vars/3-getfrom
apple/arnaud-lacurie/local-vars/4-tvf

Conversation

@arnaud-lacurie

Copy link
Copy Markdown
Collaborator

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.

Stack created with GitHub Stacks CLIGive Feedback 💬

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
arnaud-lacurie force-pushed the apple/arnaud-lacurie/local-vars/4-tvf branch from 692bb7a to a948e67 Compare September 5, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant