MaxGekk commented on code in PR #56429:
URL: https://github.com/apache/spark/pull/56429#discussion_r3389600803
##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveExecuteImmediate.scala:
##########
@@ -43,23 +43,45 @@ case class ResolveExecuteImmediate(sparkSession:
SparkSession, catalogManager: C
plan.resolveOperatorsWithPruning(_.containsPattern(EXECUTE_IMMEDIATE),
ruleId) {
case node @ UnresolvedExecuteImmediate(sqlStmtStr, args,
targetVariables) =>
if (sqlStmtStr.resolved && targetVariables.forall(_.resolved) &&
args.forall(_.resolved)) {
- // All resolved - execute immediately and handle INTO clause if
present
- if (targetVariables.nonEmpty) {
- // EXECUTE IMMEDIATE ... INTO should generate SetVariable plan
with eagerly executed
- // source
- val finalTargetVars = extractTargetVariables(targetVariables)
- val executedSource = executeImmediateQuery(sqlStmtStr, args,
hasIntoClause = true)
- SetVariable(finalTargetVars, executedSource)
- } else {
- // Regular EXECUTE IMMEDIATE without INTO - execute and return
result directly
- executeImmediateQuery(sqlStmtStr, args, hasIntoClause = false)
- }
+ ResolveExecuteImmediate.resolveExecuteImmediate(
Review Comment:
This delegation only passes `sparkSession` to the companion object. The
`catalogManager: CatalogManager` parameter declared alongside it in the class
constructor is never used — not in the class body, not in the companion object.
Since the PR is already restructuring the class, consider removing
`catalogManager` from the constructor and updating the two callers:
`BaseSessionStateBuilder.scala:242` and `HiveSessionStateBuilder.scala:137`.
##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveExecuteImmediate.scala:
##########
@@ -43,23 +43,45 @@ case class ResolveExecuteImmediate(sparkSession:
SparkSession, catalogManager: C
plan.resolveOperatorsWithPruning(_.containsPattern(EXECUTE_IMMEDIATE),
ruleId) {
case node @ UnresolvedExecuteImmediate(sqlStmtStr, args,
targetVariables) =>
if (sqlStmtStr.resolved && targetVariables.forall(_.resolved) &&
args.forall(_.resolved)) {
- // All resolved - execute immediately and handle INTO clause if
present
- if (targetVariables.nonEmpty) {
- // EXECUTE IMMEDIATE ... INTO should generate SetVariable plan
with eagerly executed
- // source
- val finalTargetVars = extractTargetVariables(targetVariables)
- val executedSource = executeImmediateQuery(sqlStmtStr, args,
hasIntoClause = true)
- SetVariable(finalTargetVars, executedSource)
- } else {
- // Regular EXECUTE IMMEDIATE without INTO - execute and return
result directly
- executeImmediateQuery(sqlStmtStr, args, hasIntoClause = false)
- }
+ ResolveExecuteImmediate.resolveExecuteImmediate(
+ sparkSession, sqlStmtStr, args, targetVariables)
} else {
- // Not all resolved yet - wait for next iteration
node
Review Comment:
The previous code had `// Not all resolved yet - wait for next iteration`
here, explaining the iterative analysis idiom (returning `node` unchanged
defers re-application to the next fixed-point pass). Consider restoring it:
```suggestion
} else {
// Not all resolved yet - wait for next iteration
node
```
--
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]
