This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 5017e28dfc1 branch-4.2: [fix](be) Fix length reads on dictionary
columns (#69279)
5017e28dfc1 is described below
commit 5017e28dfc16c93cedb730bcdee3885fd0cb1b66
Author: HappenLee <[email protected]>
AuthorDate: Tue Oct 20 16:16:42 2026 +0800
branch-4.2: [fix](be) Fix length reads on dictionary columns (#69279)
### What problem does this PR solve?
Issue Number: N/A
Related PR: #63498, #68214
Problem Summary:
With low-cardinality optimization enabled, a query such as `SELECT
length(varchar_column) FROM t` can fail with `NOT_IMPLEMENTED_ERROR:
Method insert_offsets_from_lengths is not supported` when the storage
reader only reads string offsets. `Schema::get_predicate_column_ptr()`
creates a `ColumnDictI32`, but the dictionary decoder writes lengths to
it without converting it to a string column.
This is an adapted backport of #63498 to `branch-4.2`. The offset-only
dictionary decoding path was introduced on this branch by #68214.
Convert dictionary destinations before length insertion in both
`next_batch()` and `read_by_rowids()`, matching the existing plain-page
fallback. This branch already converts dictionaries to `ColumnString`,
which implements `insert_offsets_from_lengths()`, so the removed
`PredicateColumnType` implementation from the original PR is
unnecessary.
The original SQL regression suite and its generated result file are
preserved unchanged. A BE unit test additionally covers both dictionary
page formats, nullable/non-nullable destinations, empty/repeated
strings, sequential batch appends, and rowid selection with a nonzero
page ordinal.
### Release note
Fix `length()` failing on dictionary-encoded string columns when
low-cardinality and offset-only reading optimizations are combined.
### Check List (For Author)
- Test:
- clang-format 16.0.6 check passed for both changed C++ files.
Whitespace check passed with the unchanged upstream generated `.out`
final blank separator allowed.
- Local ASAN BE unit-test build attempted with `./run-be-ut.sh -j 48
--run --filter=BinaryDictPageTest.TestOffsetOnlyDictionaryColumn`. After
initializing the OpenBLAS/Faiss submodules missing from the UT entry
point and retrying, CMake configuration still failed before compilation
because the local Clang toolchain could not resolve `OpenMP_C`
(`OpenMP_C_FLAGS` / `OpenMP_C_LIB_NAMES`). Tests have not executed; CI
validation is still required.
- The upstream SQL regression suite is included unchanged; it has not
been run against a local cluster.
- Behavior changed: Yes. Valid string-length queries no longer fail on
an unsupported dictionary-column operation.
- Does this need documentation: No
---
be/src/storage/segment/binary_dict_page.cpp | 4 +
be/test/storage/segment/binary_dict_page_test.cpp | 74 ++++++++++++++
.../string_functions/test_length_dict_encoded.out | 55 +++++++++++
.../test_length_dict_encoded.groovy | 107 +++++++++++++++++++++
4 files changed, 240 insertions(+)
diff --git a/be/src/storage/segment/binary_dict_page.cpp
b/be/src/storage/segment/binary_dict_page.cpp
index 0ec1531f3da..e272a71d820 100644
--- a/be/src/storage/segment/binary_dict_page.cpp
+++ b/be/src/storage/segment/binary_dict_page.cpp
@@ -282,6 +282,8 @@ Status BinaryDictPageDecoder::next_batch(size_t* n,
MutableColumnPtr& dst) {
*n = max_fetch;
if (_options.only_read_offsets) {
+ // ColumnDictI32 cannot insert lengths; materialize it as ColumnString
first.
+ dst = dst->convert_to_predicate_column_if_dictionary();
// OFFSET_ONLY mode: resolve dict codes to get real string lengths
// without copying actual char data. This allows length() to work.
const auto* data_array = reinterpret_cast<const
int32_t*>(_bit_shuffle_ptr->get_data(0));
@@ -326,6 +328,8 @@ Status BinaryDictPageDecoder::read_by_rowids(const rowid_t*
rowids, ordinal_t pa
auto total = *n;
if (_options.only_read_offsets) {
+ // ColumnDictI32 cannot insert lengths; materialize it as ColumnString
first.
+ dst = dst->convert_to_predicate_column_if_dictionary();
// OFFSET_ONLY mode: resolve dict codes to get real string lengths
// without copying actual char data. This allows length() to work
correctly.
const auto* data_array = reinterpret_cast<const
int32_t*>(_bit_shuffle_ptr->get_data(0));
diff --git a/be/test/storage/segment/binary_dict_page_test.cpp
b/be/test/storage/segment/binary_dict_page_test.cpp
index 79c76ba52d6..cd65a6dc1b4 100644
--- a/be/test/storage/segment/binary_dict_page_test.cpp
+++ b/be/test/storage/segment/binary_dict_page_test.cpp
@@ -26,6 +26,8 @@
#include "common/config.h"
#include "common/logging.h"
+#include "core/column/column_dictionary.h"
+#include "core/column/column_nullable.h"
#include "core/column/column_string.h"
#include "runtime/exec_env.h"
#include "storage/olap_common.h"
@@ -560,6 +562,78 @@ private:
std::unique_ptr<segment_v2::EncodingInfoResolver> _resolver;
};
+// Both decoder entry points must accept low-cardinality destinations in
OFFSET_ONLY mode.
+TEST_F(BinaryDictPageTest, TestOffsetOnlyDictionaryColumn) {
+ const std::vector<Slice> slices = {Slice(""), Slice("x"), Slice("abcd"),
Slice("x")};
+ for (bool use_v2 : {false, true}) {
+ SCOPED_TRACE(use_v2);
+ PageBuilderOptions builder_options;
+ builder_options.data_page_size = 256 * 1024;
+ builder_options.dict_page_size = 256 * 1024;
+ builder_options.dict_binary_plain_encoding =
+ use_v2 ? BinaryPlainEncodingTypePB::BINARY_PLAIN_ENCODING_V2
+ : BinaryPlainEncodingTypePB::BINARY_PLAIN_ENCODING_V1;
+ auto builder = create_and_add_data(slices, builder_options);
+ ASSERT_NE(nullptr, builder);
+ OwnedSlice data;
+ ASSERT_TRUE(builder->finish(&data).ok());
+ OwnedSlice dictionary;
+ ASSERT_TRUE(builder->get_dictionary_page(&dictionary).ok());
+ EncodingTypePB encoding;
+ ASSERT_TRUE(builder->get_dictionary_page_encoding(&encoding).ok());
+ Slice dictionary_slice = dictionary.slice();
+ std::unique_ptr<DataPage> decoded_dictionary;
+ auto dictionary_decoder =
+ create_dict_page_decoder(dictionary_slice, encoding,
decoded_dictionary);
+ ASSERT_NE(nullptr, dictionary_decoder);
+ std::vector<StringRef> words(dictionary_decoder->count());
+ ASSERT_TRUE(dictionary_decoder->get_dict_word_info(words.data()).ok());
+ Slice data_slice = data.slice();
+ std::unique_ptr<DataPage> decoded_data;
+ ASSERT_TRUE(apply_pre_decode(data_slice, decoded_data).ok());
+
+ for (bool nullable : {false, true}) {
+ SCOPED_TRACE(nullable);
+ for (bool use_rowids : {false, true}) {
+ SCOPED_TRACE(use_rowids);
+ PageDecoderOptions options;
+ options.only_read_offsets = true;
+ BinaryDictPageDecoder decoder(data_slice, options);
+ ASSERT_TRUE(decoder.init().ok());
+ ASSERT_TRUE(decoder.is_dict_encoding());
+ decoder.set_dict_decoder(words.size(), words.data());
+ MutableColumnPtr column = ColumnDictI32::create();
+ if (nullable) {
+ column = ColumnNullable::create(std::move(column),
ColumnUInt8::create());
+ }
+ const rowid_t rowids[] = {17, 19, 20};
+ if (use_rowids) {
+ size_t count = 3;
+ ASSERT_TRUE(decoder.read_by_rowids(rowids, 17, &count,
column).ok());
+ ASSERT_EQ(3, count);
+ } else {
+ // Append a second batch after the dictionary destination
has been converted.
+ size_t count = 2;
+ ASSERT_TRUE(decoder.next_batch(&count, column).ok());
+ ASSERT_EQ(2, count);
+ count = 2;
+ ASSERT_TRUE(decoder.next_batch(&count, column).ok());
+ ASSERT_EQ(2, count);
+ }
+ const size_t expected_rows = use_rowids ? 3 : slices.size();
+ ASSERT_EQ(expected_rows, column->size());
+ for (size_t i = 0; i < expected_rows; ++i) {
+ size_t source_row = use_rowids ? rowids[i] - 17 : i;
+ EXPECT_EQ(slices[source_row].size,
column->get_data_at(i).size);
+ if (nullable) {
+ EXPECT_FALSE(column->is_null_at(i));
+ }
+ }
+ }
+ }
+ }
+}
+
// Local behavior tests - test specific config behavior
TEST_F(BinaryDictPageTest, TestConfigUsePlainBinaryV2False) {
std::vector<Slice> slices;
diff --git
a/regression-test/data/query_p0/sql_functions/string_functions/test_length_dict_encoded.out
b/regression-test/data/query_p0/sql_functions/string_functions/test_length_dict_encoded.out
new file mode 100644
index 00000000000..684a50de7b9
--- /dev/null
+++
b/regression-test/data/query_p0/sql_functions/string_functions/test_length_dict_encoded.out
@@ -0,0 +1,55 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !length_nullable_not_null --
+1
+1
+1
+1
+1
+2
+
+-- !length_not_null_col --
+1
+1
+1
+1
+1
+1
+1
+1
+
+-- !length_with_nulls --
+\N 50
+\N 51
+1 24
+1 28
+1 30
+1 41
+1 5
+2 60
+
+-- !char_length_nullable_not_null --
+1
+1
+1
+1
+1
+2
+
+-- !length_nullable_is_null --
+\N
+\N
+
+-- !char_length_all_rows --
+\N 50
+\N 51
+1 24
+1 28
+1 30
+1 41
+1 5
+2 60
+
+-- !char_length_nullable_is_null --
+\N
+\N
+
diff --git
a/regression-test/suites/query_p0/sql_functions/string_functions/test_length_dict_encoded.groovy
b/regression-test/suites/query_p0/sql_functions/string_functions/test_length_dict_encoded.groovy
new file mode 100644
index 00000000000..6f64c5b9218
--- /dev/null
+++
b/regression-test/suites/query_p0/sql_functions/string_functions/test_length_dict_encoded.groovy
@@ -0,0 +1,107 @@
+// 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.
+
+// Regression test for: length() / char_length() on dict-encoded
(low-cardinality)
+// varchar columns must not throw NOT_IMPLEMENTED_ERROR when the
"only_read_offsets"
+// optimisation is active (i.e. when the storage layer resolves dict codes to
string
+// lengths without materialising actual character data).
+suite("test_length_dict_encoded") {
+ sql "DROP TABLE IF EXISTS test_length_dict_varchar"
+ sql """
+ CREATE TABLE test_length_dict_varchar (
+ col_int_undef_signed int NULL,
+ col_int_undef_signed_not_null int NOT NULL,
+ col_date_undef_signed date NULL,
+ col_date_undef_signed_not_null date NOT NULL,
+ col_varchar_5__undef_signed varchar(5) NULL,
+ col_varchar_5__undef_signed_not_null varchar(5) NOT NULL,
+ pk int NULL
+ ) ENGINE=OLAP
+ DUPLICATE KEY(col_int_undef_signed)
+ PARTITION BY RANGE(col_int_undef_signed) (
+ PARTITION p0 VALUES [('-2147483648'), ('4')),
+ PARTITION p1 VALUES [('4'), ('6')),
+ PARTITION p2 VALUES [('6'), ('7')),
+ PARTITION p3 VALUES [('7'), ('8')),
+ PARTITION p4 VALUES [('8'), ('10')),
+ PARTITION p5 VALUES [('10'), ('83647')),
+ PARTITION p100 VALUES [('83647'), ('2147483647'))
+ )
+ DISTRIBUTED BY HASH(pk) BUCKETS 10
+ PROPERTIES ('replication_allocation' = 'tag.location.default: 1')
+ """
+
+ sql """
+ INSERT INTO test_length_dict_varchar VALUES
+ (6, 5, '2023-12-13', '2023-12-11', 'o', 'i', 30),
+ (6, 6, NULL, '2023-12-18', 'w', 'l', 24),
+ (8, -8278102, '2023-12-13', '2023-12-11', 'x', 'c', 28),
+ (15971, 8, NULL, '2015-06-11', 'h', 'r', 41),
+ (6, 5, '2023-12-11', '2023-12-17', 'd', 'q', 5),
+ (7, 100, '2023-12-14', '2023-12-15', NULL, 'a', 50),
+ (7, 101, '2023-12-15', '2023-12-16', NULL, 'b', 51),
+ (9, 200, NULL, '2023-12-17', 'ab', 'd', 60)
+ """
+
+ // length() on nullable dict-encoded varchar filtered by IS NOT NULL
predicate
+ order_qt_length_nullable_not_null """
+ SELECT length(col_varchar_5__undef_signed)
+ FROM test_length_dict_varchar
+ WHERE col_varchar_5__undef_signed IS NOT NULL
+ """
+
+ // length() on NOT NULL dict-encoded varchar (no predicate needed)
+ order_qt_length_not_null_col """
+ SELECT length(col_varchar_5__undef_signed_not_null)
+ FROM test_length_dict_varchar
+ """
+
+ // length() returns NULL for NULL varchar values
+ order_qt_length_with_nulls """
+ SELECT length(col_varchar_5__undef_signed), pk
+ FROM test_length_dict_varchar
+ ORDER BY pk
+ """
+
+ // char_length() is a synonym and must work as well
+ order_qt_char_length_nullable_not_null """
+ SELECT char_length(col_varchar_5__undef_signed)
+ FROM test_length_dict_varchar
+ WHERE col_varchar_5__undef_signed IS NOT NULL
+ """
+
+ // length() returns NULL for NULL values — verify NULL passthrough
+ order_qt_length_nullable_is_null """
+ SELECT length(col_varchar_5__undef_signed)
+ FROM test_length_dict_varchar
+ WHERE col_varchar_5__undef_signed IS NULL
+ """
+
+ // char_length() on all rows including NULLs — verify dict code path with
mixed null/non-null
+ order_qt_char_length_all_rows """
+ SELECT char_length(col_varchar_5__undef_signed), pk
+ FROM test_length_dict_varchar
+ ORDER BY pk
+ """
+
+ // char_length() on NULL-only rows
+ order_qt_char_length_nullable_is_null """
+ SELECT char_length(col_varchar_5__undef_signed)
+ FROM test_length_dict_varchar
+ WHERE col_varchar_5__undef_signed IS NULL
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]