philo-he commented on code in PR #12151:
URL: https://github.com/apache/gluten/pull/12151#discussion_r3449978782
##########
gluten-ut/spark40/src/test/scala/org/apache/spark/sql/GlutenBloomFilterAggregateQuerySuite.scala:
##########
@@ -24,6 +24,16 @@ import org.apache.spark.SparkConf
import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper
import org.apache.spark.sql.internal.SQLConf
+import org.scalatest.Tag
+
+/**
+ * ScalaTest tag for the issue-12013 regression test. Run with:
+ * {{{
+ * --test-tags=org.apache.gluten.tags.Issue12013
+ * }}}
+ */
+object Issue12013 extends Tag("org.apache.gluten.tags.Issue12013")
Review Comment:
Could we drop the Issue12013 tag? Tags are usually meant for broader,
reusable categories with descriptive names.
##########
gluten-ut/spark40/src/test/scala/org/apache/spark/sql/GlutenBloomFilterAggregateQuerySuite.scala:
##########
@@ -112,6 +122,109 @@ class GlutenBloomFilterAggregateQuerySuite
}
}
+ // Regression test for https://github.com/apache/gluten/issues/12013
+ // When ExpandFallbackPolicy triggers a whole-stage AQE fallback, the
resulting plan comes
+ // from the originalPlan snapshot captured before optimizer rules ran. Moving
+ // BloomFilterMightContainJointRewriteRule to injectOptimizerRule ensures
both the
+ // BloomFilterAggregate->VeloxBloomFilterAggregate and
BloomFilterMightContain->
+ // VeloxBloomFilterMightContain substitutions are baked into originalPlan,
so the byte
+ // format stays consistent regardless of which stages fall back.
+ testGluten(
+ "Test bloom_filter_agg whole-stage fallback does not corrupt bloom filter
bytes",
Review Comment:
Could we name the test "GLUTEN-12013: xxx"? It follows the Spark convention
(already used in some Gluten tests) and seems to be better than using a tag.
##########
gluten-ut/spark40/src/test/scala/org/apache/spark/sql/GlutenBloomFilterAggregateQuerySuite.scala:
##########
@@ -112,6 +122,109 @@ class GlutenBloomFilterAggregateQuerySuite
}
}
+ // Regression test for https://github.com/apache/gluten/issues/12013
+ // When ExpandFallbackPolicy triggers a whole-stage AQE fallback, the
resulting plan comes
+ // from the originalPlan snapshot captured before optimizer rules ran. Moving
+ // BloomFilterMightContainJointRewriteRule to injectOptimizerRule ensures
both the
+ // BloomFilterAggregate->VeloxBloomFilterAggregate and
BloomFilterMightContain->
+ // VeloxBloomFilterMightContain substitutions are baked into originalPlan,
so the byte
+ // format stays consistent regardless of which stages fall back.
+ testGluten(
+ "Test bloom_filter_agg whole-stage fallback does not corrupt bloom filter
bytes",
Review Comment:
Could we also move this into `gluten-ut/test`, perhaps as a new suite? The
`gluten-ut/spark40` folders generally hold only imported Spark tests and their
variants. If it's Spark 4.0+ only, we can use `testWithMinSparkVersion`.
--
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]
