github-actions[bot] commented on code in PR #68438:
URL: https://github.com/apache/doris/pull/68438#discussion_r4082027952
##########
be/src/exprs/function/array/function_array_shuffle.cpp:
##########
@@ -70,16 +72,24 @@ class FunctionArrayShuffle : public IFunction {
block.get_by_position(arguments[0]).column->convert_to_full_column_if_const();
const auto& src_column_array = assert_cast<const
ColumnArray&>(*src_column);
- size_t seed = time(nullptr);
+ ColumnPtr dest_column_ptr;
if (arguments.size() == 2) {
- ColumnPtr seed_column =
-
block.get_by_position(arguments[1]).column->convert_to_full_column_if_const();
- seed = assert_cast<const
ColumnInt64*>(seed_column.get())->get_element(0);
+ const auto [seed_column, seed_const] =
+
unpack_if_const(block.get_by_position(arguments[1]).column);
+ const auto& seeds = assert_cast<const
ColumnInt64&>(*seed_column).get_data();
+ // Each row re-seeds with its own seed, so the result of a row only
+ // depends on its array and seed, not on the rows before it.
+ std::mt19937 g;
+ dest_column_ptr = _execute(src_column_array, [&](size_t row) ->
std::mt19937& {
+ g.seed(cast_set<uint32_t>(seeds[index_check_const(row,
seed_const)]));
Review Comment:
[P1] Skip seed evaluation for rows whose result is NULL
Default null handling unwraps the arguments, runs this method for every
physical row, and only afterward reapplies the union null map. The new callback
therefore validates seeds that cannot affect the result: for example, a mixed
block with arrays `([1, 2], NULL)` and seeds `(1, -1)` must return NULL for row
2, but this line throws on `-1`; `NULLIF(seed, -1)` can similarly retain `-1`
beneath a NULL seed. The old implementation only read seed row 0, so these
blocks succeeded. Please skip conversion/reseeding using the combined result
null map (replacing only NULL seed payloads is insufficient) and add both
mixed-row regressions.
##########
be/src/exprs/function/array/function_array_shuffle.cpp:
##########
@@ -106,7 +118,7 @@ class FunctionArrayShuffle : public IFunction {
auto last_offset = src_offsets[i - 1];
auto src_offset = src_offsets[i];
- std::shuffle(&permutation[last_offset], &permutation[src_offset],
g);
+ std::shuffle(&permutation[last_offset], &permutation[src_offset],
get_generator(i));
Review Comment:
[P2] Avoid reseeding rows that cannot be shuffled
`get_generator(i)` is evaluated before `std::shuffle`, so every empty or
singleton row still executes `g.seed(...)` and initializes the 624-word
`mt19937` state even though the shuffle consumes no random values. For blocks
dominated by trivial arrays, this turns one generator setup per block into
per-row state initialization without changing any output. Keep any required
seed range validation, but skip generator initialization and the shuffle when
`src_offset - last_offset <= 1`.
--
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]
