[SPARK-58933][SQL][4.2] Resolve expressions in INSERT target IDENTIFIER clauses - #58373
Closed
cloud-fan wants to merge 1 commit into
Closed
[SPARK-58933][SQL][4.2] Resolve expressions in INSERT target IDENTIFIER clauses#58373cloud-fan wants to merge 1 commit into
cloud-fan wants to merge 1 commit into
Conversation
…auses ### What changes were proposed in this pull request? This PR introduces a temporary binary `UnresolvedInsert` plan for INSERT statements whose target uses a dynamic `IDENTIFIER(...)` expression. Its target and input query are ordinary children, so analyzer rules can resolve parameters, functions, and SQL variables in the target expression through normal tree traversal. After the expression is evaluated, its builder creates an `UnresolvedInsertTarget` containing the raw multipart identifier, target options, and write privileges. The analyzer then converts this marker to an `UnresolvedRelation` and lowers `UnresolvedInsert` to the existing unary `InsertIntoStatement`. The target is subsequently qualified and resolved by the existing INSERT analysis path. `UnresolvedInsertTarget` is deliberately not a `NamedRelation`, so CTE substitution cannot treat an INSERT target as a readable relation. Static table names and literal `IDENTIFIER` clauses still parse directly to `InsertIntoStatement` and retain the existing behavior. `QueryExecution` resolves only the dynamic target identifier before transaction detection. This allows transactional INSERTs to be recognized without adding a separate target-resolution path or changing the long-lived child contract of `InsertIntoStatement`. `ResolveUnresolvedInsert` uses a dedicated tree pattern and rule ID, so the fixed-point Resolution batch skips plans that cannot contain the temporary carrier. ### Why are the changes needed? `InsertIntoStatement.table` is not a logical-plan child, so normal analyzer rules do not visit a dynamic `IDENTIFIER` expression stored in that slot. Nested functions, parameters, or SQL variables can therefore remain unresolved. Making `InsertIntoStatement` permanently binary would affect analyzer, planner, lineage, and command consumers that rely on its input query being its only child. The temporary `UnresolvedInsert` node limits the binary shape to identifier-expression resolution, while `UnresolvedInsertTarget` preserves the raw name until normal relation resolution. This also avoids qualifying an already-resolved identifier a second time. ### Does this PR introduce _any_ user-facing change? Yes. INSERT statements can now resolve expressions inside a target `IDENTIFIER` clause. For example, the following statement now resolves and executes correctly: ```sql INSERT INTO IDENTIFIER( lower(regexp_replace(:table_name, 'PLACEHOLDER', 'TBL'))) REPLACE WHERE id = 1 VALUES (1, 'updated') ``` ### How was this patch tested? Added parser, analyzer, end-to-end, parse-metadata lineage and classification, and transactional-catalog coverage for dynamic INSERT targets, including parameters, nested functions, SQL variables, qualified multipart names, query-source and select-list extraction, and a target sharing its name with a CTE. The following compile and test checks passed: ```bash build/sbt catalyst/Test/compile build/sbt 'sql/testOnly org.apache.spark.sql.ParametersSuite' build/sbt 'sql/testOnly org.apache.spark.sql.catalyst.parser.ParseSqlResultSuite' build/sbt 'sql/testOnly org.apache.spark.sql.connector.AppendDataTransactionSuite' build/sbt pipelines/compile build/sbt connect/compile SPARK_GENERATE_GOLDEN_FILES=1 build/sbt \ 'sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z explain.sql' SPARK_GENERATE_GOLDEN_FILES=1 build/sbt \ 'sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z explain-aqe.sql' ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: OpenAI Codex (GPT-5) Closes apache#58204 from cloud-fan/fix-identifier-dml-table-resolution. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
gengliangwang
approved these changes
Aug 28, 2026
Contributor
Author
|
thanks for review, merging to 4.2! |
cloud-fan
added a commit
that referenced
this pull request
Aug 28, 2026
…ER clauses ### What changes were proposed in this pull request? This backports #58204 (`3bc29d9d0ae031dfe0e793886ba652ca98ae7f29`) to branch-4.2. It introduces a temporary binary `UnresolvedInsert` plan for INSERT statements whose target uses a dynamic `IDENTIFIER(...)` expression. This lets normal analyzer traversal resolve parameters, functions, and SQL variables in the target before lowering the plan to the existing write plan. For branch-4.2, `INSERT ... REPLACE WHERE` is lowered to that branch's `OverwriteByExpression` representation. The transactional regression test is also expressed using the older transaction test harness. The parse-metadata changes from the original commit are omitted because the corresponding `ParseSqlResult` and `SqlStatementCodes` infrastructure is not present on branch-4.2. ### Why are the changes needed? `InsertIntoStatement.table` is not a logical-plan child, so analyzer rules do not normally visit a dynamic target expression stored there. Nested functions, parameters, or SQL variables can therefore remain unresolved. ### Does this PR introduce _any_ user-facing change? Yes. INSERT statements on branch-4.2 can now resolve expressions inside a target `IDENTIFIER` clause, matching the behavior fixed on master by #58204. ### How was this patch tested? The backported parser, analyzer, end-to-end, and transactional-catalog tests were included and adapted to branch-4.2. The following checks passed locally with Java 17: ```bash build/sbt -java-home /usr/lib/jvm/java-17-openjdk-amd64 \ catalyst/Test/compile \ 'sql/testOnly org.apache.spark.sql.ParametersSuite' \ 'sql/testOnly org.apache.spark.sql.connector.AppendDataTransactionSuite' \ pipelines/compile connect/compile ``` `ParametersSuite`: 161 tests passed. `AppendDataTransactionSuite`: 18 tests passed. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: OpenAI Codex (GPT-5) Closes #58373 from cloud-fan/SPARK-58933-branch-4.2. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Contributor
Author
|
Merge Summary:
Posted by |
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
2 participants
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.
What changes were proposed in this pull request?
This backports #58204 (
3bc29d9d0ae031dfe0e793886ba652ca98ae7f29) to branch-4.2.It introduces a temporary binary
UnresolvedInsertplan for INSERT statements whose target uses adynamic
IDENTIFIER(...)expression. This lets normal analyzer traversal resolve parameters,functions, and SQL variables in the target before lowering the plan to the existing write plan.
For branch-4.2,
INSERT ... REPLACE WHEREis lowered to that branch'sOverwriteByExpressionrepresentation. The transactional regression test is also expressed usingthe older transaction test harness. The parse-metadata changes from the original commit are omitted
because the corresponding
ParseSqlResultandSqlStatementCodesinfrastructure is not present onbranch-4.2.
Why are the changes needed?
InsertIntoStatement.tableis not a logical-plan child, so analyzer rules do not normally visit adynamic target expression stored there. Nested functions, parameters, or SQL variables can
therefore remain unresolved.
Does this PR introduce any user-facing change?
Yes. INSERT statements on branch-4.2 can now resolve expressions inside a target
IDENTIFIERclause, matching the behavior fixed on master by #58204.
How was this patch tested?
The backported parser, analyzer, end-to-end, and transactional-catalog tests were included and
adapted to branch-4.2. The following checks passed locally with Java 17:
ParametersSuite: 161 tests passed.AppendDataTransactionSuite: 18 tests passed.Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-5)