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 2709af1d507 branch-4.1: [fix](zonemap) Do not treat a legacy float or
double zone map as NaN-free #67779 (#68102)
2709af1d507 is described below
commit 2709af1d5075b9ce3c23a7a9eae4433b71e17fbb
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 17 21:15:46 2026 +0800
branch-4.1: [fix](zonemap) Do not treat a legacy float or double zone map
as NaN-free #67779 (#68102)
Cherry-picked from #67779
Co-authored-by: YangJie <[email protected]>
---
be/src/storage/index/zone_map/zone_map_index.cpp | 14 +++++
be/test/storage/segment/zone_map_index_test.cpp | 80 +++++++++++++++++++++++-
2 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/be/src/storage/index/zone_map/zone_map_index.cpp
b/be/src/storage/index/zone_map/zone_map_index.cpp
index ef4f77fe97f..2ee6f828d1e 100644
--- a/be/src/storage/index/zone_map/zone_map_index.cpp
+++ b/be/src/storage/index/zone_map/zone_map_index.cpp
@@ -79,6 +79,20 @@ Status ZoneMap::from_proto(const ZoneMapPB& zone_map, const
DataTypePtr& data_ty
};
auto field_type = data_type->get_storage_field_type();
+
+ // has_nan arrived with NaN-aware float/double zone maps, so its absence
means the writer could
+ // not report NaN and the bounds came from a comparison that never selects
one: a hidden NaN
+ // cannot be ruled out. Doris orders NaN above every other value, so `x >
c` can be true for a
+ // row that these bounds say cannot exist. Treat such a zone map as
covering everything instead
+ // of as NaN-free. A zone with no non-null value never received one, so it
has no NaN to hide;
+ // leaving it usable keeps the three ColumnPredicate checks that start
from has_not_null: the
+ // null predicates, each comparison predicate's early return, and the
in-list one's.
+ if ((field_type == FieldType::OLAP_FIELD_TYPE_FLOAT ||
+ field_type == FieldType::OLAP_FIELD_TYPE_DOUBLE) &&
+ zone_map.has_not_null() && !zone_map.has_has_nan()) {
+ zone_map_info.pass_all = true;
+ }
+
// min value and max value are valid if has_not_null is true
if (zone_map.has_not_null()) {
if (!zone_map_info.pass_all) {
diff --git a/be/test/storage/segment/zone_map_index_test.cpp
b/be/test/storage/segment/zone_map_index_test.cpp
index 2ea9503b0b9..36a4883f7c1 100644
--- a/be/test/storage/segment/zone_map_index_test.cpp
+++ b/be/test/storage/segment/zone_map_index_test.cpp
@@ -1014,6 +1014,74 @@ TEST_F(ColumnZoneMapTest, DoubleFiniteExtremesRoundTrip)
{
EXPECT_EQ(pzm.max_value.get<TYPE_DOUBLE>(),
std::numeric_limits<double>::max());
}
+TEST_F(ColumnZoneMapTest, LegacyFloatZoneMapWithoutHasNanDegradesToPassAll) {
+ // A float or double zone map written before has_nan existed says nothing
about NaN, and its
+ // bounds were produced by a comparison that never picks a NaN. Since
Doris sorts NaN above
+ // every other value, trusting such bounds drops NaN rows from `>` and
`>=`.
+ auto legacy_pb = [](double min_value, double max_value) {
+ ZoneMapPB pb;
+ pb.set_min(std::to_string(min_value));
+ pb.set_max(std::to_string(max_value));
+ pb.set_has_null(false);
+ pb.set_has_not_null(true);
+ pb.set_pass_all(false);
+ // Deliberately no set_has_nan / set_has_positive_inf /
set_has_negative_inf.
+ return pb;
+ };
+
+ for (const auto primitive_type : {TYPE_FLOAT, TYPE_DOUBLE}) {
+ for (bool nullable : {false, true}) {
+ auto data_type =
DataTypeFactory::instance().create_data_type(primitive_type, nullable);
+ ZoneMap zm;
+ ASSERT_TRUE(ZoneMap::from_proto(legacy_pb(1.0, 2.0), data_type,
zm).ok());
+ EXPECT_TRUE(zm.pass_all) << "type=" << primitive_type << ",
nullable=" << nullable;
+ // The null flags survive, and so does the pruning that reads only
them:
+ // eval_null_zonemap never consults pass_all. The ColumnPredicate
path in
+ // ColumnReader::_get_filtered_pages does return early on
pass_all, so it loses its
+ // IS NULL pruning for this zone map.
+ EXPECT_TRUE(zm.has_not_null);
+ EXPECT_FALSE(zm.has_null);
+
+ // The same bounds from a writer that does report the flag stay
usable.
+ auto pb = legacy_pb(1.0, 2.0);
+ pb.set_has_nan(false);
+ ZoneMap current;
+ ASSERT_TRUE(ZoneMap::from_proto(pb, data_type, current).ok());
+ EXPECT_FALSE(current.pass_all)
+ << "type=" << primitive_type << ", nullable=" << nullable;
+ if (primitive_type == TYPE_DOUBLE) {
+ EXPECT_EQ(1.0, current.min_value.get<TYPE_DOUBLE>());
+ EXPECT_EQ(2.0, current.max_value.get<TYPE_DOUBLE>());
+ } else {
+ EXPECT_EQ(1.0F, current.min_value.get<TYPE_FLOAT>());
+ EXPECT_EQ(2.0F, current.max_value.get<TYPE_FLOAT>());
+ }
+
+ // A legacy zone with no non-null value never received one, so no
NaN can hide in it.
+ // The bound text is irrelevant here: from_proto only parses it
when has_not_null.
+ auto all_null = legacy_pb(1.0, 2.0);
+ all_null.set_has_null(true);
+ all_null.set_has_not_null(false);
+ ZoneMap all_null_zm;
+ ASSERT_TRUE(ZoneMap::from_proto(all_null, data_type,
all_null_zm).ok());
+ EXPECT_FALSE(all_null_zm.pass_all)
+ << "type=" << primitive_type << ", nullable=" << nullable;
+ }
+ }
+
+ // Non-floating columns never carried the flag and are unaffected.
+ auto int_type = DataTypeFactory::instance().create_data_type(TYPE_INT,
false);
+ ZoneMapPB int_pb;
+ int_pb.set_min("1");
+ int_pb.set_max("9");
+ int_pb.set_has_null(false);
+ int_pb.set_has_not_null(true);
+ int_pb.set_pass_all(false);
+ ZoneMap int_zm;
+ ASSERT_TRUE(ZoneMap::from_proto(int_pb, int_type, int_zm).ok());
+ EXPECT_FALSE(int_zm.pass_all);
+}
+
TEST_F(ColumnZoneMapTest, LegacyUnparsableDoubleBoundDegradesToPassAll) {
auto make_zone_map = [](const std::string& min, const std::string& max) {
ZoneMapPB pb;
@@ -1022,6 +1090,9 @@ TEST_F(ColumnZoneMapTest,
LegacyUnparsableDoubleBoundDegradesToPassAll) {
pb.set_has_null(false);
pb.set_has_not_null(true);
pb.set_pass_all(false);
+ // The current writer always reports this flag; leaving it out would
mark the zone map as
+ // pre-NaN-tracking legacy metadata, which is a different test.
+ pb.set_has_nan(false);
return pb;
};
// 16g renderings of ±DBL_MAX, both of which read back as ∓inf.
@@ -1162,10 +1233,15 @@ TEST_F(ColumnZoneMapTest,
ReversedBoundsDegradeToPassAll) {
pb.set_has_null(false);
pb.set_has_not_null(true);
pb.set_pass_all(false);
+ // The current writer always reports this flag; leaving it out would
mark the zone map as
+ // pre-NaN-tracking legacy metadata, which is a different test.
+ pb.set_has_nan(false);
return pb;
};
- // What a page of only NaN leaves behind before 4.0: bounds that never
moved off the values
- // the writer starts from, and that round-trip exactly, so only the
reversal gives them away.
+ // add_values() starts each call from numeric_limits::max() and
::lowest(), and a page whose
+ // only non-null values are NaN or infinity leaves them there, so the
stored pair comes back
+ // reversed. These strings round-trip exactly, and the reversal is the
first signal either way:
+ // is_reversed runs before the flag overrides.
const std::string double_lowest = "-1.7976931348623157e+308";
const std::string double_highest = "1.7976931348623157e+308";
const std::string float_lowest = "-3.4028235e+38";
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
