This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 4d7b727c4ac branch-4.1: [fix](zonemap) Keep every byte of a
STRING/VARCHAR zone map bound #67949 (#67998)
4d7b727c4ac is described below
commit 4d7b727c4ac646983a1b957be1d18eae150c2cb3
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Wed Sep 16 10:53:46 2026 +0800
branch-4.1: [fix](zonemap) Keep every byte of a STRING/VARCHAR zone map
bound #67949 (#67998)
Cherry-picked from #67949
Co-authored-by: Chenyang Sun <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../data_type_serde/data_type_string_serde.cpp | 15 ++--
be/test/storage/olap_type_test.cpp | 38 ++++++-----
be/test/storage/segment/zone_map_index_test.cpp | 79 ++++++++++++++++++++++
.../string/test_string_embedded_nul_zonemap.out | 73 ++++++++++++++++++++
.../string/test_string_embedded_nul_zonemap.groovy | 79 ++++++++++++++++++++++
5 files changed, 262 insertions(+), 22 deletions(-)
diff --git a/be/src/core/data_type_serde/data_type_string_serde.cpp
b/be/src/core/data_type_serde/data_type_string_serde.cpp
index 39eafdacf9a..bccc79e75cd 100644
--- a/be/src/core/data_type_serde/data_type_string_serde.cpp
+++ b/be/src/core/data_type_serde/data_type_string_serde.cpp
@@ -847,12 +847,15 @@ Status
DataTypeStringSerDeBase<ColumnType>::from_string(StringRef& str, IColumn&
template <typename ColumnType>
Status DataTypeStringSerDeBase<ColumnType>::from_olap_string(const
std::string& str, Field& field,
const
FormatOptions& options) const {
- // CHAR(N) writes through OlapColumnDataConvertorChar are zero-padded to
- // the declared schema length, so the serialized OLAP string carries
- // trailing '\0' bytes. strnlen() drops that padding to surface the
- // logical character content in the Field. VARCHAR / STRING never write
- // trailing '\0' through this path, so strnlen is a no-op for them.
- size_t len = strnlen(str.data(), str.size());
+ // CHAR(N) is zero-padded to the declared schema length before it is
written, so its
+ // stored bytes carry trailing '\0' and stop at the first one. The page
read path cuts
+ // CHAR values the same way (see BinaryPlainPageCharStripPreDecoder), so a
bound built
+ // like this stays comparable with the rows it describes.
+ //
+ // VARCHAR and STRING keep every byte they were given, '\0' included.
Cutting such a
+ // value at an embedded '\0' would give a bound the data never held, and a
zone map
+ // built from it prunes rows that match.
+ size_t len = _type == TYPE_CHAR ? strnlen(str.data(), str.size()) :
str.size();
field = Field::create_field<TYPE_STRING>(std::string(str.data(), len));
return Status::OK();
}
diff --git a/be/test/storage/olap_type_test.cpp
b/be/test/storage/olap_type_test.cpp
index 53b76ab5dce..4a9b3f5cb5d 100644
--- a/be/test/storage/olap_type_test.cpp
+++ b/be/test/storage/olap_type_test.cpp
@@ -1867,11 +1867,9 @@ TEST_F(OlapTypeTest, timestamptz_type) {
}
}
-// from_olap_string for string types (CHAR / VARCHAR / STRING) strnlens the
-// input so any trailing '\0' bytes that came from a fixed-width CHAR write
-// are dropped before the value lands in the Field. VARCHAR / STRING ZoneMap
-// values do not normally carry trailing '\0' (the writers store the natural
-// byte length), so strnlen is a no-op for them.
+// from_olap_string cuts a CHAR value at its first '\0': a CHAR(N) write pads
the value
+// with '\0' up to the schema length, and the page read path cuts it the same
way.
+// VARCHAR and STRING are stored with their natural byte length, so they keep
every byte.
TEST_F(OlapTypeTest, from_olap_string_strings) {
struct Case {
PrimitiveType type;
@@ -1884,9 +1882,7 @@ TEST_F(OlapTypeTest, from_olap_string_strings) {
{TYPE_CHAR, std::string("abc", 3) + std::string(7, '\0'), "abc"},
{TYPE_CHAR, std::string(10, '\0'), ""},
{TYPE_CHAR, "alpha", "alpha"},
- // VARCHAR / STRING never carry trailing '\0' in their ZoneMap
- // representation, so the helper is a transparent pass-through
- // for the typical case.
+ // VARCHAR / STRING are handed back byte for byte.
{TYPE_VARCHAR, "hello", "hello"},
{TYPE_STRING, "world\nline2", "world\nline2"},
{TYPE_STRING, "", ""},
@@ -1903,14 +1899,24 @@ TEST_F(OlapTypeTest, from_olap_string_strings) {
}
}
-// VARCHAR / STRING values containing an embedded '\0' are truncated at the
-// first '\0' — the same strnlen behaviour applies to all string types. This
-// is acceptable in practice because Doris string columns do not store
-// embedded NULs in their ZoneMap representation; the test pins the contract.
-TEST_F(OlapTypeTest, from_olap_string_strings_embedded_null_truncates) {
- auto data_type = DataTypeFactory::instance().create_data_type(
- TYPE_VARCHAR, /*is_nullable=*/false, 0, 0, /*length=*/32);
- expect_from_storage_string_paths(data_type, std::string("ab\0cd", 5),
[](const Field& field) {
+// A VARCHAR / STRING value may hold a '\0' in the middle, and every byte has
+// to survive the parse. Cutting at that '\0' used to give a zone map bound
the data never
+// held, which then pruned rows that match. CHAR is still cut, because its
'\0' is padding
+// and the page read path cuts CHAR values the same way.
+TEST_F(OlapTypeTest, from_olap_string_strings_embedded_null) {
+ const std::string embedded_null("ab\0cd", 5);
+
+ for (auto type : {TYPE_VARCHAR, TYPE_STRING}) {
+ auto data_type = DataTypeFactory::instance().create_data_type(type,
/*is_nullable=*/false,
+ 0, 0,
/*length=*/32);
+ expect_from_storage_string_paths(data_type, embedded_null, [&](const
Field& field) {
+ EXPECT_EQ(field.get<TYPE_STRING>(), embedded_null) << "type=" <<
static_cast<int>(type);
+ });
+ }
+
+ auto char_type = DataTypeFactory::instance().create_data_type(TYPE_CHAR,
/*is_nullable=*/false,
+ 0, 0,
/*length=*/5);
+ expect_from_storage_string_paths(char_type, embedded_null, [](const Field&
field) {
EXPECT_EQ(field.get<TYPE_STRING>(), "ab");
});
}
diff --git a/be/test/storage/segment/zone_map_index_test.cpp
b/be/test/storage/segment/zone_map_index_test.cpp
index 17d61ef693d..2ea9503b0b9 100644
--- a/be/test/storage/segment/zone_map_index_test.cpp
+++ b/be/test/storage/segment/zone_map_index_test.cpp
@@ -579,6 +579,74 @@ public:
}
}
}
+
+ // A STRING / VARCHAR value may hold '\0' in the middle, and the zone map
+ // bound has to keep those bytes. A bound cut at the '\0' is smaller than
the data it
+ // stands for, so a pushed-down comparison prunes pages that do hold
matching rows.
+ // CHAR is the exception: it is zero-padded to the schema length on write
and the page
+ // read path cuts every CHAR value at its first '\0', so its bound is cut
here too.
+ template <PrimitiveType PType>
+ void test_embedded_nul_bound(const std::string& testname, bool
bound_is_cut) {
+ // 'a' '\0' 'b' -- a value whose middle byte is '\0'.
+ const std::string value("a\0b", 3);
+ const std::string cut_value("a");
+
+ TabletColumnPtr tab_col;
+ int32_t length = -1;
+ if constexpr (PType == TYPE_CHAR) {
+ length = 3;
+ tab_col = create_char_key(0, false, length);
+ } else if constexpr (PType == TYPE_VARCHAR) {
+ tab_col = create_varchar_key(0, false);
+ } else {
+ tab_col = create_string_key(0, false);
+ }
+ auto data_type = DataTypeFactory::instance().create_data_type(PType,
false, 0, 0, length);
+
+ std::unique_ptr<ZoneMapIndexWriter> writer;
+ ASSERT_TRUE(ZoneMapIndexWriter::create(data_type, tab_col.get(),
writer).ok());
+ Slice slices[] = {Slice(value), Slice(value)};
+ writer->add_values(slices, 2);
+ ASSERT_TRUE(writer->flush().ok());
+
+ const std::string file_path = kTestDir + "/" + testname;
+ io::FileWriterPtr file_writer;
+ ASSERT_TRUE(_fs->create_file(file_path, &file_writer).ok());
+ ColumnIndexMetaPB index_meta;
+ ASSERT_TRUE(writer->finish(file_writer.get(), &index_meta).ok());
+ ASSERT_TRUE(file_writer->close().ok());
+
+ // The bytes on disk always carry the '\0'; only the parse back can
lose it.
+ const auto& seg_zm_pb = index_meta.zone_map_index().segment_zone_map();
+ EXPECT_EQ(seg_zm_pb.min(), value);
+ EXPECT_EQ(seg_zm_pb.max(), value);
+
+ ZoneMap zone_map;
+ ASSERT_TRUE(ZoneMap::from_proto(seg_zm_pb, data_type, zone_map).ok());
+ ASSERT_FALSE(zone_map.pass_all);
+ const std::string& expected = bound_is_cut ? cut_value : value;
+ EXPECT_EQ(zone_map.min_value.template get<PType>(), expected);
+ EXPECT_EQ(zone_map.max_value.template get<PType>(), expected);
+
+ if (bound_is_cut) {
+ return;
+ }
+
+ // The page holds only 'a\0b', so every predicate below has to keep
the page.
+ const auto a = Field::create_field<PType>(cut_value);
+ ComparisonPredicateBase<PType, PredicateType::GT> gt(0, "", a);
+ EXPECT_TRUE(gt.evaluate_and(zone_map));
+ ComparisonPredicateBase<PType, PredicateType::NE> ne(0, "", a);
+ EXPECT_TRUE(ne.evaluate_and(zone_map));
+ ComparisonPredicateBase<PType, PredicateType::EQ> eq(0, "",
+
Field::create_field<PType>(value));
+ EXPECT_TRUE(eq.evaluate_and(zone_map));
+
+ // ... and 'a\0b' <= 'a' matches nothing, so this one may drop it.
+ ComparisonPredicateBase<PType, PredicateType::LE> le(0, "", a);
+ EXPECT_FALSE(le.evaluate_and(zone_map));
+ }
+
io::FileSystemSPtr _fs;
};
@@ -1443,5 +1511,16 @@ TEST_F(ColumnZoneMapTest,
AllNullPageAfterMaxLenStringPage_NoSegmentMaxDoubleInc
EXPECT_EQ(static_cast<unsigned char>(seg_zm.max().back()),
static_cast<unsigned char>('y'));
}
+// Regression test: a comparison predicate on a STRING / VARCHAR column
+// whose values hold an embedded '\0' silently lost or gained rows, because
the zone map
+// bound was parsed back with C string semantics and stopped at that '\0'.
+TEST_F(ColumnZoneMapTest, EmbeddedNulKeepsStringBound) {
+ test_embedded_nul_bound<TYPE_STRING>("embedded_nul_string",
/*bound_is_cut=*/false);
+ test_embedded_nul_bound<TYPE_VARCHAR>("embedded_nul_varchar",
/*bound_is_cut=*/false);
+ // CHAR pads with '\0' on write and cuts at the first '\0' on read, so its
bound is
+ // cut the same way and stays comparable with the rows the page returns.
+ test_embedded_nul_bound<TYPE_CHAR>("embedded_nul_char",
/*bound_is_cut=*/true);
+}
+
} // namespace segment_v2
} // namespace doris
diff --git
a/regression-test/data/datatype_p0/string/test_string_embedded_nul_zonemap.out
b/regression-test/data/datatype_p0/string/test_string_embedded_nul_zonemap.out
new file mode 100644
index 00000000000..99449dcb8cb
--- /dev/null
+++
b/regression-test/data/datatype_p0/string/test_string_embedded_nul_zonemap.out
@@ -0,0 +1,73 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !inside_string_data --
+32 3 3 610062 610062
+
+-- !inside_string_oracle --
+32 32 0
+
+-- !inside_string_gt --
+32
+
+-- !inside_string_ne --
+32
+
+-- !inside_string_le --
+0
+
+-- !inside_string_minmax --
+610062 610062
+
+-- !inside_varchar_data --
+32 3 3 610062 610062
+
+-- !inside_varchar_oracle --
+32 32 0
+
+-- !inside_varchar_gt --
+32
+
+-- !inside_varchar_ne --
+32
+
+-- !inside_varchar_le --
+0
+
+-- !inside_varchar_minmax --
+610062 610062
+
+-- !trailing_string_data --
+32 2 2 6100 6100
+
+-- !trailing_string_oracle --
+32 32 0
+
+-- !trailing_string_gt --
+32
+
+-- !trailing_string_ne --
+32
+
+-- !trailing_string_le --
+0
+
+-- !trailing_string_minmax --
+6100 6100
+
+-- !leading_string_data --
+32 2 2 0061 0061
+
+-- !leading_string_oracle --
+0 32 32
+
+-- !leading_string_gt --
+0
+
+-- !leading_string_ne --
+32
+
+-- !leading_string_le --
+32
+
+-- !leading_string_minmax --
+0061 0061
+
diff --git
a/regression-test/suites/datatype_p0/string/test_string_embedded_nul_zonemap.groovy
b/regression-test/suites/datatype_p0/string/test_string_embedded_nul_zonemap.groovy
new file mode 100644
index 00000000000..aad8036e19f
--- /dev/null
+++
b/regression-test/suites/datatype_p0/string/test_string_embedded_nul_zonemap.groovy
@@ -0,0 +1,79 @@
+// 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.
+
+// A STRING / VARCHAR value may hold a 0x00 byte. The zone map bound used to be
+// parsed back with C string semantics and stopped at that byte, so a
comparison pushed into
+// the scan read a bound the data never held and dropped pages that hold
matching rows.
+//
+// Each table below holds 32 copies of one value, so the segment min and max
are that value.
+// The SUM(...) row is the per-row answer, which never goes through the zone
map; the three
+// COUNT(*) rows run the same predicates through the scan and have to agree
with it.
+suite("test_string_embedded_nul_zonemap") {
+ def load_one_value = { String table, String type, String hex ->
+ sql "DROP TABLE IF EXISTS ${table}"
+ sql """
+ CREATE TABLE ${table} (
+ id INT,
+ s ${type}
+ )
+ DUPLICATE KEY(id)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "disable_auto_compaction" = "true"
+ )
+ """
+ def rows = (1..32).collect { "(${it}, UNHEX('${hex}'))" }.join(", ")
+ sql "INSERT INTO ${table} VALUES ${rows}"
+ }
+
+ // 'a' 0x00 'b' in a STRING column -- the 0x00 sits in the middle.
+ load_one_value("nul_inside_string", "STRING", "610062")
+ qt_inside_string_data "SELECT COUNT(*), MIN(LENGTH(s)), MAX(LENGTH(s)),
MIN(HEX(s)), MAX(HEX(s)) FROM nul_inside_string"
+ qt_inside_string_oracle "SELECT SUM(CAST(s > 'a' AS INT)), SUM(CAST(s !=
'a' AS INT)), SUM(CAST(s <= 'a' AS INT)) FROM nul_inside_string"
+ qt_inside_string_gt "SELECT COUNT(*) FROM nul_inside_string WHERE s > 'a'"
+ qt_inside_string_ne "SELECT COUNT(*) FROM nul_inside_string WHERE s != 'a'"
+ qt_inside_string_le "SELECT COUNT(*) FROM nul_inside_string WHERE s <= 'a'"
+ qt_inside_string_minmax "SELECT HEX(MIN(s)), HEX(MAX(s)) FROM
nul_inside_string"
+
+ // The same value in a VARCHAR column, which shares the zone map bound
path.
+ load_one_value("nul_inside_varchar", "VARCHAR(20)", "610062")
+ qt_inside_varchar_data "SELECT COUNT(*), MIN(LENGTH(s)), MAX(LENGTH(s)),
MIN(HEX(s)), MAX(HEX(s)) FROM nul_inside_varchar"
+ qt_inside_varchar_oracle "SELECT SUM(CAST(s > 'a' AS INT)), SUM(CAST(s !=
'a' AS INT)), SUM(CAST(s <= 'a' AS INT)) FROM nul_inside_varchar"
+ qt_inside_varchar_gt "SELECT COUNT(*) FROM nul_inside_varchar WHERE s >
'a'"
+ qt_inside_varchar_ne "SELECT COUNT(*) FROM nul_inside_varchar WHERE s !=
'a'"
+ qt_inside_varchar_le "SELECT COUNT(*) FROM nul_inside_varchar WHERE s <=
'a'"
+ qt_inside_varchar_minmax "SELECT HEX(MIN(s)), HEX(MAX(s)) FROM
nul_inside_varchar"
+
+ // 'a' 0x00 -- the 0x00 is the last byte, so the whole bound used to
shrink to 'a'.
+ load_one_value("nul_trailing_string", "STRING", "6100")
+ qt_trailing_string_data "SELECT COUNT(*), MIN(LENGTH(s)), MAX(LENGTH(s)),
MIN(HEX(s)), MAX(HEX(s)) FROM nul_trailing_string"
+ qt_trailing_string_oracle "SELECT SUM(CAST(s > 'a' AS INT)), SUM(CAST(s !=
'a' AS INT)), SUM(CAST(s <= 'a' AS INT)) FROM nul_trailing_string"
+ qt_trailing_string_gt "SELECT COUNT(*) FROM nul_trailing_string WHERE s >
'a'"
+ qt_trailing_string_ne "SELECT COUNT(*) FROM nul_trailing_string WHERE s !=
'a'"
+ qt_trailing_string_le "SELECT COUNT(*) FROM nul_trailing_string WHERE s <=
'a'"
+ qt_trailing_string_minmax "SELECT HEX(MIN(s)), HEX(MAX(s)) FROM
nul_trailing_string"
+
+ // 0x00 'a' -- the 0x00 comes first, so the bound used to shrink to the
empty string.
+ load_one_value("nul_leading_string", "STRING", "0061")
+ qt_leading_string_data "SELECT COUNT(*), MIN(LENGTH(s)), MAX(LENGTH(s)),
MIN(HEX(s)), MAX(HEX(s)) FROM nul_leading_string"
+ qt_leading_string_oracle "SELECT SUM(CAST(s > 'a' AS INT)), SUM(CAST(s !=
'a' AS INT)), SUM(CAST(s <= 'a' AS INT)) FROM nul_leading_string"
+ qt_leading_string_gt "SELECT COUNT(*) FROM nul_leading_string WHERE s >
'a'"
+ qt_leading_string_ne "SELECT COUNT(*) FROM nul_leading_string WHERE s !=
'a'"
+ qt_leading_string_le "SELECT COUNT(*) FROM nul_leading_string WHERE s <=
'a'"
+ qt_leading_string_minmax "SELECT HEX(MIN(s)), HEX(MAX(s)) FROM
nul_leading_string"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
