This is an automated email from the ASF dual-hosted git repository.
mrhhsg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 078e667a7ba [fix](column) Make struct hashing respect the outer
nullable mask (#67962)
078e667a7ba is described below
commit 078e667a7ba3112ffb50b9e80109e031ded3d47c
Author: Jerry Hu <[email protected]>
AuthorDate: Thu Oct 15 15:54:27 2026 +0800
[fix](column) Make struct hashing respect the outer nullable mask (#67962)
### What problem does this PR solve?
Issue Number: None
Problem Summary:
`ColumnStruct::update_crc32c_batch` / `update_crc32c_single` (and the
xxHash / zlib-crc range variants) ignored the `null_map` passed in by an
outer `ColumnNullable` and kept hashing the nested field columns. A NULL
struct row can still carry an arbitrary hidden payload in its fields,
for example `IF(cond, NULL, named_struct(...))` keeps the else-branch
value under the NULL flag. Two logically equal NULL structs therefore
got
different hashes and the local hash shuffle sent them to different
aggregation instances, so `GROUP BY` on a nullable struct key silently
produced one extra group per distinct hidden payload.
`ColumnArray` and `ColumnMap` already skip the NULL rows of the outer
mask; this change applies the same pattern to `ColumnStruct` for every
hash entry point that receives a null map. Rows without a null map keep
the existing column-wise batch path, so hashes of non-NULL rows are
unchanged.
### Release note
Hashing of a NULL `STRUCT` value no longer depends on the hidden field
payload stored under the NULL flag, so `GROUP BY` / `DISTINCT` on
nullable `STRUCT` keys (including `STRUCT` nested in `ARRAY` / `MAP`)
stop splitting equal NULL keys. Consequences of the changed hash value:
- During a rolling upgrade, old and new BEs may route the same NULL
`STRUCT` shuffle key to different receivers when the hidden payloads are
identical (for example stored NULLs); the divergence ends with the
upgrade. `SET GLOBAL enable_new_shuffle_hash_method = false` keeps
old/new BEs consistent only for top-level nullable `STRUCT` keys, not
for `STRUCT` nested in `ARRAY` / `MAP` / another `STRUCT`.
- An `agg_state<multi_distinct_count(ARRAY<...STRUCT...>)>` column
written by an old BE stores xxHash identifiers of its `ARRAY` values;
after the upgrade, an `ARRAY` containing a NULL `STRUCT` element gets a
new identifier, so such a persisted state can count that `ARRAY` value
once more than before when merged with new data.
### Check List (For Author)
- Test:
- Unit Test: `ColumnStructTest.*RespectsOuterNullMap` cover crc32c
batch/single, xxHash batch/range and zlib-crc range hashing of a
nullable struct whose NULL rows carry different payloads.
- Regression test:
`query_p0/aggregate/aggregate_groupby_nullable_struct_local_shuffle`
groups `IF(cond, NULL, named_struct(...))` keys through a local hash
shuffle and checks the result matches the non-local-shuffle plan.
- Behavior changed: Yes. NULL `STRUCT` rows are excluded from the hash
input, so hash values of nullable `STRUCT` keys change (see release
note).
- Does this need documentation: No
https://claude.ai/code/session_01WTHEM5JUWYV8RqrDr9B62p
### Follow-up review fix
All-NULL outer STRUCT masks now return before allocating hash scratch or
hashing hidden ARRAY/MAP fields. The batch xxHash and CRC32C tests use a
variable-length ARRAY child with a traversal counter to verify zero
child work for 33 all-NULL rows and continued dispatch for a mixed mask.
Validation at `1ed9b8212bc9933ce85f7aced0fd4302abb33a03`:
- `./run-be-ut.sh --run --filter='ColumnStructTest.*' -j2`: 10/10
passed.
- `./build.sh --be -j4` (ASAN): passed.
- clang-format, check-format, build hygiene, and `git diff --check`:
passed.
- clang-tidy attempted; it remains blocked by pre-existing `stddef.h`,
`NOLINTEND`, and nested-conditional diagnostics outside the new lines.
---
be/src/core/column/column_struct.cpp | 110 ++++++++-
be/test/core/column/column_struct_test.cpp | 270 +++++++++++++++++++++
.../column_array_update_crc32c_batch_42.out | 4 +-
...n_array_update_crc32c_batch_42.out_with_nullmap | 4 +-
.../column_array_update_crc32c_single_42.out | 2 +-
..._array_update_crc32c_single_42.out_with_nullmap | 2 +-
...egate_groupby_nullable_struct_local_shuffle.out | 22 ++
...te_groupby_nullable_struct_local_shuffle.groovy | 72 ++++++
8 files changed, 470 insertions(+), 16 deletions(-)
diff --git a/be/src/core/column/column_struct.cpp
b/be/src/core/column/column_struct.cpp
index fb15785df8b..edcf8a6a4b3 100644
--- a/be/src/core/column/column_struct.cpp
+++ b/be/src/core/column/column_struct.cpp
@@ -21,12 +21,15 @@
#include "core/column/column_struct.h"
#include <functional>
+#include <vector>
#include "core/assert_cast.h"
+#include "core/custom_allocator.h"
#include "core/data_type/primitive_type.h"
#include "core/typeid_cast.h"
#include "exec/sort/sort_block.h"
#include "pdqsort.h"
+#include "util/simd/bits.h"
class SipHash;
namespace doris {
@@ -218,24 +221,96 @@ void ColumnStruct::update_hash_with_value(size_t n,
SipHash& hash) const {
}
}
+namespace {
+
+// Invoke `func(run_start, run_end)` for every maximal run of rows in [start,
end) that are not
+// NULL according to `null_data`. An all-zero mask yields the single run
[start, end), so the
+// masked and unmasked range paths produce identical hashes and the same
number of calls.
+template <typename Func>
+void for_each_non_null_run(const uint8_t* __restrict null_data, size_t start,
size_t end,
+ Func&& func) {
+ size_t i = start;
+ while (i < end) {
+ while (i < end && null_data[i] != 0) {
+ ++i;
+ }
+ size_t run_start = i;
+ while (i < end && null_data[i] == 0) {
+ ++i;
+ }
+ if (run_start < i) {
+ func(run_start, i);
+ }
+ }
+}
+
+// Batch-hash every row through the field columns, then restore the rows that
are NULL at the
+// outer level. Surviving rows keep each field's batch hash contract (which
may differ from the
+// single/range contract, e.g. the width of a NULL default), and the field
columns are still
+// called once per column instead of once per row.
+template <typename HashType, typename Func>
+void batch_hash_skipping_null_rows(HashType* __restrict hashes, size_t rows,
+ const uint8_t* __restrict null_data, Func&&
hash_all_rows) {
+ // No field hash can survive when every row is NULL; retain the incoming
seeds.
+ if (!simd::contain_zero(null_data, rows)) {
+ return;
+ }
+ DorisVector<HashType> saved(hashes, hashes + rows);
+ hash_all_rows();
+ for (size_t i = 0; i < rows; ++i) {
+ if (null_data[i] != 0) {
+ hashes[i] = saved[i];
+ }
+ }
+}
+
+} // namespace
+
+// `null_data` is the null map of an outer Nullable wrapping this struct. A
NULL struct row
+// may still carry arbitrary payload in the field columns (e.g. produced by
IF/CASE), so those
+// rows must be skipped here rather than hashed by their hidden payload. The
field columns are
+// hashed with a null `null_data` because their own NULL semantics are handled
by themselves.
void ColumnStruct::update_xxHash_with_value(size_t start, size_t end,
uint64_t& hash,
const uint8_t* __restrict
null_data) const {
- for (const auto& column : columns) {
- column->update_xxHash_with_value(start, end, hash, nullptr);
+ if (null_data) {
+ for (const auto& column : columns) {
+ for_each_non_null_run(null_data, start, end, [&](size_t run_start,
size_t run_end) {
+ column->update_xxHash_with_value(run_start, run_end, hash,
nullptr);
+ });
+ }
+ } else {
+ for (const auto& column : columns) {
+ column->update_xxHash_with_value(start, end, hash, nullptr);
+ }
}
}
void ColumnStruct::update_crc_with_value(size_t start, size_t end, uint32_t&
hash,
const uint8_t* __restrict null_data)
const {
- for (const auto& column : columns) {
- column->update_crc_with_value(start, end, hash, nullptr);
+ if (null_data) {
+ for (const auto& column : columns) {
+ for_each_non_null_run(null_data, start, end, [&](size_t run_start,
size_t run_end) {
+ column->update_crc_with_value(run_start, run_end, hash,
nullptr);
+ });
+ }
+ } else {
+ for (const auto& column : columns) {
+ column->update_crc_with_value(start, end, hash, nullptr);
+ }
}
}
void ColumnStruct::update_hashes_with_value(uint64_t* __restrict hashes,
const uint8_t* __restrict
null_data) const {
- for (const auto& column : columns) {
- column->update_hashes_with_value(hashes, null_data);
+ auto hash_all_rows = [&]() {
+ for (const auto& column : columns) {
+ column->update_hashes_with_value(hashes, nullptr);
+ }
+ };
+ if (null_data) {
+ batch_hash_skipping_null_rows(hashes, size(), null_data,
hash_all_rows);
+ } else {
+ hash_all_rows();
}
}
@@ -259,15 +334,30 @@ void ColumnStruct::update_crcs_with_value(uint32_t*
__restrict hash, PrimitiveTy
void ColumnStruct::update_crc32c_batch(uint32_t* __restrict hashes,
const uint8_t* __restrict null_map)
const {
- for (const auto& column : columns) {
- column->update_crc32c_batch(hashes, nullptr);
+ auto hash_all_rows = [&]() {
+ for (const auto& column : columns) {
+ column->update_crc32c_batch(hashes, nullptr);
+ }
+ };
+ if (null_map) {
+ batch_hash_skipping_null_rows(hashes, size(), null_map, hash_all_rows);
+ } else {
+ hash_all_rows();
}
}
void ColumnStruct::update_crc32c_single(size_t start, size_t end, uint32_t&
hash,
const uint8_t* __restrict null_map)
const {
- for (const auto& column : columns) {
- column->update_crc32c_single(start, end, hash, nullptr);
+ if (null_map) {
+ for (const auto& column : columns) {
+ for_each_non_null_run(null_map, start, end, [&](size_t run_start,
size_t run_end) {
+ column->update_crc32c_single(run_start, run_end, hash,
nullptr);
+ });
+ }
+ } else {
+ for (const auto& column : columns) {
+ column->update_crc32c_single(start, end, hash, nullptr);
+ }
}
}
diff --git a/be/test/core/column/column_struct_test.cpp
b/be/test/core/column/column_struct_test.cpp
index 2597f3f1b30..49994ef7cc6 100644
--- a/be/test/core/column/column_struct_test.cpp
+++ b/be/test/core/column/column_struct_test.cpp
@@ -21,14 +21,23 @@
#include <gtest/gtest-test-part.h>
#include <gtest/gtest.h>
+#include <cstddef>
#include <cstdint>
#include <iostream>
+#include <limits>
+#include <string>
+#include <typeinfo>
+#include <utility>
+#include <vector>
#include "core/block/block.h"
#include "core/column/column_array.h"
#include "core/column/column_const.h"
+#include "core/column/column_dummy.h"
#include "core/column/column_map.h"
#include "core/column/column_nullable.h"
+#include "core/column/column_vector.h"
+#include "core/cow.h"
#include "core/data_type/data_type.h"
#include "core/data_type/data_type_array.h"
#include "core/data_type/data_type_date.h"
@@ -39,6 +48,7 @@
#include "core/data_type/data_type_number.h"
#include "core/data_type/data_type_string.h"
#include "core/data_type/data_type_struct.h"
+#include "core/field.h"
#include "core/types.h"
namespace doris {
@@ -50,6 +60,266 @@ protected:
void TearDown() override {}
};
+namespace {
+
+// An ARRAY element column that records how many hidden elements the ARRAY
hash visits.
+class HashCountingColumn final : public COWHelper<IColumnDummy,
HashCountingColumn> {
+private:
+ friend class COWHelper<IColumnDummy, HashCountingColumn>;
+
+ explicit HashCountingColumn(size_t rows) { s = rows; }
+
+public:
+ std::string get_name() const override { return "HashCounting"; }
+ MutableColumnPtr clone_dummy(size_t rows) const override {
+ return HashCountingColumn::create(rows);
+ }
+ bool structure_equals(const IColumn& rhs) const override {
+ return typeid(rhs) == typeid(HashCountingColumn);
+ }
+
+ void update_xxHash_with_value(size_t start, size_t end, uint64_t&,
+ const uint8_t*) const override {
+ xx_hashed_elements += end - start;
+ }
+ void update_crc32c_single(size_t start, size_t end, uint32_t&, const
uint8_t*) const override {
+ crc_hashed_elements += end - start;
+ }
+
+ mutable size_t xx_hashed_elements = 0;
+ mutable size_t crc_hashed_elements = 0;
+};
+
+struct AllNullArrayFixture {
+ static constexpr size_t rows = 33; // Cover both the SIMD chunk and scalar
tail.
+ static constexpr size_t elements_per_row = 3;
+
+ HashCountingColumn* counter;
+ ColumnPtr plain_struct;
+ ColumnPtr nullable;
+ std::vector<uint8_t> null_map = std::vector<uint8_t>(rows, 1);
+
+ AllNullArrayFixture() {
+ auto elements = HashCountingColumn::create(rows * elements_per_row);
+ counter = elements.get();
+ auto offsets = ColumnArray::ColumnOffsets::create();
+ auto nullable_null_map = ColumnUInt8::create();
+ for (size_t i = 1; i <= rows; ++i) {
+ offsets->insert_value(i * elements_per_row);
+ nullable_null_map->insert_value(1);
+ }
+ Columns fields;
+ fields.push_back(ColumnArray::create(std::move(elements),
std::move(offsets)));
+ plain_struct = ColumnStruct::create(std::move(fields));
+ nullable = ColumnNullable::create(plain_struct,
std::move(nullable_null_map));
+ }
+};
+
+// Nullable(Struct<x: Nullable(Int32)>) with 4 rows. Rows 1 and 3 are NULL at
the outer level
+// but keep different hidden payloads in the field column, which is what
IF/CASE produce when
+// only one branch is a NULL literal. Rows 0 and 3 share the same payload but
differ in nullness;
+// NULL is hashed as the field's default value, so only the NULL/NULL equality
is asserted.
+struct NullableStructFixture {
+ ColumnPtr nullable;
+ const ColumnStruct* plain_struct = nullptr;
+ std::vector<uint8_t> null_map = {0, 1, 0, 1};
+
+ NullableStructFixture() {
+ auto field = ColumnNullable::create(ColumnInt32::create(),
ColumnUInt8::create());
+ for (int32_t v : {0, std::numeric_limits<int32_t>::min(), 1, 0}) {
+ field->insert(Field::create_field<TYPE_INT>(v));
+ }
+ Columns fields;
+ fields.push_back(std::move(field));
+ ColumnPtr struct_column = ColumnStruct::create(std::move(fields));
+ plain_struct = assert_cast<const ColumnStruct*>(struct_column.get());
+
+ auto null_map_column = ColumnUInt8::create();
+ for (auto v : null_map) {
+ null_map_column->insert_value(v);
+ }
+ nullable = ColumnNullable::create(struct_column,
std::move(null_map_column));
+ }
+};
+
+} // namespace
+
+TEST_F(ColumnStructTest, Crc32cBatchRespectsOuterNullMap) {
+ NullableStructFixture fixture;
+ std::vector<uint32_t> hashes(4, 0);
+ fixture.nullable->update_crc32c_batch(hashes.data(), nullptr);
+ std::vector<uint32_t> plain_hashes(4, 0);
+ fixture.plain_struct->update_crc32c_batch(plain_hashes.data(), nullptr);
+
+ // Two logical NULLs must hash identically regardless of hidden payload.
+ EXPECT_EQ(hashes[1], hashes[3]);
+ // Non-NULL rows hash exactly like the plain struct column.
+ EXPECT_EQ(hashes[0], plain_hashes[0]);
+ EXPECT_EQ(hashes[2], plain_hashes[2]);
+
+ // Calling the struct directly with a null map skips the NULL rows.
+ std::vector<uint32_t> masked_hashes(4, 0);
+ fixture.plain_struct->update_crc32c_batch(masked_hashes.data(),
fixture.null_map.data());
+ EXPECT_EQ(masked_hashes[0], plain_hashes[0]);
+ EXPECT_EQ(masked_hashes[1], 0);
+ EXPECT_EQ(masked_hashes[2], plain_hashes[2]);
+ EXPECT_EQ(masked_hashes[3], 0);
+}
+
+TEST_F(ColumnStructTest, Crc32cSingleRespectsOuterNullMap) {
+ NullableStructFixture fixture;
+ auto hash_row = [&](const IColumn& column, size_t row) {
+ uint32_t hash = 0;
+ column.update_crc32c_single(row, row + 1, hash, nullptr);
+ return hash;
+ };
+ EXPECT_EQ(hash_row(*fixture.nullable, 1), hash_row(*fixture.nullable, 3));
+ EXPECT_EQ(hash_row(*fixture.nullable, 0), hash_row(*fixture.plain_struct,
0));
+ EXPECT_EQ(hash_row(*fixture.nullable, 2), hash_row(*fixture.plain_struct,
2));
+
+ // A multi-row range with a null map only hashes the non-NULL rows.
+ uint32_t masked = 0;
+ fixture.plain_struct->update_crc32c_single(0, 4, masked,
fixture.null_map.data());
+ uint32_t expected = 0;
+ fixture.plain_struct->update_crc32c_single(0, 1, expected, nullptr);
+ fixture.plain_struct->update_crc32c_single(2, 3, expected, nullptr);
+ EXPECT_EQ(masked, expected);
+}
+
+TEST_F(ColumnStructTest, XxHashRespectsOuterNullMap) {
+ NullableStructFixture fixture;
+ std::vector<uint64_t> hashes(4, 0);
+ fixture.nullable->update_hashes_with_value(hashes.data(), nullptr);
+ std::vector<uint64_t> plain_hashes(4, 0);
+ fixture.plain_struct->update_hashes_with_value(plain_hashes.data(),
nullptr);
+ EXPECT_EQ(hashes[1], hashes[3]);
+ EXPECT_EQ(hashes[0], plain_hashes[0]);
+ EXPECT_EQ(hashes[2], plain_hashes[2]);
+
+ auto hash_row = [&](const IColumn& column, size_t row) {
+ uint64_t hash = 0;
+ column.update_xxHash_with_value(row, row + 1, hash, nullptr);
+ return hash;
+ };
+ EXPECT_EQ(hash_row(*fixture.nullable, 1), hash_row(*fixture.nullable, 3));
+ EXPECT_EQ(hash_row(*fixture.nullable, 0), hash_row(*fixture.plain_struct,
0));
+ EXPECT_EQ(hash_row(*fixture.nullable, 2), hash_row(*fixture.plain_struct,
2));
+}
+
+TEST_F(ColumnStructTest, CrcRangeRespectsOuterNullMap) {
+ NullableStructFixture fixture;
+ auto hash_row = [&](const IColumn& column, size_t row) {
+ uint32_t hash = 0;
+ column.update_crc_with_value(row, row + 1, hash, nullptr);
+ return hash;
+ };
+ EXPECT_EQ(hash_row(*fixture.nullable, 1), hash_row(*fixture.nullable, 3));
+ EXPECT_EQ(hash_row(*fixture.nullable, 0), hash_row(*fixture.plain_struct,
0));
+ EXPECT_EQ(hash_row(*fixture.nullable, 2), hash_row(*fixture.plain_struct,
2));
+}
+
+// Struct<x: Nullable(BIGINT)> whose row 0 has a NULL field and row 1 a
non-NULL field, wrapped
+// once with an outer NULL on row 1 and once without. Row 0 is the same
logical value in both
+// blocks and must hash identically: the batch path keeps the field's batch
NULL encoding even
+// when an unrelated outer NULL forces the masked path.
+TEST_F(ColumnStructTest,
BatchHashKeepsChildBatchContractWithUnrelatedOuterNull) {
+ auto make_block = [](const std::vector<uint8_t>& outer_null_map) {
+ auto field = ColumnNullable::create(ColumnInt64::create(),
ColumnUInt8::create());
+ field->insert_default();
+ field->insert(Field::create_field<TYPE_BIGINT>(int64_t(5)));
+ Columns fields;
+ fields.push_back(std::move(field));
+ ColumnPtr struct_column = ColumnStruct::create(std::move(fields));
+ auto null_map_column = ColumnUInt8::create();
+ for (auto v : outer_null_map) {
+ null_map_column->insert_value(v);
+ }
+ return ColumnNullable::create(struct_column,
std::move(null_map_column));
+ };
+ ColumnPtr with_outer_null = make_block({0, 1});
+ ColumnPtr without_outer_null = make_block({0, 0});
+
+ constexpr uint32_t seed32 = 0x12345678;
+ std::vector<uint32_t> crc_a(2, seed32);
+ std::vector<uint32_t> crc_b(2, seed32);
+ with_outer_null->update_crc32c_batch(crc_a.data(), nullptr);
+ without_outer_null->update_crc32c_batch(crc_b.data(), nullptr);
+ EXPECT_EQ(crc_a[0], crc_b[0]);
+
+ constexpr uint64_t seed64 = 0x123456789abcdefULL;
+ std::vector<uint64_t> xx_a(2, seed64);
+ std::vector<uint64_t> xx_b(2, seed64);
+ with_outer_null->update_hashes_with_value(xx_a.data(), nullptr);
+ without_outer_null->update_hashes_with_value(xx_b.data(), nullptr);
+ EXPECT_EQ(xx_a[0], xx_b[0]);
+}
+
+TEST_F(ColumnStructTest, AllNullBatchSkipsVariableLengthChildXxHash) {
+ AllNullArrayFixture fixture;
+ std::vector<uint64_t> hashes(fixture.rows, 11);
+ fixture.plain_struct->update_hashes_with_value(hashes.data(),
fixture.null_map.data());
+ EXPECT_EQ(fixture.counter->xx_hashed_elements, 0);
+ for (auto hash : hashes) {
+ EXPECT_EQ(hash, 11);
+ }
+
+ fixture.nullable->update_hashes_with_value(hashes.data(), nullptr);
+ EXPECT_EQ(fixture.counter->xx_hashed_elements, 0);
+ for (auto hash : hashes) {
+ EXPECT_EQ(hash, hashes[0]);
+ }
+
+ fixture.null_map.back() = 0;
+ fixture.plain_struct->update_hashes_with_value(hashes.data(),
fixture.null_map.data());
+ EXPECT_GT(fixture.counter->xx_hashed_elements, 0);
+}
+
+TEST_F(ColumnStructTest, AllNullBatchSkipsVariableLengthChildCrc32c) {
+ AllNullArrayFixture fixture;
+ std::vector<uint32_t> hashes(fixture.rows, 13);
+ fixture.plain_struct->update_crc32c_batch(hashes.data(),
fixture.null_map.data());
+ EXPECT_EQ(fixture.counter->crc_hashed_elements, 0);
+ for (auto hash : hashes) {
+ EXPECT_EQ(hash, 13);
+ }
+
+ fixture.nullable->update_crc32c_batch(hashes.data(), nullptr);
+ EXPECT_EQ(fixture.counter->crc_hashed_elements, 0);
+ for (auto hash : hashes) {
+ EXPECT_EQ(hash, hashes[0]);
+ }
+
+ fixture.null_map.back() = 0;
+ fixture.plain_struct->update_crc32c_batch(hashes.data(),
fixture.null_map.data());
+ EXPECT_GT(fixture.counter->crc_hashed_elements, 0);
+}
+
+TEST_F(ColumnStructTest, RangeHashWithAllZeroMaskMatchesUnmasked) {
+ NullableStructFixture fixture;
+ std::vector<uint8_t> zeros(4, 0);
+ {
+ uint32_t masked = 7;
+ uint32_t plain = 7;
+ fixture.plain_struct->update_crc32c_single(0, 4, masked, zeros.data());
+ fixture.plain_struct->update_crc32c_single(0, 4, plain, nullptr);
+ EXPECT_EQ(masked, plain);
+ }
+ {
+ uint64_t masked = 7;
+ uint64_t plain = 7;
+ fixture.plain_struct->update_xxHash_with_value(0, 4, masked,
zeros.data());
+ fixture.plain_struct->update_xxHash_with_value(0, 4, plain, nullptr);
+ EXPECT_EQ(masked, plain);
+ }
+ {
+ uint32_t masked = 7;
+ uint32_t plain = 7;
+ fixture.plain_struct->update_crc_with_value(0, 4, masked,
zeros.data());
+ fixture.plain_struct->update_crc_with_value(0, 4, plain, nullptr);
+ EXPECT_EQ(masked, plain);
+ }
+}
+
TEST_F(ColumnStructTest, StructTypeTesterase) {
DataTypePtr key_type = (std::make_shared<DataTypeString>());
DataTypePtr value_type = (std::make_shared<DataTypeInt32>());
diff --git
a/be/test/expected_result/vec/columns/column_array_update_crc32c_batch_42.out
b/be/test/expected_result/vec/columns/column_array_update_crc32c_batch_42.out
index 33e8d33173a..4ba6d12d5fd 100644
---
a/be/test/expected_result/vec/columns/column_array_update_crc32c_batch_42.out
+++
b/be/test/expected_result/vec/columns/column_array_update_crc32c_batch_42.out
@@ -6,11 +6,11 @@
2912776853
3311270576
213263290
-3622140960
+213263290
973448334
4140789047
422470270
1499855502
1781817829
3138811262
-1757854213
\ No newline at end of file
+243623783
\ No newline at end of file
diff --git
a/be/test/expected_result/vec/columns/column_array_update_crc32c_batch_42.out_with_nullmap
b/be/test/expected_result/vec/columns/column_array_update_crc32c_batch_42.out_with_nullmap
index 33e8d33173a..4ba6d12d5fd 100644
---
a/be/test/expected_result/vec/columns/column_array_update_crc32c_batch_42.out_with_nullmap
+++
b/be/test/expected_result/vec/columns/column_array_update_crc32c_batch_42.out_with_nullmap
@@ -6,11 +6,11 @@
2912776853
3311270576
213263290
-3622140960
+213263290
973448334
4140789047
422470270
1499855502
1781817829
3138811262
-1757854213
\ No newline at end of file
+243623783
\ No newline at end of file
diff --git
a/be/test/expected_result/vec/columns/column_array_update_crc32c_single_42.out
b/be/test/expected_result/vec/columns/column_array_update_crc32c_single_42.out
index be0367bd02e..15e46575237 100644
---
a/be/test/expected_result/vec/columns/column_array_update_crc32c_single_42.out
+++
b/be/test/expected_result/vec/columns/column_array_update_crc32c_single_42.out
@@ -1 +1 @@
-2261631211;834435238;2794456307
\ No newline at end of file
+2583553548;3415904288;2794456307
\ No newline at end of file
diff --git
a/be/test/expected_result/vec/columns/column_array_update_crc32c_single_42.out_with_nullmap
b/be/test/expected_result/vec/columns/column_array_update_crc32c_single_42.out_with_nullmap
index be0367bd02e..15e46575237 100644
---
a/be/test/expected_result/vec/columns/column_array_update_crc32c_single_42.out_with_nullmap
+++
b/be/test/expected_result/vec/columns/column_array_update_crc32c_single_42.out_with_nullmap
@@ -1 +1 @@
-2261631211;834435238;2794456307
\ No newline at end of file
+2583553548;3415904288;2794456307
\ No newline at end of file
diff --git
a/regression-test/data/query_p0/aggregate/aggregate_groupby_nullable_struct_local_shuffle.out
b/regression-test/data/query_p0/aggregate/aggregate_groupby_nullable_struct_local_shuffle.out
new file mode 100644
index 00000000000..f94a7ac40b0
--- /dev/null
+++
b/regression-test/data/query_p0/aggregate/aggregate_groupby_nullable_struct_local_shuffle.out
@@ -0,0 +1,22 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !local_shuffle_crc32c_groups --
+\N 2
+{"x":1} 1
+
+-- !local_shuffle_crc32c_summary --
+2 3 1 2
+
+-- !local_shuffle_legacy_hash_groups --
+\N 2
+{"x":1} 1
+
+-- !local_shuffle_legacy_hash_summary --
+2 3 1 2
+
+-- !no_local_shuffle_groups --
+\N 2
+{"x":1} 1
+
+-- !no_local_shuffle_summary --
+2 3 1 2
+
diff --git
a/regression-test/suites/query_p0/aggregate/aggregate_groupby_nullable_struct_local_shuffle.groovy
b/regression-test/suites/query_p0/aggregate/aggregate_groupby_nullable_struct_local_shuffle.groovy
new file mode 100644
index 00000000000..0e672743469
--- /dev/null
+++
b/regression-test/suites/query_p0/aggregate/aggregate_groupby_nullable_struct_local_shuffle.groovy
@@ -0,0 +1,72 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("aggregate_groupby_nullable_struct_local_shuffle") {
+ // IF(cond, NULL, named_struct(...)) yields logical NULL structs whose
hidden field payloads
+ // differ per row. GROUP BY must treat them as one group no matter how the
rows are hashed
+ // by the local shuffle before aggregation.
+ sql """ DROP TABLE IF EXISTS agg_groupby_nullable_struct_ls """
+ sql """
+ CREATE TABLE agg_groupby_nullable_struct_ls (
+ pk INT NOT NULL
+ )
+ DUPLICATE KEY(pk)
+ DISTRIBUTED BY HASH(pk) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1"
+ )
+ """
+ sql """ INSERT INTO agg_groupby_nullable_struct_ls VALUES (2), (7), (11)
"""
+
+ def groupQuery = """
+ SELECT k, COUNT(*) AS c
+ FROM (
+ SELECT IF(pk IN (2, 7), NULL,
+ named_struct('x', CASE WHEN pk = 2 THEN 0
+ WHEN pk = 7 THEN -2147483648
+ ELSE 1 END)) AS k
+ FROM agg_groupby_nullable_struct_ls
+ ) z
+ GROUP BY k
+ """
+ def summaryQuery = """
+ SELECT COUNT(*) AS groups_n, SUM(c) AS rows_n, MIN(c) AS min_c, MAX(c)
AS max_c
+ FROM (${groupQuery}) q
+ """
+
+ // Force a local hash shuffle on the struct key before the single-phase
aggregation.
+ sql """ SET enable_local_shuffle_planner = true """
+ sql """ SET enable_query_cache = false """
+ sql """ SET agg_phase = 1 """
+ sql """ SET enable_bucketed_hash_agg = false """
+ sql """ SET experimental_use_serial_exchange = true """
+ sql """ SET ignore_storage_data_distribution = true """
+ sql """ SET parallel_pipeline_task_num = 3 """
+
+ sql """ SET enable_local_shuffle = true """
+ sql """ SET enable_new_shuffle_hash_method = true """
+ order_qt_local_shuffle_crc32c_groups "${groupQuery}"
+ order_qt_local_shuffle_crc32c_summary "${summaryQuery}"
+
+ sql """ SET enable_new_shuffle_hash_method = false """
+ order_qt_local_shuffle_legacy_hash_groups "${groupQuery}"
+ order_qt_local_shuffle_legacy_hash_summary "${summaryQuery}"
+
+ sql """ SET enable_local_shuffle = false """
+ order_qt_no_local_shuffle_groups "${groupQuery}"
+ order_qt_no_local_shuffle_summary "${summaryQuery}"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
