address comment

apache · Feb 25, 2025 · 293afaa · 293afaa
1 parent 4ea076d
commit 293afaa
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 16 deletions.
diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/execution/WholeStageTransformer.scala b/gluten-substrait/src/main/scala/org/apache/gluten/execution/WholeStageTransformer.scala
@@ -118,7 +118,8 @@ trait ValidatablePlan extends GlutenPlan with LogLevelUtil {
     if (!validationResult.ok()) {
       TestStats.addFallBackClassName(this.getClass.toString)
     }
-    if (validationFailFast) validationResult else schemaValidationResult.merge(validationResult)
+    if (validationFailFast) validationResult
+    else ValidationResult.merge(schemaValidationResult, validationResult)
   }
 
   protected def doValidateInternal(): ValidationResult = ValidationResult.succeeded

diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/extension/ValidationResult.scala b/gluten-substrait/src/main/scala/org/apache/gluten/extension/ValidationResult.scala
@@ -26,7 +26,6 @@ import org.apache.spark.sql.catalyst.trees.TreeNode
 sealed trait ValidationResult {
   def ok(): Boolean
   def reason(): String
-  def merge(other: ValidationResult): ValidationResult
 }
 
 object ValidationResult {
@@ -43,28 +42,23 @@ object ValidationResult {
     override def ok(): Boolean = true
     override def reason(): String = throw new UnsupportedOperationException(
       "Succeeded validation doesn't have failure details")
-
-    override def merge(other: ValidationResult): ValidationResult = {
-      if (!other.ok()) {
-        return other
-      }
-      this
-    }
   }
 
   private case class Failed(override val reason: String) extends ValidationResult {
     override def ok(): Boolean = false
-
-    override def merge(other: ValidationResult): ValidationResult = {
-      if (!other.ok()) {
-        return ValidationResult.failed(reason + other.reason(), prefix = "")
-      }
-      this
-    }
   }
 
   def succeeded: ValidationResult = Succeeded
   def failed(reason: String, prefix: String = "\n - "): ValidationResult = Failed(prefix + reason)
+  def merge(left: ValidationResult, right: ValidationResult): ValidationResult =
+    (left.ok(), right.ok()) match {
+      case (_, true) =>
+        left
+      case (true, false) =>
+        right
+      case (false, false) =>
+        failed(left.reason() + right.reason(), prefix = "")
+    }
 
   implicit class EncodeFallbackTagImplicits(result: ValidationResult) {
     def tagOnFallback(plan: TreeNode[_]): Unit = {