From e52efb0fcc07efaec21c1184545e01d58bbc7489 Mon Sep 17 00:00:00 2001 From: Mryange Date: Tue, 15 Sep 2026 15:01:20 +0800 Subject: [PATCH] [fix](agg) Align complex aggregate null ordering (#67439) Problem Summary: Complex-type `min`, `max`, `min_by`, and `max_by` used a NULL ordering direction different from ordinary SQL array comparisons. This could select inconsistent results for arrays containing NULL elements. The aggregate comparison direction is aligned with SQL array ordering, and regression expectations are updated. https://github.com/apache/doris-website/pull/4121 ### Release note Complex-type aggregate ordering now follows SQL array NULL ordering. ### Check List (For Author) - Test - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason - Behavior changed: - [ ] No. - [ ] Yes. - Does this need documentation? - [ ] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label --- .../exprs/aggregate/aggregate_function_min_max.h | 10 +++++----- .../test_aggregate_all_functions2.out | 14 ++++++++------ .../test_aggregate_all_functions2.groovy | 1 + 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/be/src/exprs/aggregate/aggregate_function_min_max.h b/be/src/exprs/aggregate/aggregate_function_min_max.h index 04cee942d8aff7..1b024251f54321 100644 --- a/be/src/exprs/aggregate/aggregate_function_min_max.h +++ b/be/src/exprs/aggregate/aggregate_function_min_max.h @@ -596,7 +596,7 @@ struct SingleValueDataComplexType { } bool change_if_less(const IColumn& column, size_t row_num, Arena& arena) { - if (!has() || column_data->compare_at(0, row_num, column, -1) == 1) { + if (!has() || column_data->compare_at(0, row_num, column, 1) == 1) { change(column, row_num, arena); return true; } else { @@ -605,7 +605,7 @@ struct SingleValueDataComplexType { } bool change_if_less(const Self& to, Arena& arena) { - if (to.has() && (!has() || column_data->compare_at(0, 0, *to.column_data, -1) == 1)) { + if (to.has() && (!has() || column_data->compare_at(0, 0, *to.column_data, 1) == 1)) { change(to, arena); return true; } else { @@ -614,7 +614,7 @@ struct SingleValueDataComplexType { } bool change_if_greater(const IColumn& column, size_t row_num, Arena& arena) { - if (!has() || column_data->compare_at(0, row_num, column, -1) == -1) { + if (!has() || column_data->compare_at(0, row_num, column, 1) == -1) { change(column, row_num, arena); return true; } else { @@ -623,7 +623,7 @@ struct SingleValueDataComplexType { } bool change_if_greater(const Self& to, Arena& arena) { - if (to.has() && (!has() || column_data->compare_at(0, 0, *to.column_data, -1) == -1)) { + if (to.has() && (!has() || column_data->compare_at(0, 0, *to.column_data, 1) == -1)) { change(to, arena); return true; } else { @@ -640,7 +640,7 @@ struct SingleValueDataComplexType { type == TYPE_AGG_STATE) { return false; } else { - return !column_data->compare_at(0, row_num, column, -1); + return !column_data->compare_at(0, row_num, column, 1); } } diff --git a/regression-test/data/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.out b/regression-test/data/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.out index 41ca9183b9c002..ae45f567d79c4e 100644 --- a/regression-test/data/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.out +++ b/regression-test/data/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.out @@ -389,16 +389,19 @@ true [1, 2, 5] [1, 2] [11, 22, 33, 44] [3, 1] [10] [] -[11, null, null, 55] [1, null, null, 4] +[11, null, null, 55] [1, 2, 3, 4] -- !maxminby_array_2 -- [1, 2, 5] [11, 22, 33, 44] -- !maxminby_array_3 -- -{"A":10, "B":1} {"x":50, "y":60} +{"A":10, "B":5} {"x":50, "y":60} -- !maxminby_array_4 -- -{"a":10, "b":"tt"} {"a":4, "b":"delta"} +{"a":10, "b":"ten"} {"a":4, "b":"delta"} + +-- !maxminby_array_null_order -- +[11, null, null, 55] [1, 2, 3, 4] -- !maxminby_array_5 -- 1 [5, 6] [7] @@ -422,7 +425,7 @@ true 2 {"foo":1, "bar":2} {"foo":2, "bar":1} 3 {"A":10, "B":1} {"key1":99, "key2":98} 4 {"A":5, "B":10} {"x":50, "y":60} -5 {"A":10, "B":5} {"A":null, "B":null} +5 {"A":10, "B":5} {"A":null, "B":5} -- !maxminby_struct_2 -- {"a":5, "b":"echo"} {"a":10, "b":"tt"} @@ -439,7 +442,7 @@ true 2 {"a":5, "b":"echo"} {"a":6, "b":"zulu"} 3 {"a":10, "b":"tt"} {"a":8, "b":"eight"} 4 {"a":9, "b":"nine"} {"a":4, "b":"delta"} -5 {"a":10, "b":"ten"} {"a":null, "b":null} +5 {"a":10, "b":"ten"} {"a":null, "b":"ten"} -- !maxmin_array_3 -- [[3, 4], [3, 4]] [[1, 2], [3, 4]] @@ -449,4 +452,3 @@ true -- !maxminby_array_7 -- [[3, 4], [3, 4]] [[1, 2], [3, 4]] - diff --git a/regression-test/suites/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.groovy b/regression-test/suites/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.groovy index d150b976d03a7b..c450af3c1c6677 100644 --- a/regression-test/suites/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.groovy +++ b/regression-test/suites/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.groovy @@ -317,6 +317,7 @@ suite("test_aggregate_all_functions2") { qt_maxminby_array_2 """SELECT max_by(arr, weight), min_by(arr, weight) from test_maxmin""" qt_maxminby_array_3 """SELECT max_by(mp, arr), min_by(mp, arr) from test_maxmin""" qt_maxminby_array_4 """SELECT max_by(st, arr), min_by(st, arr) from test_maxmin""" + qt_maxminby_array_null_order """SELECT max_by(arr, arr), min_by(arr, arr) from test_maxmin where id = 5""" qt_maxminby_array_5 """SELECT id, max_by(arr, weight), min_by(arr, weight) from test_maxmin group by id order by id"""