peter-toth commented on code in PR #55949:
URL: https://github.com/apache/spark/pull/55949#discussion_r3265424569
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala:
##########
@@ -459,6 +459,23 @@ trait CheckAnalysis extends LookupCatalog with
QueryErrorsBase with PlanToString
messageParameters = Map("name" -> "IDENTIFIER", "expr" ->
p.identifierExpr.sql)
)
+ case c: CacheTableAsSelect if c.tempViewName.resolved =>
Review Comment:
The `if c.tempViewName.resolved` guard was added in commit 90f09a90f5
specifically to keep this invariant check from pre-empting the proper
`UNRESOLVED_COLUMN` user-facing error when `IDENTIFIER(<unresolved-col>)`
itself fails to resolve. The comment documents the intent well, but there's no
test that pins this behavior — a future change that drops the guard (e.g. `case
c: CacheTableAsSelect =>` without the guard) would silently swap the
user-facing error for a `SparkException internal error`, with nothing in CI to
catch it.
Worth adding a regression test alongside the SPARK-46625 cache-table tests
in `ParametersSuite`. Rough shape:
```scala
test("SPARK-46625: CACHE TABLE IDENTIFIER(<unresolved-col>) reports
UNRESOLVED_COLUMN, not an internal error") {
val ex = intercept[AnalysisException] {
spark.sql(
"""CACHE TABLE IDENTIFIER(unresolved_col) AS SELECT 1 AS
a""".stripMargin)
}
assert(ex.getCondition.startsWith("UNRESOLVED_COLUMN"))
// and explicitly NOT the internal-error condition the invariant case
throws
assert(!ex.getMessage.contains("CacheTableAsSelect.tempViewName must be"))
}
```
That pins both halves of the design — the guard fires for unresolved
IDENTIFIERs (so the catch-all `LogicalPlan` case below produces
`UNRESOLVED_COLUMN`), AND the case below it stays the catch-all, not pre-empted
by this branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
