Skip to content

[SPARK-58933][SQL] Simplify and fix parsed INSERT target handling - #58378

Closed
cloud-fan wants to merge 3 commits into
apache:masterfrom
cloud-fan:fix-identifier-dml-followup
Closed

[SPARK-58933][SQL] Simplify and fix parsed INSERT target handling#58378
cloud-fan wants to merge 3 commits into
apache:masterfrom
cloud-fan:fix-identifier-dml-followup

Conversation

@cloud-fan

@cloud-fancloud-fan commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This follow-up to #58204 simplifies and corrects parsed INSERT target handling:

  • Make the SQL parser always produce UnresolvedInsert for table-target INSERT statements. A
    static target is represented by UnresolvedInsertTarget, while a dynamic IDENTIFIER
    expression remains in PlanWithUnresolvedIdentifier. The analyzer lowers either form to
    InsertIntoStatement once the target identifier is ready for relation resolution.
  • Update parser-oriented consumers, including SQL statement classification, parse_sql lineage
    collection, and pipeline flow registration, to handle the single parsed INSERT shape.
    InsertIntoStatement handling remains where plans can be created programmatically or have
    already been lowered by analysis.
  • Scope the parse_sql CTE-shadow exemption specifically to UnresolvedInsertTarget. DELETE,
    UPDATE, and MERGE targets continue to follow CTE substitution semantics.
  • Run early dynamic INSERT target resolution inside the analysis planning tracker and report its
    failures through QueryPlanningTracker.setAnalysisFailed.

Why are the changes needed?

The parser previously produced InsertIntoStatement for static targets and UnresolvedInsert for
dynamic targets. Parser consumers therefore had to understand both shapes even though
UnresolvedInsert is only an intermediate node and is lowered immediately after its target is
ready. Using one parsed representation makes that boundary explicit and removes duplicated
matching logic.

The target-role exemption added by #58204 was also broader than required. A CTE-shadowed DELETE,
UPDATE, or MERGE target could be reported as a catalog target even though CTE substitution replaces
that relation. In addition, early dynamic target resolution was absent from analysis timing and
failure reporting.

Does this PR introduce any user-facing change?

Yes, within unreleased master only. parse_sql no longer reports CTE-shadowed DELETE, UPDATE, or
MERGE targets as catalog target-table references. There is no change relative to a released Spark
version.

How was this patch tested?

Added and updated regression coverage, then ran:

build/sbt \
"catalyst/testOnly org.apache.spark.sql.catalyst.parser.PlanParserSuite org.apache.spark.sql.catalyst.parser.DDLParserSuite org.apache.spark.sql.catalyst.parser.IdentifierClauseParserSuite" \
"sql/testOnly org.apache.spark.sql.ParametersSuite org.apache.spark.sql.catalyst.parser.ParseSqlResultSuite org.apache.spark.sql.execution.QueryExecutionSuite org.apache.spark.sql.execution.command.v2.CreateFlowCommandSuite" \
pipelines/compile

All 284 Catalyst tests and 204 SQL tests passed, and the pipelines module compiled successfully.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: OpenAI Codex (GPT-5)

@cloud-fancloud-fan changed the title [SPARK-58933][SQL] Address dynamic INSERT review follow-ups[SPARK-58933][SQL] Simplify and fix parsed INSERT target handlingAug 28, 2026
@cloud-fan

Copy link
Copy Markdown
ContributorAuthor

thanks for review, merging to master/4.x/4.3/4.2

cloud-fan added a commit that referenced this pull request Aug 28, 2026
### What changes were proposed in this pull request?
This follow-up to #58204 simplifies and corrects parsed INSERT target handling:
* Make the SQL parser always produce `UnresolvedInsert` for table-target INSERT statements. A
static target is represented by `UnresolvedInsertTarget`, while a dynamic `IDENTIFIER`
expression remains in `PlanWithUnresolvedIdentifier`. The analyzer lowers either form to
`InsertIntoStatement` once the target identifier is ready for relation resolution.
* Update parser-oriented consumers, including SQL statement classification, `parse_sql` lineage
collection, and pipeline flow registration, to handle the single parsed INSERT shape.
`InsertIntoStatement` handling remains where plans can be created programmatically or have
already been lowered by analysis.
* Scope the `parse_sql` CTE-shadow exemption specifically to `UnresolvedInsertTarget`. DELETE,
UPDATE, and MERGE targets continue to follow CTE substitution semantics.
* Run early dynamic INSERT target resolution inside the analysis planning tracker and report its
failures through `QueryPlanningTracker.setAnalysisFailed`.
### Why are the changes needed?
The parser previously produced `InsertIntoStatement` for static targets and `UnresolvedInsert` for
dynamic targets. Parser consumers therefore had to understand both shapes even though
`UnresolvedInsert` is only an intermediate node and is lowered immediately after its target is
ready. Using one parsed representation makes that boundary explicit and removes duplicated
matching logic.
The target-role exemption added by #58204 was also broader than required. A CTE-shadowed DELETE,
UPDATE, or MERGE target could be reported as a catalog target even though CTE substitution replaces
that relation. In addition, early dynamic target resolution was absent from analysis timing and
failure reporting.
### Does this PR introduce _any_ user-facing change?
Yes, within unreleased master only. `parse_sql` no longer reports CTE-shadowed DELETE, UPDATE, or
MERGE targets as catalog target-table references. There is no change relative to a released Spark
version.
### How was this patch tested?
Added and updated regression coverage, then ran:
```bash
build/sbt \
"catalyst/testOnly org.apache.spark.sql.catalyst.parser.PlanParserSuite org.apache.spark.sql.catalyst.parser.DDLParserSuite org.apache.spark.sql.catalyst.parser.IdentifierClauseParserSuite" \
"sql/testOnly org.apache.spark.sql.ParametersSuite org.apache.spark.sql.catalyst.parser.ParseSqlResultSuite org.apache.spark.sql.execution.QueryExecutionSuite org.apache.spark.sql.execution.command.v2.CreateFlowCommandSuite" \
pipelines/compile
```
All 284 Catalyst tests and 204 SQL tests passed, and the pipelines module compiled successfully.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-5)
Closes#58378 from cloud-fan/fix-identifier-dml-followup.
Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 900b44e)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan

Copy link
Copy Markdown
ContributorAuthor

Merge Summary:

Posted by merge_spark_pr.py

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@cloud-fan@zhengruifeng