xiangfu0 commented on code in PR #19991:
URL: https://github.com/apache/pinot/pull/19991#discussion_r4381384179
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/ModeAggregationFunction.java:
##########
@@ -495,18 +587,22 @@ public SerializedIntermediateResult
serializeIntermediateResult(Map<? extends Nu
}
@Override
- public Map<? extends Number, Long>
deserializeIntermediateResult(CustomObject customObject) {
+ public Map<?, Long> deserializeIntermediateResult(CustomObject customObject)
{
return ObjectSerDeUtils.deserialize(customObject);
}
@Override
public ColumnDataType getFinalResultColumnType() {
- return ColumnDataType.DOUBLE;
+ return _resultType;
}
@Nullable
@Override
- public Double extractFinalResult(@Nullable Map<? extends Number, Long>
intermediateResult) {
+ public Comparable<?> extractFinalResult(@Nullable Map<?, Long>
intermediateResult) {
+ if (_resultType != ColumnDataType.DOUBLE) {
+ return intermediateResult == null || intermediateResult.isEmpty() ? null
Review Comment:
Made the comparator in `TableResizer` null-safe when an ORDER BY expression
is an aggregation (group keys and literals keep the fast path), and added
`ModeQueriesTest#testNonNumericOrderByEmptyGroup` with your query for ASC and
DESC, in 9bb1bf3b82.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-query-runtime/src/test/resources/queries/UDFAggregates.json:
##########
@@ -108,6 +127,28 @@
"sql": "SELECT /*+ aggOptions(is_skip_leaf_stage_aggregate='true') */
string_col, FIRST_WITH_TIME(decimal_col, timestamp_col),
LAST_WITH_TIME(decimal_col, timestamp_col), FIRST_WITH_TIME(bytes_col,
timestamp_col), LAST_WITH_TIME(bytes_col, timestamp_col) FROM {tbl} GROUP BY
string_col",
"outputs": [["a", "1E-307", "1E+307", "deadbeef", "fdeadbee"], ["b",
"1E-308", "1E+308", "efdeadbe", "beefdead"], ["c", "1E+328", "1E-317",
"deadbe02", "adbeefde"]]
},
+ {
+ "comment": "MODE returns the type of a non-numeric argument, with ties
broken by the smallest value",
+ "sql": "SELECT MODE(string_col), MODE(UPPER(string_col)),
MODE(bool_col), MODE(timestamp_col), MODE(int_col) FROM {tbl}",
+ "outputs": [["c", "C", false, "2020-01-01 03:10:12.0", 2]]
+ },
+ {
+ "sql": "SELECT bool_col, MODE(string_col), MODE(timestamp_col) FROM
{tbl} GROUP BY bool_col",
+ "outputs": [[false, "b", "2020-01-01 08:32:12.0"], [true, "a",
"2020-01-01 03:10:12.0"]]
+ },
+ {
+ "sql": "SELECT /*+ aggOptions(is_skip_leaf_stage_aggregate='true') */
int_col, MODE(string_col), MODE(bool_col), MODE(timestamp_col) FROM {tbl} GROUP
BY int_col",
Review Comment:
Switched the MODE case to `is_skip_leaf_stage_group_by='true'` in 9bb1bf3b82.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
