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 c313df16082 branch-4.1: [fix](zonemap) Treat reversed zone map bounds
as invalid #67431 (#67550)
c313df16082 is described below
commit c313df16082278a1b7eff6181e089edd43329d3b
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Sep 7 09:17:04 2026 +0800
branch-4.1: [fix](zonemap) Treat reversed zone map bounds as invalid #67431
(#67550)
Cherry-picked from #67431
Co-authored-by: Chenyang Sun <[email protected]>
---
be/src/storage/index/zone_map/zone_map_index.cpp | 36 ++++-
be/test/storage/segment/zone_map_index_test.cpp | 181 +++++++++++++++++++++++
2 files changed, 209 insertions(+), 8 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 5504b4916c9..ef4f77fe97f 100644
--- a/be/src/storage/index/zone_map/zone_map_index.cpp
+++ b/be/src/storage/index/zone_map/zone_map_index.cpp
@@ -46,6 +46,21 @@ namespace doris {
struct uint24_t;
namespace segment_v2 {
+namespace {
+
+// Only FLOAT and DOUBLE can come out reversed: any value of any other type
moves both bounds.
+bool is_reversed(const Field& min_value, const Field& max_value, FieldType
field_type) {
+ if (FieldType::OLAP_FIELD_TYPE_FLOAT == field_type) {
+ return min_value.get<TYPE_FLOAT>() > max_value.get<TYPE_FLOAT>();
+ }
+ if (FieldType::OLAP_FIELD_TYPE_DOUBLE == field_type) {
+ return min_value.get<TYPE_DOUBLE>() > max_value.get<TYPE_DOUBLE>();
+ }
+ return false;
+}
+
+} // namespace
+
Status ZoneMap::from_proto(const ZoneMapPB& zone_map, const DataTypePtr&
data_type,
ZoneMap& zone_map_info) {
zone_map_info.has_null = zone_map.has_null();
@@ -66,6 +81,19 @@ Status ZoneMap::from_proto(const ZoneMapPB& zone_map, const
DataTypePtr& data_ty
auto field_type = data_type->get_storage_field_type();
// 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) {
+ parse_bound(zone_map.min(), zone_map_info.min_value);
+ parse_bound(zone_map.max(), zone_map_info.max_value);
+ }
+
+ // NaN and infinity only set the flags below, never min/max, so a page
holding nothing
+ // else leaves both at the values add_values() starts from: min =
DBL_MAX and
+ // max = -DBL_MAX, neither of which is a value in the page.
+ if (!zone_map_info.pass_all &&
+ is_reversed(zone_map_info.min_value, zone_map_info.max_value,
field_type)) {
+ zone_map_info.pass_all = true;
+ }
+
if (zone_map.has_negative_inf()) {
if (FieldType::OLAP_FIELD_TYPE_FLOAT == field_type) {
static auto constexpr float_neg_inf =
-std::numeric_limits<float>::infinity();
@@ -76,10 +104,6 @@ Status ZoneMap::from_proto(const ZoneMapPB& zone_map, const
DataTypePtr& data_ty
} else {
return Status::InternalError("invalid zone map with negative
Infinity");
}
- } else {
- if (!zone_map_info.pass_all) {
- parse_bound(zone_map.min(), zone_map_info.min_value);
- }
}
if (zone_map.has_nan()) {
@@ -102,10 +126,6 @@ Status ZoneMap::from_proto(const ZoneMapPB& zone_map,
const DataTypePtr& data_ty
} else {
return Status::InternalError("invalid zone map with positive
Infinity");
}
- } else {
- if (!zone_map_info.pass_all) {
- parse_bound(zone_map.max(), zone_map_info.max_value);
- }
}
}
return Status::OK();
diff --git a/be/test/storage/segment/zone_map_index_test.cpp
b/be/test/storage/segment/zone_map_index_test.cpp
index d79f0f225f0..17d61ef693d 100644
--- a/be/test/storage/segment/zone_map_index_test.cpp
+++ b/be/test/storage/segment/zone_map_index_test.cpp
@@ -17,6 +17,7 @@
#include "storage/index/zone_map/zone_map_index.h"
+#include <fmt/format.h>
#include <gtest/gtest-message.h>
#include <gtest/gtest-test-part.h>
@@ -24,7 +25,9 @@
#include <limits>
#include <memory>
#include <string>
+#include <vector>
+#include "common/config.h"
#include "core/data_type/data_type_factory.hpp"
#include "core/data_type/define_primitive_type.h"
#include "core/decimal12.h"
@@ -981,6 +984,184 @@ TEST_F(ColumnZoneMapTest,
LegacyUnparsableDoubleBoundDegradesToPassAll) {
}
}
+// Every value written to a FLOAT or DOUBLE zone map is one of seven shapes,
and only an ordinary
+// finite value moves the recorded bounds -- NaN and infinity go to the flags
instead, and
+// DBL_MAX/-DBL_MAX happen to be the very values the bounds start from. Walk
every non-empty subset
+// of the seven and check the one property a zone map has to hold: either it
says it is unusable,
+// or its bounds cover every value the page holds, so nothing it contains is
ever pruned away.
+template <PrimitiveType Type>
+void test_every_value_combination(const std::string& test_dir) {
+ using CppType = typename PrimitiveTypeTraits<Type>::CppType;
+ constexpr bool is_double = Type == TYPE_DOUBLE;
+ const std::vector<std::pair<const char*, CppType>> candidates = {
+ {"NaN", std::numeric_limits<CppType>::quiet_NaN()},
+ {"+inf", std::numeric_limits<CppType>::infinity()},
+ {"-inf", -std::numeric_limits<CppType>::infinity()},
+ {"max", std::numeric_limits<CppType>::max()},
+ {"lowest", std::numeric_limits<CppType>::lowest()},
+ {"1.5", static_cast<CppType>(1.5)},
+ {"20.5", static_cast<CppType>(20.5)}};
+
+ // Doris orders NaN above every number, so rank it beyond infinity to
compare bounds the way
+ // the scan does.
+ auto rank = [](CppType v) {
+ return std::isnan(v) ? std::numeric_limits<double>::infinity() :
static_cast<double>(v);
+ };
+ auto covers = [&](CppType low, CppType high, CppType v) {
+ if (std::isnan(v)) {
+ return std::isnan(high);
+ }
+ return (std::isnan(low) ? false : rank(low) <= rank(v)) &&
+ (std::isnan(high) ? true : rank(v) <= rank(high));
+ };
+
+ auto fs = io::global_local_filesystem();
+ auto column = create_float_column < is_double ?
FieldType::OLAP_FIELD_TYPE_DOUBLE
+ :
FieldType::OLAP_FIELD_TYPE_FLOAT > (0, true);
+ const TabletColumn* field = &(*column);
+ auto data_type_ptr = DataTypeFactory::instance().create_data_type(Type,
false);
+
+ size_t pass_all_count = 0;
+ for (uint32_t mask = 1; mask < (1u << candidates.size()); ++mask) {
+ std::vector<CppType> values;
+ std::string label;
+ for (size_t i = 0; i < candidates.size(); ++i) {
+ if (mask & (1u << i)) {
+ values.push_back(candidates[i].second);
+ label += (label.empty() ? "" : ",");
+ label += candidates[i].first;
+ }
+ }
+
+ std::string filename =
+ fmt::format("{}/every_value_{}_{}", test_dir, is_double ?
"double" : "float", mask);
+ std::unique_ptr<ZoneMapIndexWriter> builder(nullptr);
+ static_cast<void>(ZoneMapIndexWriter::create(data_type_ptr, field,
builder));
+ // Stay above zone_map_row_num_threshold so the writer does not
invalidate the page for
+ // being small, which would hide what is being tested here.
+ const size_t rows = config::zone_map_row_num_threshold + 5;
+ for (size_t i = 0; i < rows; ++i) {
+ CppType value = values[i % values.size()];
+ builder->add_values((const uint8_t*)&value, 1);
+ }
+ ASSERT_TRUE(builder->flush().ok()) << label;
+
+ ColumnIndexMetaPB index_meta;
+ {
+ io::FileWriterPtr file_writer;
+ ASSERT_TRUE(fs->create_file(filename, &file_writer).ok()) << label;
+ ASSERT_TRUE(builder->finish(file_writer.get(), &index_meta).ok())
<< label;
+ ASSERT_TRUE(file_writer->close().ok()) << label;
+ }
+
+ ZoneMap zone_map;
+
ASSERT_TRUE(ZoneMap::from_proto(index_meta.zone_map_index().segment_zone_map(),
+ data_type_ptr, zone_map)
+ .ok())
+ << label;
+ ASSERT_TRUE(zone_map.has_not_null) << label;
+
+ if (zone_map.pass_all) {
+ ++pass_all_count;
+ continue;
+ }
+ auto low = zone_map.min_value.get<Type>();
+ auto high = zone_map.max_value.get<Type>();
+ for (auto value : values) {
+ EXPECT_TRUE(covers(low, high, value))
+ << "values {" << label << "} left bounds that do not cover
" << value;
+ }
+ }
+
+ // A page reports no bounds exactly when it held no finite value, which is
every non-empty
+ // subset of NaN, +inf and -inf: seven of the 127.
+ EXPECT_EQ(7, pass_all_count) << (is_double ? "double" : "float");
+}
+
+TEST_F(ColumnZoneMapTest, EveryValueCombinationDouble) {
+ test_every_value_combination<TYPE_DOUBLE>(kTestDir);
+}
+
+TEST_F(ColumnZoneMapTest, EveryValueCombinationFloat) {
+ test_every_value_combination<TYPE_FLOAT>(kTestDir);
+}
+
+TEST_F(ColumnZoneMapTest, ReversedBoundsDegradeToPassAll) {
+ auto make_zone_map = [](const std::string& min, const std::string& max) {
+ ZoneMapPB pb;
+ pb.set_min(min);
+ pb.set_max(max);
+ pb.set_has_null(false);
+ pb.set_has_not_null(true);
+ pb.set_pass_all(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.
+ const std::string double_lowest = "-1.7976931348623157e+308";
+ const std::string double_highest = "1.7976931348623157e+308";
+ const std::string float_lowest = "-3.4028235e+38";
+ const std::string float_highest = "3.4028235e+38";
+
+ for (bool nullable : {false, true}) {
+ for (auto type : {TYPE_DOUBLE, TYPE_FLOAT}) {
+ const bool is_double = type == TYPE_DOUBLE;
+ auto data_type =
DataTypeFactory::instance().create_data_type(type, nullable);
+ const auto& lowest = is_double ? double_lowest : float_lowest;
+ const auto& highest = is_double ? double_highest : float_highest;
+
+ ZoneMap reversed;
+ ASSERT_TRUE(
+ ZoneMap::from_proto(make_zone_map(highest, lowest),
data_type, reversed).ok());
+ EXPECT_TRUE(reversed.pass_all) << "nullable=" << nullable << ",
double=" << is_double;
+
+ // The same bounds the right way round stay usable.
+ ZoneMap sound;
+ ASSERT_TRUE(ZoneMap::from_proto(make_zone_map(lowest, highest),
data_type, sound).ok());
+ EXPECT_FALSE(sound.pass_all) << "nullable=" << nullable << ",
double=" << is_double;
+ }
+ }
+
+ // 4.0 and later add a flag for what the page held instead, but the bounds
are still the
+ // reversed pair whatever the flags say.
+ auto double_type =
DataTypeFactory::instance().create_data_type(TYPE_DOUBLE, false);
+ for (bool nan : {false, true}) {
+ for (bool pos_inf : {false, true}) {
+ for (bool neg_inf : {false, true}) {
+ auto pb = make_zone_map(double_highest, double_lowest);
+ pb.set_has_nan(nan);
+ pb.set_has_positive_inf(pos_inf);
+ pb.set_has_negative_inf(neg_inf);
+ ZoneMap flagged;
+ ASSERT_TRUE(ZoneMap::from_proto(pb, double_type,
flagged).ok());
+ EXPECT_TRUE(flagged.pass_all)
+ << "nan=" << nan << ", +inf=" << pos_inf << ", -inf="
<< neg_inf;
+ }
+ }
+ }
+
+ // 4.0 and 4.1 wrote bounds with digits10 + 1 digits, so a FLOAT page of
only NaN recorded
+ // 3.402823e+38 rather than FLT_MAX. It parses back finite and no longer
equals the value the
+ // writer starts from -- the reversal survives the lossy round trip where
the value does not.
+ auto truncated = make_zone_map("3.402823e+38", "-3.402823e+38");
+ truncated.set_has_nan(true);
+ ZoneMap from_4_0;
+ ASSERT_TRUE(ZoneMap::from_proto(truncated,
+
DataTypeFactory::instance().create_data_type(TYPE_FLOAT, false),
+ from_4_0)
+ .ok());
+ EXPECT_TRUE(from_4_0.pass_all);
+
+ // A flag on top of bounds that do describe finite values leaves the zone
map usable.
+ auto pb = make_zone_map("1.5", "20.5");
+ pb.set_has_nan(true);
+ ZoneMap partly_nan;
+ ASSERT_TRUE(ZoneMap::from_proto(pb, double_type, partly_nan).ok());
+ EXPECT_FALSE(partly_nan.pass_all);
+ EXPECT_TRUE(std::isnan(partly_nan.max_value.get<TYPE_DOUBLE>()));
+ EXPECT_EQ(partly_nan.min_value.get<TYPE_DOUBLE>(), 1.5);
+}
+
TabletColumnPtr create_timestamptz_column(int32_t id, bool is_nullable) {
auto column = std::make_shared<TabletColumn>();
column->_unique_id = id;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
