From f230e56d7d71ee66097aad27fdeabeb6d0dde6a1 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 8 Dec 2025 11:57:10 -0700 Subject: [PATCH 1/6] make code less confusing --- .../apache/comet/rules/CometExecRule.scala | 46 ++++++++----------- 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala b/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala index 89e38e5d7b..43ad55f27d 100644 --- a/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala +++ b/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala @@ -191,13 +191,13 @@ case class CometExecRule(session: SparkSession) extends Rule[SparkPlan] { // For AQE broadcast stage on a Comet broadcast exchange case s @ BroadcastQueryStageExec(_, _: CometBroadcastExchangeExec, _) => - convertToCometIfAllChildrenAreNative(s, CometExchangeSink).getOrElse(s) + convertToComet(s, CometExchangeSink).getOrElse(s) case s @ BroadcastQueryStageExec( _, ReusedExchangeExec(_, _: CometBroadcastExchangeExec), _) => - convertToCometIfAllChildrenAreNative(s, CometExchangeSink).getOrElse(s) + convertToComet(s, CometExchangeSink).getOrElse(s) // `CometBroadcastExchangeExec`'s broadcast output is not compatible with Spark's broadcast // exchange. It is only used for Comet native execution. We only transform Spark broadcast @@ -205,8 +205,8 @@ case class CometExecRule(session: SparkSession) extends Rule[SparkPlan] { // broadcast exchange is forced to be enabled by Comet config. case plan if plan.children.exists(_.isInstanceOf[BroadcastExchangeExec]) => val newChildren = plan.children.map { - case b: BroadcastExchangeExec => - convertToCometIfAllChildrenAreNative(b, CometBroadcastExchangeExec).getOrElse(b) + case b: BroadcastExchangeExec if b.children.forall(_.isInstanceOf[CometNativeExec]) => + convertToComet(b, CometBroadcastExchangeExec).getOrElse(b) case other => other } if (!newChildren.exists(_.isInstanceOf[BroadcastExchangeExec])) { @@ -227,25 +227,29 @@ case class CometExecRule(session: SparkSession) extends Rule[SparkPlan] { // For AQE shuffle stage on a Comet shuffle exchange case s @ ShuffleQueryStageExec(_, _: CometShuffleExchangeExec, _) => - convertToCometIfAllChildrenAreNative(s, CometExchangeSink).getOrElse(s) + convertToComet(s, CometExchangeSink).getOrElse(s) // For AQE shuffle stage on a reused Comet shuffle exchange // Note that we don't need to handle `ReusedExchangeExec` for non-AQE case, because // the query plan won't be re-optimized/planned in non-AQE mode. case s @ ShuffleQueryStageExec(_, ReusedExchangeExec(_, _: CometShuffleExchangeExec), _) => - convertToCometIfAllChildrenAreNative(s, CometExchangeSink).getOrElse(s) + convertToComet(s, CometExchangeSink).getOrElse(s) case s: ShuffleExchangeExec => convertToComet(s, CometShuffleExchangeExec).getOrElse(s) case op => - val handler = allExecs - .get(op.getClass) - .map(_.asInstanceOf[CometOperatorSerde[SparkPlan]]) - handler match { - case Some(handler) => - return convertToCometIfAllChildrenAreNative(op, handler).getOrElse(op) - case _ => + // if all children are native (or if this is a leaf node) then see if there is a + // registered handler for creating a fully native plan + if (op.children.forall(_.isInstanceOf[CometNativeExec])) { + val handler = allExecs + .get(op.getClass) + .map(_.asInstanceOf[CometOperatorSerde[SparkPlan]]) + handler match { + case Some(handler) => + return convertToComet(op, handler).getOrElse(op) + case _ => + } } op match { @@ -449,26 +453,12 @@ case class CometExecRule(session: SparkSession) extends Rule[SparkPlan] { } } - /** - * Convert a Spark plan to a Comet plan using the specified serde handler, but only if all - * children are native. - */ - private def convertToCometIfAllChildrenAreNative( - op: SparkPlan, - handler: CometOperatorSerde[_]): Option[SparkPlan] = { - if (op.children.forall(_.isInstanceOf[CometNativeExec])) { - convertToComet(op, handler) - } else { - None - } - } - /** Convert a Spark plan to a Comet plan using the specified serde handler */ private def convertToComet(op: SparkPlan, handler: CometOperatorSerde[_]): Option[SparkPlan] = { val serde = handler.asInstanceOf[CometOperatorSerde[SparkPlan]] if (isOperatorEnabled(serde, op)) { val builder = OperatorOuterClass.Operator.newBuilder().setPlanId(op.id) - if (op.children.forall(_.isInstanceOf[CometNativeExec])) { + if (op.children.nonEmpty && op.children.forall(_.isInstanceOf[CometNativeExec])) { val childOp = op.children.map(_.asInstanceOf[CometNativeExec].nativeOp) childOp.foreach(builder.addChildren) return serde From 9ea53582699adfd2fc0a28197ce38e50135f5e37 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 8 Dec 2025 13:12:07 -0700 Subject: [PATCH 2/6] fix regression --- .../main/scala/org/apache/comet/rules/CometExecRule.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala b/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala index 43ad55f27d..4625ac642a 100644 --- a/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala +++ b/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala @@ -262,11 +262,12 @@ case class CometExecRule(session: SparkSession) extends Rule[SparkPlan] { // Some execs that comet will not accelerate, such as command execs. op case _ => - if (!hasExplainInfo(op)) { + if (op.children.forall(_.isInstanceOf[CometNativeExec]) && !hasExplainInfo(op)) { // An operator that is not supported by Comet withInfo(op, s"${op.nodeName} is not supported") } else { - // Already has fallback reason, do not override it + // Already has fallback reason, or previous operator already fell back to + // Spark, so do not override it op } } From 5245166da0af436cfd940665d3f9ea259d62a408 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 8 Dec 2025 16:02:27 -0700 Subject: [PATCH 3/6] golden files 3.5 --- .../q28.native_iceberg_compat/extended.txt | 8 ++++---- .../approved-plans-v1_4-spark3_5/q28/extended.txt | 8 ++++---- .../q88.native_iceberg_compat/extended.txt | 12 ++++++------ .../approved-plans-v1_4-spark3_5/q88/extended.txt | 12 ++++++------ 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q28.native_iceberg_compat/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q28.native_iceberg_compat/extended.txt index d2c0985d1f..809265b4d5 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q28.native_iceberg_compat/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q28.native_iceberg_compat/extended.txt @@ -1,7 +1,7 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : :- CometColumnarToRow : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q28/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q28/extended.txt index d2c0985d1f..809265b4d5 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q28/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q28/extended.txt @@ -1,7 +1,7 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : :- CometColumnarToRow : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q88.native_iceberg_compat/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q88.native_iceberg_compat/extended.txt index caaa611f9e..1e1247665c 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q88.native_iceberg_compat/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q88.native_iceberg_compat/extended.txt @@ -1,9 +1,9 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin +: : : :- BroadcastNestedLoopJoin +: : : : :- BroadcastNestedLoopJoin : : : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : : : :- CometColumnarToRow : : : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q88/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q88/extended.txt index caaa611f9e..1e1247665c 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q88/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q88/extended.txt @@ -1,9 +1,9 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin +: : : :- BroadcastNestedLoopJoin +: : : : :- BroadcastNestedLoopJoin : : : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : : : :- CometColumnarToRow : : : : : : : +- CometHashAggregate From 57b9035f6d6b4cc096e2132a0a05f1b86bf124e3 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 8 Dec 2025 16:06:51 -0700 Subject: [PATCH 4/6] golden files 3.4 --- .../q28.native_iceberg_compat/extended.txt | 8 ++++---- .../approved-plans-v1_4/q28/extended.txt | 8 ++++---- .../q88.native_iceberg_compat/extended.txt | 12 ++++++------ .../approved-plans-v1_4/q88/extended.txt | 12 ++++++------ 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q28.native_iceberg_compat/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q28.native_iceberg_compat/extended.txt index d2c0985d1f..809265b4d5 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q28.native_iceberg_compat/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q28.native_iceberg_compat/extended.txt @@ -1,7 +1,7 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : :- CometColumnarToRow : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q28/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q28/extended.txt index d2c0985d1f..809265b4d5 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q28/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q28/extended.txt @@ -1,7 +1,7 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : :- CometColumnarToRow : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q88.native_iceberg_compat/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q88.native_iceberg_compat/extended.txt index caaa611f9e..1e1247665c 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q88.native_iceberg_compat/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q88.native_iceberg_compat/extended.txt @@ -1,9 +1,9 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin +: : : :- BroadcastNestedLoopJoin +: : : : :- BroadcastNestedLoopJoin : : : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : : : :- CometColumnarToRow : : : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q88/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q88/extended.txt index caaa611f9e..1e1247665c 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q88/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q88/extended.txt @@ -1,9 +1,9 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin +: : : :- BroadcastNestedLoopJoin +: : : : :- BroadcastNestedLoopJoin : : : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : : : :- CometColumnarToRow : : : : : : : +- CometHashAggregate From c5dd0f8f8755da67d2ad89814d047913fdbf5b71 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 8 Dec 2025 16:12:41 -0700 Subject: [PATCH 5/6] golden files 4.0 --- .../q28.native_iceberg_compat/extended.txt | 8 ++++---- .../approved-plans-v1_4-spark4_0/q28/extended.txt | 8 ++++---- .../q88.native_iceberg_compat/extended.txt | 12 ++++++------ .../approved-plans-v1_4-spark4_0/q88/extended.txt | 12 ++++++------ 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q28.native_iceberg_compat/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q28.native_iceberg_compat/extended.txt index d2c0985d1f..809265b4d5 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q28.native_iceberg_compat/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q28.native_iceberg_compat/extended.txt @@ -1,7 +1,7 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : :- CometColumnarToRow : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q28/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q28/extended.txt index d2c0985d1f..809265b4d5 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q28/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q28/extended.txt @@ -1,7 +1,7 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : :- CometColumnarToRow : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q88.native_iceberg_compat/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q88.native_iceberg_compat/extended.txt index caaa611f9e..1e1247665c 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q88.native_iceberg_compat/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q88.native_iceberg_compat/extended.txt @@ -1,9 +1,9 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin +: : : :- BroadcastNestedLoopJoin +: : : : :- BroadcastNestedLoopJoin : : : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : : : :- CometColumnarToRow : : : : : : : +- CometHashAggregate diff --git a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q88/extended.txt b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q88/extended.txt index caaa611f9e..1e1247665c 100644 --- a/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q88/extended.txt +++ b/spark/src/test/resources/tpcds-plan-stability/approved-plans-v1_4-spark4_0/q88/extended.txt @@ -1,9 +1,9 @@ - BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -:- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] -: : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] +BroadcastNestedLoopJoin +:- BroadcastNestedLoopJoin +: :- BroadcastNestedLoopJoin +: : :- BroadcastNestedLoopJoin +: : : :- BroadcastNestedLoopJoin +: : : : :- BroadcastNestedLoopJoin : : : : : :- BroadcastNestedLoopJoin [COMET: BroadcastNestedLoopJoin is not supported] : : : : : : :- CometColumnarToRow : : : : : : : +- CometHashAggregate From 4bfea30c8fe5be9a43062e34a071d1a26e261959 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 8 Dec 2025 21:32:54 -0700 Subject: [PATCH 6/6] address feedback --- .../org/apache/comet/rules/CometExecRule.scala | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala b/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala index 4625ac642a..f490202537 100644 --- a/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala +++ b/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala @@ -254,20 +254,22 @@ case class CometExecRule(session: SparkSession) extends Rule[SparkPlan] { op match { case _: CometPlan | _: AQEShuffleReadExec | _: BroadcastExchangeExec | - _: BroadcastQueryStageExec | _: AdaptiveSparkPlanExec => + _: BroadcastQueryStageExec | _: AdaptiveSparkPlanExec | _: ExecutedCommandExec | + _: V2CommandExec => // Some execs should never be replaced. We include // these cases specially here so we do not add a misleading 'info' message op - case _: ExecutedCommandExec | _: V2CommandExec => - // Some execs that comet will not accelerate, such as command execs. - op case _ => + // The operator was not converted to a Comet plan. Possible reasons for this happening: + // 1. Comet does not support this operator. + // 2. The operator could not be supported based on query context and current + // configs. In this case, it should have already been tagged with fallback + // reasons. + // 3. The operator has children that could not be converted, so execution + // has already fallen back to Spark. if (op.children.forall(_.isInstanceOf[CometNativeExec]) && !hasExplainInfo(op)) { - // An operator that is not supported by Comet withInfo(op, s"${op.nodeName} is not supported") } else { - // Already has fallback reason, or previous operator already fell back to - // Spark, so do not override it op } }