github-actions[bot] commented on code in PR #69217:
URL: https://github.com/apache/doris/pull/69217#discussion_r4311571665
##########
be/src/storage/index/zone_map/zone_map_index.cpp:
##########
@@ -48,6 +51,137 @@ struct uint24_t;
namespace segment_v2 {
namespace {
+// Encoding ids are persisted in ZoneMapPB; never change their meaning.
+constexpr uint32_t BINARY_ZONE_MAP_ENCODING = 1;
+
+// Persisted fixed-width types, excluding legacy DATE, DATETIME and DECIMALV2.
Those types
+// retain their text bounds; execution-only TIMEV2 and internal offsets have
no zone map writer.
+template <typename Action>
+bool dispatch_binary_zone_map(PrimitiveType type, Action&& action) {
+ switch (type) {
+#define BINARY_ZONE_MAP_TYPE(TYPE) \
+ case TYPE: \
+ action.template operator()<TYPE>(); \
+ return true;
+ BINARY_ZONE_MAP_TYPE(TYPE_BOOLEAN)
+ BINARY_ZONE_MAP_TYPE(TYPE_TINYINT)
+ BINARY_ZONE_MAP_TYPE(TYPE_SMALLINT)
+ BINARY_ZONE_MAP_TYPE(TYPE_INT)
+ BINARY_ZONE_MAP_TYPE(TYPE_BIGINT)
+ BINARY_ZONE_MAP_TYPE(TYPE_LARGEINT)
+ BINARY_ZONE_MAP_TYPE(TYPE_FLOAT)
+ BINARY_ZONE_MAP_TYPE(TYPE_DOUBLE)
+ BINARY_ZONE_MAP_TYPE(TYPE_DECIMAL32)
+ BINARY_ZONE_MAP_TYPE(TYPE_DECIMAL64)
+ BINARY_ZONE_MAP_TYPE(TYPE_DECIMAL128I)
+ BINARY_ZONE_MAP_TYPE(TYPE_DECIMAL256)
+ BINARY_ZONE_MAP_TYPE(TYPE_DATEV2)
+ BINARY_ZONE_MAP_TYPE(TYPE_DATETIMEV2)
+ BINARY_ZONE_MAP_TYPE(TYPE_TIMESTAMP_NS)
+ BINARY_ZONE_MAP_TYPE(TYPE_TIMESTAMPTZ)
+ BINARY_ZONE_MAP_TYPE(TYPE_IPV4)
+ BINARY_ZONE_MAP_TYPE(TYPE_IPV6)
+ BINARY_ZONE_MAP_TYPE(TYPE_UUID)
+#undef BINARY_ZONE_MAP_TYPE
+ default:
+ return false;
+ }
+}
+
+template <PrimitiveType Type>
+std::string encode_binary_bound(const Field& field) {
+ using Native = typename PrimitiveTypeTraits<Type>::StorageFieldType;
+ Native value;
+ if constexpr (is_decimalv3(Type)) {
+ value = field.get<Type>().value;
+ } else if constexpr (is_date_v2_or_datetime_v2(Type) || Type ==
TYPE_TIMESTAMPTZ ||
+ Type == TYPE_TIMESTAMP_NS) {
+ value = field.get<Type>().to_date_int_val();
+ } else {
+ value = field.get<Type>();
+ }
+ std::string bytes(sizeof(Native), '\0');
+ auto* out = reinterpret_cast<uint8_t*>(bytes.data());
+ if constexpr (sizeof(Native) == 32) {
+ for (size_t i = 0; i < 4; ++i) {
+ encode_fixed64_le(out + i * sizeof(uint64_t), value.items[i]);
+ }
+ } else if constexpr (sizeof(Native) == 16) {
+ encode_fixed128_le(out, std::bit_cast<uint128_t>(value));
+ } else if constexpr (sizeof(Native) == 8) {
+ encode_fixed64_le(out, std::bit_cast<uint64_t>(value));
+ } else if constexpr (sizeof(Native) == 4) {
+ encode_fixed32_le(out, std::bit_cast<uint32_t>(value));
+ } else if constexpr (sizeof(Native) == 2) {
+ encode_fixed16_le(out, std::bit_cast<uint16_t>(value));
+ } else {
+ static_assert(sizeof(Native) == 1);
+ encode_fixed8(out, std::bit_cast<uint8_t>(value));
+ }
+ return bytes;
+}
+
+template <PrimitiveType Type>
+Status decode_binary_bound(const std::string& bytes, Field& field) {
+ using Native = typename PrimitiveTypeTraits<Type>::StorageFieldType;
+ // A truncated/corrupt persisted bound must not reach pruning or metadata
MIN/MAX aggregation.
+ if (bytes.size() != sizeof(Native)) {
+ return Status::Corruption("Invalid binary zone map bound: expected {}
bytes, got {}",
+ sizeof(Native), bytes.size());
+ }
+ const auto* in = reinterpret_cast<const uint8_t*>(bytes.data());
+ Native value;
+ if constexpr (sizeof(Native) == 32) {
+ for (size_t i = 0; i < 4; ++i) {
+ value.items[i] = decode_fixed64_le(in + i * sizeof(uint64_t));
+ }
+ } else if constexpr (sizeof(Native) == 16) {
+ value = std::bit_cast<Native>(decode_fixed128_le(in));
+ } else if constexpr (sizeof(Native) == 8) {
+ value = std::bit_cast<Native>(decode_fixed64_le(in));
+ } else if constexpr (sizeof(Native) == 4) {
+ value = std::bit_cast<Native>(decode_fixed32_le(in));
+ } else if constexpr (sizeof(Native) == 2) {
+ value = std::bit_cast<Native>(decode_fixed16_le(in));
+ } else {
+ static_assert(sizeof(Native) == 1);
+ value = std::bit_cast<Native>(decode_fixed8(in));
+ }
+ field = Field::create_field_from_olap_value<Type>(value);
+ return Status::OK();
+}
+
+Status decode_zone_map_bounds(const ZoneMapPB& zone_map, const DataTypePtr&
data_type,
+ ZoneMap& result) {
+ // A bound that fails to parse makes the zone map invalid: mark it
pass_all so it prunes
+ // nothing, instead of failing the scan that loads it.
+ auto parse_bound = [&](const std::string& bound, Field& value) {
+ if (!data_type->get_serde()->from_zonemap_string(bound, value).ok()) {
+ result.pass_all = true;
+ }
+ };
+
+ if (zone_map.value_encoding() == BINARY_ZONE_MAP_ENCODING) {
+ Status status;
+ const bool supported = dispatch_binary_zone_map(
+ data_type->get_primitive_type(), [&]<PrimitiveType Type>() {
+ status = decode_binary_bound<Type>(zone_map.min(),
result.min_value);
+ if (status.ok()) {
+ status = decode_binary_bound<Type>(zone_map.max(),
result.max_value);
+ }
+ });
+ if (!supported) {
+ return Status::NotSupported("Binary zone map for unsupported type
{}",
Review Comment:
[P2] Decode VARIANT page bounds with the physical leaf type
With `enable_binary_zone_map=true`, a DUP table containing repeated
`{"a":1}` and `{"a":{"b":2}}` rows can persist numeric `a` and `a.b` subcolumns
(use enough rows to retain valid page zone maps). A query such as `WHERE v['a']
IS NULL` or `IS NOT NULL` remains eligible for expression zone-map pruning. In
`_apply_expr_zonemap_to_row_ranges`, `get_column_reader_for_pruning` supplies
the physical scalar leaf's PB, but `get_data_type_of` returns `TYPE_VARIANT`
for the hierarchical prefix `a`. Passing that logical type here makes dispatch
fail and aborts the query with `Binary zone map for unsupported type ...`; the
legacy text path instead marks the range `pass_all`. Decode page bounds with
the physical reader's persisted type and conservatively skip pruning when the
reconstructed logical path is incompatible. Please add binary-enabled mixed
scalar/object VARIANT query coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
