sql: name resolution/expansion is performed in-place in the AST #22847
Labels
A-sql-optimizer
SQL logical planning and optimizations.
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
T-sql-queries
SQL Queries Team
Comments
knz
added
the
C-bug
nstewart
added
the
S-3-erroneous-edge-case
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
May 13, 2019
This change adds infrastructure for annotating AST nodes. Only certain nodes can be annotated (currently `UnresolvedObjectName`). The parser sets up annotation indices for these nodes, and the annotations are stored in a separate Annotation container. The annotations will allow us to stop modifying the AST during type and semantic checking. The annotation for an `UnresolvedObjectName` will contain the resolved `*TableName`. See cockroachdb#34240 and cockroachdb#22847 for more context. Release note: None
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
May 15, 2019
This change adds infrastructure for annotating AST nodes. Only certain nodes can be annotated (currently `UnresolvedObjectName`). The parser sets up annotation indices for these nodes, and the annotations are stored in a separate Annotation container. The annotations will allow us to stop modifying the AST during type and semantic checking. The annotation for an `UnresolvedObjectName` will contain the resolved `*TableName`. See cockroachdb#34240 and cockroachdb#22847 for more context. Release note: None
craig bot
pushed a commit
that referenced
this issue
May 15, 2019
37484: sql: add AST annotations r=RaduBerinde a=RaduBerinde #### sql: add AST annotations This change adds infrastructure for annotating AST nodes. Only certain nodes can be annotated (currently `UnresolvedObjectName`). The parser sets up annotation indices for these nodes, and the annotations are stored in a separate Annotation container. The annotations will allow us to stop modifying the AST during type and semantic checking. The annotation for an `UnresolvedObjectName` will contain the resolved `*TableName`. See #34240 and #22847 for more context. Release note: None #### sql: use UnresolvedObjectName in AlterTable Change AlterTable to contain an `UnresolvedObjectName`. Release note: None Co-authored-by: Radu Berinde <[email protected]>
|
We have marked this issue as stale because it has been inactive for |
RaduBerinde
removed
the
S-3-erroneous-edge-case
|
Removing the erroneous-edge-case label because there are no known correctness issues - we don't reuse the AST because of these modifications. Leaving the issue open because we would like to stop making these modifications and reuse ASTs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The facilities
ResolveExistingName/ResolveTargetName/ColumnItemResolver(previously:QualifyWithDatabaseName,findColumn) edit the AST node in-place (TableNameorColumnItem).The result of this name resolution however are context-dependent on session variables and the current schema in KV: the current database, the search path, and the descriptors currently visible.
Therefore it is unsafe + incorrect to reuse the modified AST across multiple executes.
The fix for this must either avoid modifying the AST in-place, or ensure the modified AST is not reused.
cc @jordanlewis
Jira issue: CRDB-5841
The text was updated successfully, but these errors were encountered: