[SPARK-34701][SQL][FOLLOW-UP] Children/innerChildren should be mutually exclusive for AnalysisOnlyCommand #32447
Conversation
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
cc @cloud-fan |
|
Test build #138185 has finished for PR 32447 at commit
|
sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
Outdated
Show resolved
Hide resolved
| @@ -46,5 +47,6 @@ trait AnalysisOnlyCommand extends Command { | |||
| val isAnalyzed: Boolean | |||
| def childrenToAnalyze: Seq[LogicalPlan] | |||
| override final def children: Seq[LogicalPlan] = if (isAnalyzed) Nil else childrenToAnalyze | |||
| override def innerChildren: Seq[QueryPlan[_]] = if (isAnalyzed) childrenToAnalyze else Nil | |||
There was a problem hiding this comment.
@cloud-fan, updated. Do you also want to explicitly override this to be Nil for CacheTable, CacheTableAsSelect and UncacheTable?
There was a problem hiding this comment.
Does it have real impact in EXPLAIN?
There was a problem hiding this comment.
There is a change, but I think it's for better:
Before:
== Parsed Logical Plan ==
'CacheTableAsSelect tempTable, SELECT key FROM testData, false, false
+- 'Project ['key]
+- 'UnresolvedRelation [testData], [], false
== Analyzed Logical Plan ==
CacheTableAsSelect tempTable, Project [key#13], SELECT key FROM testData, false, true
== Optimized Logical Plan ==
CacheTableAsSelect tempTable, Project [key#13], SELECT key FROM testData, false, true
== Physical Plan ==
CacheTableAsSelect tempTable, Project [key#13], SELECT key FROM testData, false
New:
== Parsed Logical Plan ==
'CacheTableAsSelect tempTable, SELECT key FROM testData, false, false
+- 'Project ['key]
+- 'UnresolvedRelation [testData], [], false
== Analyzed Logical Plan ==
CacheTableAsSelect tempTable, SELECT key FROM testData, false, true
+- Project [key#13]
+- SubqueryAlias testdata
+- View (`testData`, [key#13,value#14])
+- SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).key AS key#13, staticinvoke(class org.apache.spark.unsafe.types.UTF8String, StringType, fromString, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).value, true, false) AS value#14]
+- ExternalRDD [obj#12]
== Optimized Logical Plan ==
CacheTableAsSelect tempTable, SELECT key FROM testData, false, true
+- Project [key#13]
+- SubqueryAlias testdata
+- View (`testData`, [key#13,value#14])
+- SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).key AS key#13, staticinvoke(class org.apache.spark.unsafe.types.UTF8String, StringType, fromString, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).value, true, false) AS value#14]
+- ExternalRDD [obj#12]
== Physical Plan ==
CacheTableAsSelect tempTable, Project [key#13], SELECT key FROM testData, false
WDYT?
There was a problem hiding this comment.
And we can improve the EXPLAIN for physical plans as well as a future PR if needed.
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
imback82
changed the title
|
Test build #138217 has finished for PR 32447 at commit
|
|
thanks, merging to master! |
What changes were proposed in this pull request?
This is a follow up to #32032 (comment). Basically,
children/innerChildrenshould be mutually exclusive forAlterViewAsCommandandCreateViewCommand, which extendAnalysisOnlyCommand. Otherwise, there could be an issue in theEXPLAINcommand. Currently, this is not an issue, because these commands will be analyzed (children will always be empty) when theEXPLAINcommand is run.Why are the changes needed?
To be future-proof where these commands are directly used.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added new tsts