Copilot commented on code in PR #12242:
URL: https://github.com/apache/gluten/pull/12242#discussion_r3357608267
##########
gluten-ut/spark41/src/test/scala/org/apache/spark/sql/GlutenBitmapExpressionsQuerySuite.scala:
##########
@@ -37,4 +37,19 @@ class GlutenBitmapExpressionsQuerySuite
"Expected native HashAggregateExecBaseTransformer in plan"
)
}
+
+ test("bitmap_or_agg routes to native") {
+ val df = spark.sql(
+ "SELECT bitmap_or_agg(bm) FROM (" +
+ "SELECT bitmap_construct_agg(bitmap_bit_position(col)) AS bm " +
+ "FROM values (1L), (2L), (3L) AS t(col)" +
+ ") sub")
+ df.collect()
+ assert(
+ collectWithSubqueries(df.queryExecution.executedPlan) {
+ case h: HashAggregateExecBaseTransformer => h
+ }.nonEmpty,
+ "Expected native HashAggregateExecBaseTransformer in plan"
+ )
+ }
Review Comment:
This test can pass even if `bitmap_or_agg` is *not* routed to native,
because the inner `bitmap_construct_agg` is also an aggregate and may produce a
`HashAggregateExecBaseTransformer` on its own. To make the assertion
specifically validate `bitmap_or_agg`, restructure the query to avoid an inner
aggregate (e.g., aggregate directly over per-row bitmap inputs if supported),
or assert that the transformer node you match contains the `bitmap_or_agg`
aggregate expression (not just that *some* native hash aggregate exists in the
plan).
##########
gluten-ut/spark40/src/test/scala/org/apache/spark/sql/GlutenBitmapExpressionsQuerySuite.scala:
##########
@@ -37,4 +37,19 @@ class GlutenBitmapExpressionsQuerySuite
"Expected native HashAggregateExecBaseTransformer in plan"
)
}
+
+ test("bitmap_or_agg routes to native") {
+ val df = spark.sql(
+ "SELECT bitmap_or_agg(bm) FROM (" +
+ "SELECT bitmap_construct_agg(bitmap_bit_position(col)) AS bm " +
+ "FROM values (1L), (2L), (3L) AS t(col)" +
+ ") sub")
+ df.collect()
+ assert(
+ collectWithSubqueries(df.queryExecution.executedPlan) {
+ case h: HashAggregateExecBaseTransformer => h
+ }.nonEmpty,
+ "Expected native HashAggregateExecBaseTransformer in plan"
+ )
+ }
Review Comment:
Same issue as Spark 4.1 UT: the matched native aggregate could come from the
inner `bitmap_construct_agg`, so the test doesn't strictly prove
`bitmap_or_agg` is routed to native. Consider removing the inner aggregate or
asserting on the matched node's aggregate expressions to ensure `bitmap_or_agg`
is what triggered native planning.
##########
gluten-ut/spark41/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -91,6 +91,7 @@ class ClickHouseTestSettings extends BackendTestSettings {
enableSuite[GlutenBitmapExpressionsQuerySuite]
// bitmap_construct_agg is not supported natively in CH backend.
.excludeCH("bitmap_construct_agg routes to native")
+ .excludeCH("bitmap_or_agg routes to native")
Review Comment:
The comment only mentions `bitmap_construct_agg`, but this block now
excludes `bitmap_or_agg` as well. Update the comment to reflect both exclusions
so future readers understand why multiple bitmap tests are disabled for CH.
##########
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenBitmapExpressionsQuerySuite.scala:
##########
@@ -37,4 +37,19 @@ class GlutenBitmapExpressionsQuerySuite
"Expected native HashAggregateExecBaseTransformer in plan"
)
}
+
+ test("bitmap_or_agg routes to native") {
+ val df = spark.sql(
+ "SELECT bitmap_or_agg(bm) FROM (" +
+ "SELECT bitmap_construct_agg(bitmap_bit_position(col)) AS bm " +
+ "FROM values (1L), (2L), (3L) AS t(col)" +
+ ") sub")
+ df.collect()
+ assert(
+ collectWithSubqueries(df.queryExecution.executedPlan) {
+ case h: HashAggregateExecBaseTransformer => h
+ }.nonEmpty,
+ "Expected native HashAggregateExecBaseTransformer in plan"
+ )
+ }
Review Comment:
Same issue as the Spark 4.0/4.1 UT: because the query contains two
aggregates, `.nonEmpty` over `HashAggregateExecBaseTransformer` can be
satisfied by the inner aggregate alone. Update the query/assertion so it
specifically verifies the `bitmap_or_agg` aggregate is planned as native.
--
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]
