Skip to content

[SPARK-46625][SQL][FOLLOWUP] Resolve identifier expression in InsertIntoStatement/V2WriteCommand table slot #56024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
haoyangeng-db wants to merge 1 commit into
base: master
Choose a base branch
from
+48 −0
Open

[SPARK-46625][SQL][FOLLOWUP] Resolve identifier expression in InsertIntoStatement/V2WriteCommand table slot #56024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,32 @@ class Analyzer(
}

def doApply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp {
// `InsertIntoStatement.table` and `V2WriteCommand.table` are non-child `LogicalPlan`
// slots (`child = query`), so the default `resolveOperatorsUp` + `mapExpressions`
// traversal never resolves expressions placed inside them. For a
// `PlanWithUnresolvedIdentifier`, `identifierExpr` (e.g. an `UnresolvedAttribute`
// referring to a SQL variable in `INSERT INTO IDENTIFIER(target_table) ...`) must
// be resolved here before `ResolveIdentifierClause` can materialize the relation.
// Mirror the recursion that `BindParameters` and `ResolveIdentifierClause` already
// do for the same shape (SPARK-46625). The `!identifierExpr.resolved` guard makes
// the case idempotent under bottom-up traversal.
case i: InsertIntoStatement
if i.table.isInstanceOf[PlanWithUnresolvedIdentifier] &&
!i.table.asInstanceOf[PlanWithUnresolvedIdentifier].identifierExpr.resolved =>
val p = i.table.asInstanceOf[PlanWithUnresolvedIdentifier]
val resolvedExpr = resolveExpressionByPlanChildren(
p.identifierExpr, i, includeLastResort = true)
Copy link
Copy Markdown
Contributor

@cloud-fan cloud-fan May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this resolve against p rather than i? Passing i widens the resolution scope to i.children = Seq(query), so a nested UnresolvedAttribute in identifierExpr can match a query output column. With a colliding name like

DECLARE VARIABLE a STRING DEFAULT 't_shadow';
CREATE TABLE t_shadow (a INT) USING PARQUET;
INSERT INTO IDENTIFIER(a) SELECT 42 AS a;

the column wins and IdentifierResolution.evalIdentifierExpr then throws NOT_A_CONSTANT_STRING.NOT_CONSTANT (since an AttributeReference is not foldable). Resolving against p (whose children are Nil for the INSERT/OverwriteByExpression path built by buildWriteTableSlot) mirrors how PlanWithUnresolvedIdentifier is resolved in regular child slots via the generic fallback at line 1865 — column resolution becomes a no-op and only the last-resort variable resolution fires.

Same point applies to the V2WriteCommand case below (line 1561).

resolveColumnDefaultInCommandInputQuery(
Copy link
Copy Markdown
Contributor

@cloud-fan cloud-fan May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wrap looks like a no-op in this iteration: ResolveColumnDefaultInCommandInputQuery.apply short-circuits unless i.table.resolved, and i.table here is still a PlanWithUnresolvedIdentifier (just with the identifier expression now resolved). The column-default work happens later via the existing case i: InsertIntoStatement => resolveColumnDefaultInCommandInputQuery(i) once ResolveIdentifierClause has materialized the table. The V2WriteCommand case below doesn't wrap — consider dropping the wrap here too for symmetry:

Suggested change
resolveColumnDefaultInCommandInputQuery(
i.copy(table = p.copy(identifierExpr = resolvedExpr))

i.copy(table = p.copy(identifierExpr = resolvedExpr)))

case w: V2WriteCommand
if w.table.isInstanceOf[PlanWithUnresolvedIdentifier] &&
!w.table.asInstanceOf[PlanWithUnresolvedIdentifier].identifierExpr.resolved =>
val p = w.table.asInstanceOf[PlanWithUnresolvedIdentifier]
val resolvedExpr = resolveExpressionByPlanChildren(
p.identifierExpr, w, includeLastResort = true)
w.withNewTable(p.copy(identifierExpr = resolvedExpr))

// Don't wait other rules to resolve the child plans of `InsertIntoStatement` as we need
// to resolve column "DEFAULT" in the child plans so that they must be unresolved.
case i: InsertIntoStatement => resolveColumnDefaultInCommandInputQuery(i)
Expand Down
22 changes: 22 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/ParametersSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2586,6 +2586,28 @@ class ParametersSuite extends SharedSparkSession {
s"Expected :tname inside OverwriteByExpression.table to be bound, got:\n$boundOverwrite")
}

// SPARK-46625 followup: `INSERT INTO IDENTIFIER(<sql-variable>) ...` places a
// `PlanWithUnresolvedIdentifier` in `InsertIntoStatement.table`, whose `identifierExpr`
// holds an `UnresolvedAttribute` for the variable name. That slot is a non-child
// `LogicalPlan`, so the default `ResolveReferences` traversal never resolves the
// attribute, `ResolveIdentifierClause` cannot fire (it waits on `identifierExpr.resolved`),
// and analysis fails. Verify that the explicit `InsertIntoStatement` case added to
// `ResolveReferences` rewrites the attribute to a `VariableReference` and the insert
// completes end-to-end.
test("SPARK-46625: INSERT INTO IDENTIFIER(<sql-variable>) resolves variable in table slot") {
withTable("t_var_insert") {
sql("CREATE TABLE t_var_insert (a INT) USING PARQUET")
sql("DECLARE OR REPLACE VARIABLE target_table STRING")
try {
sql("SET VAR target_table = 't_var_insert'")
sql("INSERT INTO IDENTIFIER(target_table) SELECT 42 AS a")
Copy link
Copy Markdown
Contributor

@cloud-fan cloud-fan May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth adding a companion test where the SQL variable name matches a query output column (e.g. variable a, table t_shadow(a INT), INSERT INTO IDENTIFIER(a) SELECT 42 AS a) to lock in that the IDENTIFIER expression doesn't see query output columns. With the resolution-scope question above, that test would fail today on the colliding-name case; after narrowing the scope it should succeed.

checkAnswer(spark.table("t_var_insert"), Row(42))
} finally {
sql("DROP TEMPORARY VARIABLE IF EXISTS target_table")
}
}
}

// SPARK-46625: `CacheTableAsSelect.tempViewName` is an `Expression` slot, so an
// `IDENTIFIER(<non-literal>)` produces an `ExpressionWithUnresolvedIdentifier` there instead of
// wrapping the entire command in a `PlanWithUnresolvedIdentifier`. Verify on the parsed plan
Expand Down