sarutak commented on code in PR #59425:
URL: https://github.com/apache/spark/pull/59425#discussion_r4319009978
##########
sql/connect/server/src/test/scala/org/apache/spark/sql/connect/pipelines/SparkDeclarativePipelinesServerSuite.scala:
##########
@@ -1164,4 +1164,127 @@ class SparkDeclarativePipelinesServerSuite
relation.getCommand.getSqlCommand.getInput)
}
}
+
+ // Helper to build a SqlCommand ExecutePlanRequest with
PipelineAnalysisContext.
+ private def buildSqlCommandRequestWithPipelineContext(
+ graphId: String,
+ input: proto.Relation): proto.ExecutePlanRequest = {
+ val context = proto.PipelineAnalysisContext
+ .newBuilder()
+ .setDataflowGraphId(graphId)
Review Comment:
Nit, not blocking. This helper only sets `dataflowGraphId`, never
`flowName`, so all three new tests exercise the path outside a pipeline flow
function. The guard runs at the `hasPipelineAnalysisContext` check, which is
before the `isInsidePipelineFlowFunction` early return, so the bug existed
inside a flow function too: on the old code the guard ran `executeSQLWithRefs`
(e.g. a `CREATE TABLE` root) and the resulting `CommandResult` passed the
blocklist. There was no second execution there (the handler then returns the
relation), but the command still ran. That is also the most common real shape:
inside a query function, `spark.sql("... {df}", df=...)` sends exactly this
WITH_RELATIONS with `SubqueryAlias` references, and `register_flow` wraps
`flow.func()` in `add_pipeline_analysis_context(..., flow_name=flow.name)`.
Could the helper take an optional `flowName` and the two command tests run
both with and without it (e.g. via `gridTest`)? On the old code the
flow-function variant would also fail on both `intercept[RuntimeException]` and
`assert(!tableExists)`; on the current code it pins that the guard keeps
running before the flow-function return. This dovetails with @HyukjinKwon's
suggestion below to give the helper a reference-input variant, so both are
about widening what the new tests actually cover.
--
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]
