Copilot commented on code in PR #12294:
URL: https://github.com/apache/gluten/pull/12294#discussion_r3410029856
##########
backends-clickhouse/src/test/scala/org/apache/gluten/execution/GlutenClickhouseCountDistinctSuite.scala:
##########
@@ -103,9 +103,8 @@ class GlutenClickhouseCountDistinctSuite extends
GlutenClickHouseWholeStageTrans
// skewness agg is not supported, will cause fallback
val sql = "select count(distinct(a,b)) , skewness(b) from " +
"values (0, null,1), (0,null,1), (1, 1,1), (2, 2, 1) ,(2,2,2),(3,3,3) as
data(a,b,c)"
- assertThrows[UnsupportedOperationException] {
- spark.sql(sql).show
- }
+ compareResultsAgainstVanillaSpark(sql, true, { _ => })
Review Comment:
This change weakens the assertion: it no longer verifies the previous
behavior (throw) nor does it assert the new intended behavior (native support
vs fallback). If `skewness` is now supported, the test should assert no
fallback (and ideally validate the executed plan contains the expected
native/CH operator). If it’s still expected to fallback, assert that fallback
actually occurred rather than passing an empty callback.
##########
cpp-ch/local-engine/Parser/aggregate_function_parser/SimpleStatisticsFunctions.cpp:
##########
@@ -63,7 +63,14 @@ class AggregateFunctionParserStddev final : public
AggregateFunctionParser
return func_node;
}
};
+// for skewness
+struct SkewnessNameStruct
+{
+ static constexpr auto spark_name = "skewness";
+ static constexpr auto ch_name = "skewSamp";
+};
+static const
AggregateFunctionParserRegister<AggregateFunctionParserStddev<SkewnessNameStruct>>
registerer_skewness;
Review Comment:
`AggregateFunctionParserStddev<SkewnessNameStruct>` is conceptually
confusing (a stddev parser registering skewness). Even if it works, it makes
the code harder to understand/extend. Consider introducing a more generic
parser type/name (or a thin `AggregateFunctionParserSkewness` alias/wrapper) so
future readers don’t have to infer that the stddev parser is generic enough for
skewness.
--
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]
