gaoyangxiaozhu commented on code in PR #6390:
URL: https://github.com/apache/incubator-gluten/pull/6390#discussion_r1675364652
##########
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala:
##########
@@ -299,16 +299,15 @@ case class OffloadProject() extends OffloadSingleNode
with LogLevelUtil {
// Project is still not transformable after remove `input_file_name`
expressions.
projectExec
} else {
- // the project with `input_file_name` expression should have at most
- // one data source, reference:
+ // the project with `input_file_name` expression may have multiple data
source
+ // by union all, reference:
//
https://github.com/apache/spark/blob/e459674127e7b21e2767cc62d10ea6f1f941936c
- //
/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala#L506
+ //
/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala#L519
val leafScans = findScanNodes(projectExec)
- assert(leafScans.size <= 1)
- if (leafScans.isEmpty || FallbackTags.nonEmpty(leafScans(0))) {
+ if (leafScans.isEmpty || leafScans.forall(FallbackTags.nonEmpty)) {
Review Comment:
hey @zml1206 ，using `exists` is better then `forall`, as `exists` from
semantically, it more intuitively reflects we should fallback if any one scan
node fallback. But here for `union` case, all scan has same schema, that means
if one scan node fallback, other scan nodes must also fallback if my assumption
is right, so i am fine here if still using `forall`.
But yes, changing to `exists` is better.
--
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]
