kazuyukitanimura commented on code in PR #3454:
URL: https://github.com/apache/datafusion-comet/pull/3454#discussion_r2934127830
##########
dev/diffs/4.0.1.diff:
##########
@@ -1144,8 +1167,23 @@ index c1c041509c3..7d463e4b85e 100644
.withExtensions { extensions =>
extensions.injectColumnar(session =>
MyColumnarRule(PreRuleReplaceAddWithBrokenVersion(), MyPostRule()))
}
+diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/SparkSessionJobTaggingAndCancellationSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/SparkSessionJobTaggingAndCancellationSuite.scala
+index 5ba69c8f9d9..ac1256afe88 100644
+---
a/sql/core/src/test/scala/org/apache/spark/sql/SparkSessionJobTaggingAndCancellationSuite.scala
++++
b/sql/core/src/test/scala/org/apache/spark/sql/SparkSessionJobTaggingAndCancellationSuite.scala
+@@ -82,6 +82,10 @@ class SparkSessionJobTaggingAndCancellationSuite
+ }
+
+ test("Tags set from session are prefixed with session UUID") {
++ // This test relies on job scheduling order which is timing-dependent and
becomes unreliable
++ // when Comet is enabled due to changes in async execution behaviour.
++ assume(!classic.SparkSession.isCometEnabled,
++ "Skipped when Comet is enabled: test results are timing-dependent")
Review Comment:
Not a blocker but we use
```
IgnoreComet
```
For skipping tsets
##########
dev/diffs/4.0.1.diff:
##########
@@ -354,33 +394,27 @@ index 0f42502f1d9..f616024a9c2 100644
}
test("A cached table preserves the partitioning and ordering of its cached
SparkPlan") {
-@@ -1626,7 +1627,9 @@ class CachedTableSuite extends QueryTest with
SQLTestUtils
- }
- }
-
-- test("SPARK-35332: Make cache plan disable configs configurable - check
AQE") {
-+ test("SPARK-35332: Make cache plan disable configs configurable - check
AQE",
-+ IgnoreComet("TODO: ignore for first stage of 4.0 " +
-+ "https://github.com/apache/datafusion-comet/issues/1948";)) {
- withSQLConf(SQLConf.SHUFFLE_PARTITIONS.key -> "2",
- SQLConf.COALESCE_PARTITIONS_MIN_PARTITION_NUM.key -> "1",
- SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "true") {
-@@ -1661,7 +1664,12 @@ class CachedTableSuite extends QueryTest with
SQLTestUtils
+@@ -1659,9 +1660,15 @@ class CachedTableSuite extends QueryTest with
SQLTestUtils
+ _.nodeName.contains("TableCacheQueryStage"))
+ val aqeNode = findNodeInSparkPlanInfo(inMemoryScanNode.get,
_.nodeName.contains("AdaptiveSparkPlan"))
- val aqePlanRoot = findNodeInSparkPlanInfo(inMemoryScanNode.get,
- _.nodeName.contains("ResultQueryStage"))
+- val aqePlanRoot = findNodeInSparkPlanInfo(inMemoryScanNode.get,
+- _.nodeName.contains("ResultQueryStage"))
- aqePlanRoot.get.children.head.nodeName == "AQEShuffleRead"
-+ aqeNode.get.children.head.nodeName == "AQEShuffleRead" ||
-+ (aqeNode.get.children.head.nodeName.contains("WholeStageCodegen") &&
-+ aqeNode.get.children.head.children.head.nodeName ==
"ColumnarToRow" &&
-+ aqeNode.get.children.head.children.head.children.head.nodeName ==
"InputAdapter" &&
-+
aqeNode.get.children.head.children.head.children.head.children.head.nodeName ==
-+ "AQEShuffleRead")
++ // Spark 4.0 wraps results in ResultQueryStage. The coalescing
indicator is AQEShuffleRead
++ // as the direct child of InputAdapter.
++ // AdaptiveSparkPlan -> ResultQueryStage -> WholestageCodegen ->
++ // CometColumnarToRow -> InputAdapter -> AQEShuffleRead (if
coalesced)
++ val resultStage = aqeNode.get.children.head // ResultQueryStage
++ val wsc = resultStage.children.head // WholeStageCodegen
++ val c2r = wsc.children.head // ColumnarToRow or
CometColumnarToRow
++ val inputAdapter = c2r.children.head // InputAdapter
++ inputAdapter.children.head.nodeName == "AQEShuffleRead"
Review Comment:
This new test is not equivalent because some of the nodeName tests are
skipped.
Should the change be something like
```
...
(aqeNode.get.children.head.children.head.nodeName == "ColumnarToRow" ||
aqeNode.get.children.head.children.head.nodeName == "CometColumnarToRow") &&
...
```
--
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]
