andygrove commented on a change in pull request #29273:
URL: https://github.com/apache/spark/pull/29273#discussion_r461746060
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/Columnar.scala
##########
@@ -490,8 +490,10 @@ case class ApplyColumnarRulesAndInsertTransitions(conf:
SQLConf, columnarRules:
// The tree feels kind of backwards
// Columnar Processing will start here, so transition from row to
columnar
RowToColumnarExec(insertTransitions(plan))
- } else {
+ } else if (!plan.isInstanceOf[RowToColumnarExec]) {
Review comment:
I had gone down the same path at one point with similar changes, but
then had to introduce a new `RowToColumnarExecLike` trait so that it would also
work with the `GpuRowToColumnarExec` implementation that we have in our plugin.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/Columnar.scala
##########
@@ -490,8 +490,10 @@ case class ApplyColumnarRulesAndInsertTransitions(conf:
SQLConf, columnarRules:
// The tree feels kind of backwards
// Columnar Processing will start here, so transition from row to
columnar
RowToColumnarExec(insertTransitions(plan))
- } else {
+ } else if (!plan.isInstanceOf[RowToColumnarExec]) {
Review comment:
I wonder if there is a more generic way to test for this, along the
lines of `else if (plan.supportsColumnar &&
!plan.children.head.supportsColumnar)`?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/Columnar.scala
##########
@@ -490,8 +490,10 @@ case class ApplyColumnarRulesAndInsertTransitions(conf:
SQLConf, columnarRules:
// The tree feels kind of backwards
// Columnar Processing will start here, so transition from row to
columnar
RowToColumnarExec(insertTransitions(plan))
- } else {
+ } else if (!plan.isInstanceOf[RowToColumnarExec]) {
Review comment:
Yes, exactly like that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
