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]
