All committers have signed the CLA.

Detailed:
Added a new aggregation function:

Usage 1:
groupSortedArray(n)(param1)

It returns an array with the n first values from a field param1 sorted by itself.

 select groupSortedArray(2)(number) from numbers(100)
┌─groupSortedArray(2)(number)─┐
│ [0,1]                       │
└─────────────────────────────┘

Usage 2:
groupSortedArray(n)(param1, param2)

It returns an array with the n first values from a field param1 sorted param2 (field or expression).

SELECT groupSortedArray(5)(number, number) FROM numbers(1000000)

SELECT groupSortedArray(5)(number, -number) FROM numbers(100)
┌─groupSortedArray(5)(number, negate(number))─┐
│ [99,98,97,96,95]                            │
└─────────────────────────────────────────────┘

What if we want to sort by multiple columns?

SELECT groupSortedArray(5)(number, -number, number % 10) FROM numbers(100)

SELECT groupSortedArray(5)(number, (-number, number % 10)) FROM numbers(100)

I like first approach more, because sorting of tuple much slower.

What if we want to sort by multiple columns?

SELECT groupSortedArray(5)(number, -number, number % 10) FROM numbers(100)

SELECT groupSortedArray(5)(number, (-number, number % 10)) FROM numbers(100)

I like first approach more, because sorting of tuple much slower.

It is not supported. Initially it only accepts integer expressions for this second parameter.

What if we want to sort by multiple columns?

SELECT groupSortedArray(5)(number, -number, number % 10) FROM numbers(100)

SELECT groupSortedArray(5)(number, (-number, number % 10)) FROM numbers(100)

I like first approach more, because sorting of tuple much slower.

It is not supported. Initially it only accepts integer expressions for this second parameter.

I'm preparing a fixup supporting mode 2. Sorting by any field/tuple.

If the function named groupSortedArray, it may sound like
"group sorted items into array" - incorrect
"group items into sorted array" - correct

If the function named groupArraySorted, it will sound like
"group items into array and get it sorted" - correct

See also #3708

@Mergifyio update

update

✅ Branch has been successfully updated

Thank you for your inputs, I have been adding the requested modifications and I think it is again ready for review.

List of modifications:

• Renamed to groupArraySorted
• New selection function for long arrays size (More than 1000)
• Added performance test for long arrays size
• Fixed memory access issue
• Simplified aggregation class creation methods
• Fixed syntax checks and other CI issues
• Added support for any type in the second parameter (sorted by)
• Rewritten addBatchSinglePlace() function to remove dynamic allocations and to make the code mode understandable

I can still see a failing check but I think it is not related with my code. (test_s3_zero_copy_replication/test.py::test_s3_zero_copy_drop_detached - FAILED)

Hey!

I would like to ask: @palegre-tiny and PR reviewers, do you know whether it would be difficult to add support for:

• groupArraySorted -Array extensions, like groupArraySortedArray and groupUniqArraySortedArray,
• SimpleAggregateFunction(groupArraySorted) and with -Array extensions?

For context, we will likely need these features for a project that we discuss currently at our company. We were willing to implement these functionalities from scratch on our own, then it was good news for us that somebody else is working on a similar design, so that we have a better starting point and a source of inspiration.

We would like to use SimpleAggregateFunction(groupArraySortedArray) and SimpleAggregateFunction(groupUniqArraySortedArray) in AggregatingMergeTree. Extensions with -Array flatten the input arrays into a single output array, and SimpleAggregateFunction is a wrapper for aggregate functions like groupArrayArray.

Could you share some thoughts about these extensions above:

• Can we reuse the design for groupArraySorted for -Array extensions? Are there any obstacles in the current design?
• Can we simply enable groupArraySorted as a valid choice for SimpleAggregateFunction in checkSupportedFunctions in src/DataTypes/DataTypeCustomSimpleAggregateFunction.cpp? Edit: this leads to Incompatible data types between aggregate function 'groupArraySorted' which returns Array(Array(String)) and column storage type Array(String), which is natural I guess, as a function with -Array suffix is needed to flatten the result.

There are likely many details to work out here anyway, but if you have enough context to give us hints, it would be great!

Thanks!

Jakub Kuklis
Contentsquare

Thanks Jakub for your comments.

1. About groupArraySorted +Array extensions

If I understood correctly you suggest something like this:

table_test_array
┌─a───────────┬─b─┐
│ [1,3,5,7,9] │ 2 │
│ [0,2,4,6,8] │ 1 │
└─────────────┴───┘

groupArraySortedArray(a) from table_test_array
┌─groupArraySortedArray(a)─┐
│ [0,1,2,3,4]              │
└──────────────────────────┘

And that is clear, but what if we want to use second parameter (sort expression), we would do the following:

groupArraySortedArray(a,b) from table_test_array
┌─groupArraySortedArray(a,b)─┐
│ [0,2,4,6,8]                │
└────────────────────────────┘

this case we would not split the arrays because each has different weight.

It is actually working like that but I have doubts about if it is clear from ussage perspective.

2. Use with SimpleAggregateFunction

I don't think we can use it, again when using 2 parameters we need to store the weight (sort expression) attached to the values, even if at the end we only show the values.

@Mergifyio update

update

✅ Branch has been successfully updated

For sorted by second parameter usage + array extension: It is necessary to use array in both parameters:

select groupArraySortedArray([1, 2, 3], [10, 0, 5])

┌─groupArraySortedArray([1, 2, 3], [10, 0, 5])─┐
│ [2,3,1]                                      │
└──────────────────────────────────────────────┘

@Mergifyio update

update

✅ Branch has been successfully updated

Hi @filimonov @evillique,
It's been a while since the last review/comment, is there anything I can do to continue this?
Thank you

@Mergifyio update

update

✅ Branch has been successfully updated

@evillique I did not understand the algorithm of this function in five seconds. Let's move it under "experimental" flag.