marin-ma commented on code in PR #8798:
URL: https://github.com/apache/incubator-gluten/pull/8798#discussion_r1968080339
##########
gluten-substrait/src/main/scala/org/apache/gluten/extension/GlutenPlan.scala:
##########
@@ -26,6 +26,7 @@ import org.apache.spark.sql.catalyst.trees.TreeNode
sealed trait ValidationResult {
def ok(): Boolean
def reason(): String
+ def merge(other: ValidationResult): ValidationResult
Review Comment:
If we want to consolidate the two, the validation failure should match the
tagged operator. However, the validation result failures are not strictly bound
to the tagged operator. For example in ShuffleExchangeExec, we add a sort
before exchange for round-robin partitioning and the validation failure of the
Sort will add a fallback tag on ColumnarShuffleExchange.
Merging the validation reason is more for the debugging purpose to give the
developer more information by not returning the validation result immediately.
As it's merged in a single validation call, it should be considered as one
source of fallback. It seems that appending FallbackTags is intended to add
multiple fallback reasons to an operator when fallback is triggered by
different sources.
--
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]
