From 40fc52abc1d1f50bae2655a4fdc24222451b0fbe Mon Sep 17 00:00:00 2001 From: Hongze Zhang Date: Thu, 31 Oct 2024 10:10:19 +0800 Subject: [PATCH] [GLUTEN-7143][VL] RAS: Fix fallen back plan nodes are not tagged with meaningful fallback reasons (#7731) --- .../gluten/expression/ExpressionConverter.scala | 2 +- .../extension/columnar/enumerated/RasOffload.scala | 13 ++++++++----- .../expressions/GlutenExpressionMappingSuite.scala | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala index 72586b034936..f1bf2d00d0ab 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala @@ -484,7 +484,7 @@ object ExpressionConverter extends SQLConfHelper with Logging { c) case c if c.getClass.getSimpleName.equals("CheckOverflowInTableInsert") => throw new GlutenNotSupportException( - "CheckOverflowInTableInsert is used in ansi mode, but gluten does not support ANSI mode." + "CheckOverflowInTableInsert is used in ANSI mode, but Gluten does not support ANSI mode." ) case b: BinaryArithmetic if DecimalArithmeticUtil.isDecimalArithmetic(b) => DecimalArithmeticUtil.checkAllowDecimalArithmetic() diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/enumerated/RasOffload.scala b/gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/enumerated/RasOffload.scala index fc29b36f08d8..1299fffb995c 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/enumerated/RasOffload.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/enumerated/RasOffload.scala @@ -17,7 +17,7 @@ package org.apache.gluten.extension.columnar.enumerated import org.apache.gluten.extension.GlutenPlan -import org.apache.gluten.extension.columnar.OffloadSingleNode +import org.apache.gluten.extension.columnar.{FallbackTags, OffloadSingleNode} import org.apache.gluten.extension.columnar.rewrite.RewriteSingleNode import org.apache.gluten.extension.columnar.validator.Validator import org.apache.gluten.ras.path.Pattern @@ -93,16 +93,19 @@ object RasOffload { // 3. Validate current node. If passed, offload it. validator.validate(from) match { case Validator.Passed => - val offloaded = base.offload(from) - val offloadedNodes = offloaded.collect[GlutenPlan] { case t: GlutenPlan => t } - if (offloadedNodes.exists(!_.doValidate().ok())) { + val offloadedPlan = base.offload(from) + val offloadedNodes = offloadedPlan.collect[GlutenPlan] { case t: GlutenPlan => t } + val outComes = offloadedNodes.map(_.doValidate()).filter(!_.ok()) + if (outComes.nonEmpty) { // 4. If native validation fails on the offloaded node, return the // original one. + outComes.foreach(FallbackTags.add(from, _)) from } else { - offloaded + offloadedPlan } case Validator.Failed(reason) => + FallbackTags.add(from, reason) from } } diff --git a/gluten-ut/test/src/test/scala/org/apache/gluten/expressions/GlutenExpressionMappingSuite.scala b/gluten-ut/test/src/test/scala/org/apache/gluten/expressions/GlutenExpressionMappingSuite.scala index 3b52721fe6a1..68b9ab1a3697 100644 --- a/gluten-ut/test/src/test/scala/org/apache/gluten/expressions/GlutenExpressionMappingSuite.scala +++ b/gluten-ut/test/src/test/scala/org/apache/gluten/expressions/GlutenExpressionMappingSuite.scala @@ -104,9 +104,9 @@ class GlutenExpressionMappingSuite sql("create table t2 (b decimal(10,4)) using parquet") val msg = - "CheckOverflowInTableInsert is used in ansi mode, but gluten does not support ANSI mode." + "CheckOverflowInTableInsert is used in ANSI mode, but Gluten does not support ANSI mode." import org.apache.spark.sql.execution.GlutenImplicits._ - val fallbackSummary = sql("insert overwrite t2 select * from t1").fallbackSummary + val fallbackSummary = sql("insert overwrite t2 select * from t1").fallbackSummary() assert(fallbackSummary.fallbackNodeToReason.flatMap(_.values).exists(_.contains(msg))) } }