github-actions[bot] commented on code in PR #63389:
URL: https://github.com/apache/doris/pull/63389#discussion_r3296624986
##########
be/src/storage/segment/segment_iterator.cpp:
##########
@@ -3435,6 +3470,99 @@ Status
SegmentIterator::_construct_compound_expr_context() {
return Status::OK();
}
+Status SegmentIterator::_apply_expr_zonemap_to_row_ranges(const
VExprContextSPtrs& conjuncts,
+ rowid_t min_rowid,
+ RowRanges*
row_ranges) {
+ DORIS_CHECK(row_ranges != nullptr);
+ if (!config::enable_expr_zonemap_filter || conjuncts.empty() ||
row_ranges->is_empty()) {
+ return Status::OK();
+ }
+
+ std::unordered_map<int, VExprContextSPtrs> ctxs_by_slot;
+ for (const auto& conjunct : conjuncts) {
+ auto slot_index = single_slot_zonemap_index(conjunct);
+ if (slot_index.has_value()) {
+ ctxs_by_slot[*slot_index].emplace_back(conjunct);
+ }
+ }
+ // Page zone maps are stored per column. Multi-slot expressions need page
alignment across
+ // multiple column readers and are therefore left to segment-level pruning
for now.
+ if (ctxs_by_slot.empty()) {
+ return Status::OK();
+ }
+
+ ColumnIteratorOptions iter_opts {
+ .use_page_cache = _opts.use_page_cache,
+ .file_reader = _file_reader.get(),
+ .stats = _opts.stats,
+ .io_ctx = _opts.io_ctx,
+ };
+ for (const auto& [slot_index, slot_conjuncts] : ctxs_by_slot) {
+ if (cast_set<size_t>(slot_index) >= _schema->num_column_ids()) {
+ continue;
+ }
+ const auto cid = _schema->column_id(cast_set<size_t>(slot_index));
+ if (!_segment->can_apply_predicate_safely(cid, *_schema,
+
_opts.target_cast_type_for_variants, _opts)) {
+ continue;
+ }
+ const auto* field = _schema->column(cid);
+ if (field == nullptr) {
+ continue;
+ }
+ std::shared_ptr<ColumnReader> reader;
+ Status st = _segment->get_column_reader(*field, &reader, _opts.stats);
+ if (st.is<ErrorCode::NOT_FOUND>()) {
+ continue;
+ }
+ RETURN_IF_ERROR(st);
+ if (reader == nullptr || !reader->has_zone_map()) {
+ continue;
+ }
+ const std::vector<ZoneMapPB>* page_zone_maps = nullptr;
+ RETURN_IF_ERROR(reader->get_page_zone_maps(iter_opts,
&page_zone_maps));
Review Comment:
This now loads page zonemaps for every single-slot common expression even
when the expression cannot use zonemap evaluation.
`single_slot_zonemap_index()` only checks slot count, while unsupported roots
return `kUnsupported` and `VExprContext::evaluate_zonemap_filter()` turns that
into `kMayMatch`, so a predicate like `abs(k) = 1` will read all page zonemaps
and page row ranges and then keep every page. Because
`enable_expr_zonemap_filter` defaults to true and common expr pushdown accepts
arbitrary slot-based expressions, this is a storage hot-path regression. Please
filter the conjunct list to expressions that have a supported zonemap evaluator
before calling `get_page_zone_maps()`, or otherwise cache that capability so
unsupported expressions skip this page path entirely.
##########
be/src/exprs/expr_zonemap_filter.cpp:
##########
@@ -0,0 +1,425 @@
+// 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.
+
+#include "exprs/expr_zonemap_filter.h"
+
+#include <algorithm>
+#include <compare>
+#include <limits>
+#include <string>
+
+#include "core/column/column.h"
+#include "core/data_type/data_type_nullable.h"
+#include "exprs/hybrid_set.h"
+#include "exprs/vdirect_in_predicate.h"
+#include "exprs/vexpr.h"
+#include "exprs/vliteral.h"
+#include "exprs/vslot_ref.h"
+
+namespace doris::expr_zonemap {
+namespace {
+
+constexpr size_t kInZoneMapPointCheckThreshold = 64;
+
+VExprSPtr unwrap_identity_cast(const VExprSPtr& expr) {
+ if (expr->node_type() != TExprNodeType::CAST_EXPR ||
expr->get_num_children() != 1) {
+ return expr;
+ }
+ auto child = expr->get_child(0);
+ if
(!remove_nullable(expr->data_type())->equals(*remove_nullable(child->data_type())))
{
+ return expr;
+ }
+ return unwrap_identity_cast(child);
+}
+
+std::optional<Field> field_from_literal_expr(const VExprSPtr& expr,
DataTypePtr* literal_type) {
+ auto unwrapped = unwrap_identity_cast(expr);
+ auto literal = std::dynamic_pointer_cast<VLiteral>(unwrapped);
+ if (literal == nullptr) {
+ return std::nullopt;
+ }
+ Field field;
+ literal->get_column_ptr()->get(0, field);
+ *literal_type = literal->get_data_type();
+ return field;
+}
+
+std::optional<int> slot_index_from_expr(const VExprSPtr& expr) {
+ auto unwrapped = unwrap_identity_cast(expr);
+ auto slot = std::dynamic_pointer_cast<VSlotRef>(unwrapped);
+ if (slot == nullptr) {
+ return std::nullopt;
+ }
+ return slot->column_id();
+}
+
+bool field_types_compatible(PrimitiveType lhs, PrimitiveType rhs) {
+ return lhs == rhs || (is_string_type(lhs) && is_string_type(rhs));
+}
+
+bool field_less(const Field& lhs, const Field& rhs) {
+ return (lhs <=> rhs) == std::strong_ordering::less;
+}
+
+bool field_less_equal(const Field& lhs, const Field& rhs) {
+ const auto cmp = lhs <=> rhs;
+ return cmp == std::strong_ordering::less || cmp ==
std::strong_ordering::equal;
+}
+
+bool field_greater(const Field& lhs, const Field& rhs) {
+ return (lhs <=> rhs) == std::strong_ordering::greater;
+}
+
+bool field_greater_equal(const Field& lhs, const Field& rhs) {
+ const auto cmp = lhs <=> rhs;
+ return cmp == std::strong_ordering::greater || cmp ==
std::strong_ordering::equal;
+}
+
+ZoneMapFilterResult unsupported(ZoneMapEvalContext const& ctx) {
+ ++ctx.stats.unsupported_expr_count;
+ return ZoneMapFilterResult::kUnsupported;
+}
+
+ComparisonOp symmetric_op(ComparisonOp op) {
+ switch (op) {
+ case ComparisonOp::EQ:
+ case ComparisonOp::NE:
+ return op;
+ case ComparisonOp::LT:
+ return ComparisonOp::GT;
+ case ComparisonOp::LE:
+ return ComparisonOp::GE;
+ case ComparisonOp::GT:
+ return ComparisonOp::LT;
+ case ComparisonOp::GE:
+ return ComparisonOp::LE;
+ }
+ __builtin_unreachable();
+}
+
+bool value_in_range(const Field& value, const Field& min_value, const Field&
max_value) {
+ return field_greater_equal(value, min_value) && field_less_equal(value,
max_value);
+}
+
+std::optional<std::string> next_prefix(std::string_view prefix) {
+ // ZoneMap string bounds are compared by bytewise Field ordering. For
starts_with(s, p),
+ // the safe upper bound is the next byte string after p: p <= s <
next_prefix(p).
+ std::string upper(prefix);
+ for (auto i = static_cast<int64_t>(upper.size()) - 1; i >= 0; --i) {
+ auto byte = static_cast<unsigned char>(upper[i]);
+ if (byte != std::numeric_limits<unsigned char>::max()) {
+ upper[i] = static_cast<char>(byte + 1);
+ upper.resize(i + 1);
+ return upper;
+ }
+ }
+ return std::nullopt;
+}
+
+Field string_field(std::string_view value) {
+ return Field::create_field<TYPE_STRING>(std::string(value));
+}
+
+} // namespace
+
+std::optional<SlotLiteral> extract_slot_and_literal(const VExprSPtrs& args) {
+ if (args.size() != 2) {
+ return std::nullopt;
+ }
+
+ if (auto slot_index = slot_index_from_expr(args[0]);
slot_index.has_value()) {
+ DataTypePtr literal_type;
+ auto literal = field_from_literal_expr(args[1], &literal_type);
+ if (!literal.has_value()) {
+ return std::nullopt;
+ }
+ return SlotLiteral {.slot_index = *slot_index,
+ .literal = std::move(*literal),
+ .literal_type = std::move(literal_type),
+ .literal_on_left = false};
+ }
+
+ if (auto slot_index = slot_index_from_expr(args[1]);
slot_index.has_value()) {
+ DataTypePtr literal_type;
+ auto literal = field_from_literal_expr(args[0], &literal_type);
+ if (!literal.has_value()) {
+ return std::nullopt;
+ }
+ return SlotLiteral {.slot_index = *slot_index,
+ .literal = std::move(*literal),
+ .literal_type = std::move(literal_type),
+ .literal_on_left = true};
+ }
+
+ return std::nullopt;
+}
+
+bool check_type_compatible(const ZoneMapEvalContext& ctx, int slot_index,
+ const DataTypePtr& literal_type) {
+ const auto* slot_type = ctx.data_type(slot_index);
+ if (slot_type == nullptr || *slot_type == nullptr || literal_type ==
nullptr) {
+ return false;
+ }
+ if (!remove_nullable(*slot_type)->equals(*remove_nullable(literal_type))) {
+ ++ctx.stats.type_mismatch_count;
+ return false;
+ }
+ return true;
+}
+
+const segment_v2::ZoneMap* fetch_zone_map(const ZoneMapEvalContext& ctx, int
slot_index) {
+ auto zone_map = ctx.zone_map(slot_index);
+ if (zone_map == nullptr) {
+ ++ctx.stats.unsupported_expr_count;
+ }
+ return zone_map;
+}
+
+bool range_stats_usable_for_zonemap(const segment_v2::ZoneMap& zone_map,
+ const DataTypePtr& data_type) {
+ if (zone_map.pass_all || zone_map.has_nan || zone_map.has_positive_inf ||
+ zone_map.has_negative_inf) {
+ return false;
+ }
+ if (data_type == nullptr) {
+ return false;
+ }
+ auto primitive_type = remove_nullable(data_type)->get_primitive_type();
+ if (primitive_type == TYPE_CHAR) {
+ return false;
+ }
+ return field_types_compatible(zone_map.min_value.get_type(),
primitive_type) &&
+ field_types_compatible(zone_map.max_value.get_type(),
primitive_type);
+}
+
+ZoneMapFilterResult eval_comparison_zonemap(const ZoneMapEvalContext& ctx,
+ const VExprSPtrs& arguments,
ComparisonOp op) {
+ auto slot_literal = extract_slot_and_literal(arguments);
+ if (!slot_literal.has_value()) {
+ return unsupported(ctx);
+ }
+ if (slot_literal->literal.is_null()) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ if (!check_type_compatible(ctx, slot_literal->slot_index,
slot_literal->literal_type)) {
+ return unsupported(ctx);
+ }
+ auto zone_map_ref = fetch_zone_map(ctx, slot_literal->slot_index);
+ if (zone_map_ref == nullptr) {
+ return ZoneMapFilterResult::kUnsupported;
+ }
+ const auto& zone_map = *zone_map_ref;
+ if (!zone_map.has_not_null) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ const auto* slot_type = ctx.data_type(slot_literal->slot_index);
+ if (!range_stats_usable_for_zonemap(zone_map, *slot_type)) {
+ return unsupported(ctx);
+ }
+
+ const auto effective_op = slot_literal->literal_on_left ? symmetric_op(op)
: op;
+ const auto& literal = slot_literal->literal;
+ switch (effective_op) {
+ case ComparisonOp::EQ:
+ return field_less(literal, zone_map.min_value) ||
field_greater(literal, zone_map.max_value)
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ case ComparisonOp::NE:
+ return zone_map.min_value == literal && zone_map.max_value == literal
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ case ComparisonOp::LT:
+ return field_greater_equal(zone_map.min_value, literal) ?
ZoneMapFilterResult::kNoMatch
+ :
ZoneMapFilterResult::kMayMatch;
+ case ComparisonOp::LE:
+ return field_greater(zone_map.min_value, literal) ?
ZoneMapFilterResult::kNoMatch
+ :
ZoneMapFilterResult::kMayMatch;
+ case ComparisonOp::GT:
+ return field_less_equal(zone_map.max_value, literal) ?
ZoneMapFilterResult::kNoMatch
+ :
ZoneMapFilterResult::kMayMatch;
+ case ComparisonOp::GE:
+ return field_less(zone_map.max_value, literal) ?
ZoneMapFilterResult::kNoMatch
+ :
ZoneMapFilterResult::kMayMatch;
+ }
+ __builtin_unreachable();
+}
+
+ZoneMapFilterResult eval_null_zonemap(const ZoneMapEvalContext& ctx, const
VExprSPtrs& arguments,
+ bool is_null) {
+ if (arguments.size() != 1) {
+ return unsupported(ctx);
+ }
+ auto slot_index = slot_index_from_expr(arguments[0]);
+ if (!slot_index.has_value()) {
+ return unsupported(ctx);
+ }
+ auto zone_map_ref = fetch_zone_map(ctx, *slot_index);
+ if (zone_map_ref == nullptr) {
+ return ZoneMapFilterResult::kUnsupported;
+ }
+ const auto& zone_map = *zone_map_ref;
+ if (is_null) {
+ return zone_map.has_null ? ZoneMapFilterResult::kMayMatch :
ZoneMapFilterResult::kNoMatch;
+ }
+ return zone_map.has_not_null ? ZoneMapFilterResult::kMayMatch :
ZoneMapFilterResult::kNoMatch;
+}
+
+ZoneMapFilterResult eval_starts_with_zonemap(const ZoneMapEvalContext& ctx,
+ const VExprSPtrs& arguments) {
+ auto slot_literal = extract_slot_and_literal(arguments);
+ if (!slot_literal.has_value() || slot_literal->literal_on_left) {
+ return unsupported(ctx);
+ }
+ if (slot_literal->literal.is_null()) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ const auto* slot_type = ctx.data_type(slot_literal->slot_index);
+ if (slot_type == nullptr || *slot_type == nullptr ||
slot_literal->literal_type == nullptr) {
+ return unsupported(ctx);
+ }
+ if (!is_string_type(remove_nullable(*slot_type)->get_primitive_type()) ||
+
!is_string_type(remove_nullable(slot_literal->literal_type)->get_primitive_type()))
{
+ ++ctx.stats.type_mismatch_count;
+ return unsupported(ctx);
+ }
+ auto zone_map_ref = fetch_zone_map(ctx, slot_literal->slot_index);
+ if (zone_map_ref == nullptr) {
+ return ZoneMapFilterResult::kUnsupported;
+ }
+ const auto& zone_map = *zone_map_ref;
+ if (!zone_map.has_not_null) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ if (!range_stats_usable_for_zonemap(zone_map, *slot_type)) {
+ return unsupported(ctx);
+ }
+
+ const auto prefix = slot_literal->literal.as_string_view();
+ if (prefix.empty()) {
+ return ZoneMapFilterResult::kMayMatch;
+ }
+ auto lower = string_field(prefix);
+ if (field_less(zone_map.max_value, lower)) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ auto upper_prefix = next_prefix(prefix);
+ if (upper_prefix.has_value() && !field_less(zone_map.min_value,
string_field(*upper_prefix))) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ return ZoneMapFilterResult::kMayMatch;
+}
+
+Status materialize_in_filter_for_zonemap(HybridSetBase& filter, const
DataTypePtr& data_type,
+ std::vector<Field>& values, Field&
min_value,
+ Field& max_value) {
+ values.clear();
+ auto* iterator = filter.begin();
+ while (iterator->has_next()) {
+ const void* value = iterator->get_value();
+ if (value != nullptr) {
+ TExprNode literal_node = create_texpr_node_from_hybrid_set_value(
+ value, remove_nullable(data_type)->get_primitive_type(),
+ remove_nullable(data_type)->get_precision(),
+ remove_nullable(data_type)->get_scale());
+ auto literal = VLiteral::create_shared(literal_node);
+ Field field;
+ literal->get_column_ptr()->get(0, field);
+ values.emplace_back(std::move(field));
+ }
+ iterator->next();
+ }
+ if (values.empty()) {
+ return Status::OK();
+ }
+ auto minmax = std::minmax_element(
+ values.begin(), values.end(),
+ [](const Field& lhs, const Field& rhs) { return field_less(lhs,
rhs); });
+ min_value = *minmax.first;
+ max_value = *minmax.second;
+ return Status::OK();
+}
+
+ZoneMapFilterResult eval_in_zonemap(const ZoneMapEvalContext& ctx, const
VExprSPtr& slot_expr,
+ bool is_not_in, const std::vector<Field>&
values,
+ const Field& min_value, const Field&
max_value) {
+ if (values.empty()) {
+ return is_not_in ? ZoneMapFilterResult::kMayMatch :
ZoneMapFilterResult::kNoMatch;
+ }
+ auto slot_index = slot_index_from_expr(slot_expr);
+ if (!slot_index.has_value()) {
+ return unsupported(ctx);
+ }
+ auto* slot_type = ctx.data_type(*slot_index);
+ if (slot_type == nullptr || *slot_type == nullptr) {
+ return unsupported(ctx);
+ }
+ auto zone_map_ref = fetch_zone_map(ctx, *slot_index);
+ if (zone_map_ref == nullptr) {
+ return ZoneMapFilterResult::kUnsupported;
+ }
+ const auto& zone_map = *zone_map_ref;
+ if (!zone_map.has_not_null) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ if (!range_stats_usable_for_zonemap(zone_map, *slot_type)) {
+ return unsupported(ctx);
+ }
+
+ if (is_not_in) {
+ if (zone_map.min_value == zone_map.max_value) {
+ const bool only_value_is_filtered =
+ std::any_of(values.begin(), values.end(),
+ [&](const Field& value) { return value ==
zone_map.min_value; });
+ return only_value_is_filtered ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ }
+ return ZoneMapFilterResult::kMayMatch;
+ }
+
+ if (field_less(zone_map.max_value, min_value) ||
field_greater(zone_map.min_value, max_value)) {
+ return ZoneMapFilterResult::kNoMatch;
Review Comment:
`eval_in_zonemap()` reaches `Field` comparisons without validating that the
materialized IN-list values have a type compatible with the slot/zonemap type.
The comparison path calls `check_type_compatible()` before any `Field`
ordering, but IN only checks that the zonemap range matches the slot type. If a
BE expression has an INT slot with STRING literal fields,
`field_less(zone_map.max_value, min_value)` throws from `Field::operator<=>`
instead of conservatively returning `kUnsupported`. Please add the same
compatibility guard for the IN min/max/value fields before these comparisons
and cover this fallback in the unit tests.
--
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]
