HyukjinKwon commented on code in PR #59310:
URL: https://github.com/apache/spark/pull/59310#discussion_r4267601699
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -4878,6 +4878,14 @@ private[sql] object QueryCompilationErrors extends
QueryErrorsBase with Compilat
)
}
+ def subqueryPlanNotAttached(u: UnresolvedPlanId): Throwable = {
+ new AnalysisException(
Review Comment:
**Non-blocking (P2):** This makes the planner's eager-analysis fallback
record a failed SQL execution for valid queries.
`SparkConnectPlanner.transformWithColumns` (and `transformWithColumnsRenamed`)
first try `Dataset.ofRows(session, child).withColumns(...)` while still inside
the `WithRelations` root transform, before the attached subquery plans are
substituted. They fall back to a lazy plan on `AnalysisException` or
`SparkException`. With this change, that eager attempt fails at the new
`CheckAnalysis` case with an `AnalysisException`.
`QueryExecution.assertAnalyzed` handles an `AnalysisException` by calling
`SQLExecution.withNewExecutionIdOnError(this, Some("analyze"))`, which posts
SQL execution start and end events before the planner catches the error. On
master, the same attempt fails with the internal `SparkException`, which
`assertAnalyzed` does not report.
For example, the Python client's `t1.withColumn("scalar",
spark.range(1).select(sf.col("c1").outer() + sf.col("c2").outer()).scalar())`,
covered by `test_subquery_in_with_columns` under Connect, attaches the subquery
in `WithRelations` and returns the right result. Yet every time it is planned,
the SQL UI and every SQL listener now see a failed `analyze` execution carrying
`SUBQUERY_PLAN_NOT_ATTACHED`. Results are unaffected, but the SQL tab and any
monitoring that counts failed executions show false failures.
**Recommended change:** In SparkConnectPlanner.transformWithColumns and
transformWithColumnsRenamed, skip the eager analysis attempt when the new
column expressions or the input plan still contain an unsubstituted plan id
(the UNRESOLVED_PLAN_ID tree pattern), and build the lazy
UnresolvedStarWithColumns or UnresolvedStarWithColumnsRenames plan directly.
Add a Connect planner test covering this case.
**Why this works:** The eager attempt cannot succeed while a plan id is
unsubstituted, so skipping it yields the same plan the fallback already
produced, and no AnalysisException reaches assertAnalyzed during planning of a
valid query.
**Scope:** Avoid the doomed eager-analysis attempt in the planner's
withColumns and withColumnsRenamed fallbacks when a subquery plan id is still
unsubstituted.
**Compatibility:** The planned result of withColumns and withColumnsRenamed
requests is unchanged from the merge target.
**Risks:** The pattern check must cover both the new expressions and the
input plan, or a plan id nested in the input would still trigger the event.
**Constraints:** Keep the eager path unchanged for withColumns and
withColumnsRenamed requests that contain no unsubstituted plan id. Keep the
fallback plan shape identical to the merge target.
**Success:** Planning a valid WithRelations withColumn or withColumnsRenamed
whose expressions or input still contain an unsubstituted plan id posts no SQL
execution events and returns the same lazy plan as the merge target.
withColumns and withColumnsRenamed requests without plan ids still take the
eager-analysis path with unchanged results.
**Verification:**
- **Regression:** Planning a WithRelations whose root is a withColumns with
a scalar subquery records no SQL execution start or end event and yields the
expected result.
- **Compatibility:** Connect DataFrame subquery tests that use withColumn
and withColumnsRenamed keep producing the same results, and withColumns without
subqueries keeps its eager-analysis behavior.
--
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]
