Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 176 additions & 8 deletions be/src/exprs/expr_zonemap_filter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
#include "exprs/expr_zonemap_filter.h"

#include <algorithm>
#include <cmath>
#include <limits>
#include <set>
#include <type_traits>
#include <utility>

#include "common/check.h"
Expand Down Expand Up @@ -49,6 +52,33 @@ std::optional<std::pair<Field, DataTypePtr>> field_from_literal_expr(const VExpr
return std::make_pair(std::move(field), literal->get_data_type());
}

std::optional<int32_t> struct_field_ordinal(const Field& field) {
int64_t ordinal = -1;
switch (field.get_type()) {
case TYPE_BOOLEAN:
ordinal = field.get<TYPE_BOOLEAN>();
break;
case TYPE_TINYINT:
ordinal = field.get<TYPE_TINYINT>();
break;
case TYPE_SMALLINT:
ordinal = field.get<TYPE_SMALLINT>();
break;
case TYPE_INT:
ordinal = field.get<TYPE_INT>();
break;
case TYPE_BIGINT:
ordinal = field.get<TYPE_BIGINT>();
break;
default:
return std::nullopt;
}
if (ordinal <= 0 || ordinal > std::numeric_limits<int32_t>::max()) {
return std::nullopt;
}
return static_cast<int32_t>(ordinal - 1);
}

bool value_in_range(const Field& value, const Field& min_value, const Field& max_value) {
return value >= min_value && value <= max_value;
}
Expand All @@ -60,6 +90,29 @@ bool dictionary_contains(const DictionaryEvalContext::SlotDictionary& dictionary
});
}

bool bloom_filter_probes_equal(const BloomFilterProbe& lhs, const BloomFilterProbe& rhs) {
return lhs.slot_index == rhs.slot_index && lhs.path == rhs.path &&
data_types_compatible(lhs.value_type, rhs.value_type);
}

template <typename T>
bool floating_point_bloom_filter_may_contain(const segment_v2::BloomFilter& bloom_filter, T value) {
static_assert(std::is_floating_point_v<T>);
// Doris equality collapses NaN payloads and signed zeros, while Parquet Bloom hashes physical
// bytes. A negative probe is safe only after covering the entire Doris-equivalent class.
if (std::isnan(value)) {
return true;
}
const auto test_value = [&](T candidate) {
return bloom_filter.test_bytes(reinterpret_cast<const char*>(&candidate),
sizeof(candidate));
};
if (test_value(value)) {
return true;
}
return value == T {0} && test_value(-value);
}

bool bloom_filter_may_contain(const BloomFilterEvalContext::SlotBloomFilter& slot_filter,
const Field& value) {
DORIS_CHECK(slot_filter.data_type != nullptr);
Expand All @@ -84,13 +137,11 @@ bool bloom_filter_may_contain(const BloomFilterEvalContext::SlotBloomFilter& slo
}
case TYPE_FLOAT: {
const float typed_value = value.get<TYPE_FLOAT>();
return slot_filter.bloom_filter->test_bytes(reinterpret_cast<const char*>(&typed_value),
sizeof(typed_value));
return floating_point_bloom_filter_may_contain(*slot_filter.bloom_filter, typed_value);
}
case TYPE_DOUBLE: {
const double typed_value = value.get<TYPE_DOUBLE>();
return slot_filter.bloom_filter->test_bytes(reinterpret_cast<const char*>(&typed_value),
sizeof(typed_value));
return floating_point_bloom_filter_may_contain(*slot_filter.bloom_filter, typed_value);
}
case TYPE_CHAR:
case TYPE_VARCHAR:
Expand Down Expand Up @@ -213,6 +264,123 @@ std::optional<SlotLiteral> extract_slot_and_literal(const VExprSPtrs& args) {
return std::nullopt;
}

std::optional<BloomFilterProbe> extract_bloom_filter_probe(const VExprSPtr& expr) {
if (expr == nullptr || expr->data_type() == nullptr) {
return std::nullopt;
}
if (auto slot = std::dynamic_pointer_cast<VSlotRef>(expr); slot) {
return BloomFilterProbe {
.slot_index = slot->column_id(), .value_type = slot->data_type(), .path = {}};
}
if ((expr->fn().name.function_name != "element_at" &&
expr->fn().name.function_name != "struct_element") ||
expr->get_num_children() != 2) {
return std::nullopt;
}

auto probe = extract_bloom_filter_probe(expr->get_child(0));
auto selector = field_from_literal_expr(expr->get_child(1));
if (!probe.has_value() || !selector.has_value() || selector->first.is_null()) {
return std::nullopt;
}
const auto parent_type = remove_nullable(expr->get_child(0)->data_type());
if (parent_type == nullptr) {
return std::nullopt;
}

BloomFilterPathElement path_element;
switch (parent_type->get_primitive_type()) {
case TYPE_STRUCT: {
path_element.kind = BloomFilterPathKind::STRUCT_FIELD;
const auto selector_type = remove_nullable(selector->second);
if (selector_type == nullptr) {
return std::nullopt;
}
if (is_string_type(selector_type->get_primitive_type())) {
path_element.field_name = selector->first.get<TYPE_STRING>();
} else {
auto ordinal = struct_field_ordinal(selector->first);
if (!ordinal.has_value()) {
return std::nullopt;
}
path_element.field_ordinal = *ordinal;
}
break;
}
case TYPE_ARRAY:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Connect LIST-to-STRUCT probes to production localization

The recursive extractor accepts a LIST_ELEMENT -> STRUCT_FIELD path, but TableColumnMapper::collect_struct_element_chain() rejects a struct accessor whose parent is the computed array element. Thus element_at(element_at(items, 1), 'a') = 7 produces no file-local conjunct even when the table and file ARRAY<STRUCT<a: INT>> schemas are identical, and this Parquet Bloom path is never reached; the existing ArrayWrapperDoesNotBuildNestedPredicateFilter test confirms that request is empty. Please add a schema-safe localization path plus a mapper-to-Parquet regression, or narrow the advertised recursive LIST/STRUCT support to the path shapes that can actually reach the reader.

// Array element positions share one repeated Parquet leaf; membership in that leaf is a
// necessary condition for any element_at(array, constant) equality to match.
path_element.kind = BloomFilterPathKind::LIST_ELEMENT;
break;
default:
return std::nullopt;
}
probe->value_type = expr->data_type();
probe->path.push_back(std::move(path_element));
return probe;
}

bool collect_unique_bloom_filter_probe(const VExprSPtr& expr,
std::optional<BloomFilterProbe>* result) {
DORIS_CHECK(result != nullptr);
if (auto probe = extract_bloom_filter_probe(expr); probe.has_value()) {
if (result->has_value() && !bloom_filter_probes_equal(**result, *probe)) {
return false;
}
*result = std::move(probe);
return true;
}
if (expr == nullptr) {
return true;
}
for (uint16_t child_idx = 0; child_idx < expr->get_num_children(); ++child_idx) {
const auto& child = expr->get_child(child_idx);
if (child == nullptr || child->is_literal()) {
continue;
}
// Every Bloom-capable branch must bind to the same leaf; a conflicting subtree cannot be
// treated like a branch without a probe because the compound evaluator would use it.
if (!collect_unique_bloom_filter_probe(child, result)) {
return false;
}
}
return true;
}

std::optional<BloomFilterProbe> extract_bloom_filter_predicate_probe(const VExprSPtr& expr) {
std::optional<BloomFilterProbe> result;
if (!collect_unique_bloom_filter_probe(expr, &result)) {
return std::nullopt;
}
return result;
}

std::optional<SlotLiteral> extract_bloom_filter_slot_and_literal(const VExprSPtrs& args) {
if (args.size() != 2) {
return std::nullopt;
}
for (size_t probe_idx = 0; probe_idx < args.size(); ++probe_idx) {
auto probe = extract_bloom_filter_probe(args[probe_idx]);
auto literal = field_from_literal_expr(args[1 - probe_idx]);
if (!probe.has_value() || !literal.has_value()) {
continue;
}
auto [literal_value, literal_type] = std::move(*literal);
return SlotLiteral {.slot_index = probe->slot_index,
.slot_type = probe->value_type,
.literal = std::move(literal_value),
.literal_type = std::move(literal_type),
.literal_on_left = probe_idx == 1};
}
return std::nullopt;
}

bool can_evaluate_bloom_filter_equality(const VExprSPtrs& args) {
auto slot_literal = extract_bloom_filter_slot_and_literal(args);
return slot_literal.has_value() && !slot_literal->literal.is_null() &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve FLOAT/DOUBLE equality classes when probing Parquet Blooms

This capability now admits nested FLOAT/DOUBLE equality (and is reused by null-safe equality), but the native adapter hashes the literal's raw IEEE bytes. Doris considers +0.0 == -0.0 and equates NaNs, while Parquet Bloom filters hash their distinct PLAIN encodings, so a Bloom containing -0.0 can reject a +0.0 probe and falsely skip a matching Row Group. Please probe both zero encodings and conservatively return kMayMatch for NaN, or disable FLOAT/DOUBLE Bloom pruning; add an external-writer signed-zero/NaN regression.

data_types_compatible(slot_literal->slot_type, slot_literal->literal_type);
}

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 ||
Expand Down Expand Up @@ -374,14 +542,14 @@ ZoneMapFilterResult eval_in_bloom_filter(const BloomFilterEvalContext& ctx,
if (is_not_in) {
return ZoneMapFilterResult::kUnsupported;
}
auto slot = std::dynamic_pointer_cast<VSlotRef>(slot_expr);
DORIS_CHECK(slot != nullptr);
auto slot_filter = ctx.slot(slot->column_id());
auto probe = extract_bloom_filter_probe(slot_expr);
DORIS_CHECK(probe.has_value());
auto slot_filter = ctx.slot(probe->slot_index);
if (slot_filter == nullptr || slot_filter->data_type == nullptr ||
slot_filter->bloom_filter == nullptr) {
return ZoneMapFilterResult::kUnsupported;
}
DORIS_CHECK(data_types_compatible(slot_filter->data_type, slot->data_type()));
DORIS_CHECK(data_types_compatible(slot_filter->data_type, probe->value_type));
if (values.empty()) {
return ZoneMapFilterResult::kNoMatch;
}
Expand Down
30 changes: 30 additions & 0 deletions be/src/exprs/expr_zonemap_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <compare>
#include <map>
#include <optional>
#include <string>
#include <vector>

#include "common/check.h"
Expand Down Expand Up @@ -99,8 +100,37 @@ struct SlotLiteral {
bool literal_on_left;
};

enum class BloomFilterPathKind {
STRUCT_FIELD,
LIST_ELEMENT,
};

struct BloomFilterPathElement {
BloomFilterPathKind kind;
std::string field_name;
int32_t field_ordinal = -1;

bool operator==(const BloomFilterPathElement&) const = default;
};

struct BloomFilterProbe {
int slot_index;
DataTypePtr value_type;
std::vector<BloomFilterPathElement> path;

bool operator==(const BloomFilterProbe&) const = default;
};

std::optional<SlotLiteral> extract_slot_and_literal(const VExprSPtrs& args);

std::optional<BloomFilterProbe> extract_bloom_filter_probe(const VExprSPtr& expr);

std::optional<BloomFilterProbe> extract_bloom_filter_predicate_probe(const VExprSPtr& expr);

std::optional<SlotLiteral> extract_bloom_filter_slot_and_literal(const VExprSPtrs& args);

bool can_evaluate_bloom_filter_equality(const VExprSPtrs& args);

TExprNode create_texpr_node_from_hybrid_set_value(const void* data, const PrimitiveType& type,
int precision, int scale);

Expand Down
16 changes: 15 additions & 1 deletion be/src/exprs/function/comparison_equal_for_null.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "core/data_type/data_type_number.h"
#include "core/types.h"
#include "exprs/aggregate/aggregate_function.h"
#include "exprs/expr_zonemap_filter.h"
#include "exprs/function/function.h"
#include "exprs/function/function_helpers.h"
#include "exprs/function/simple_function_factory.h"
Expand Down Expand Up @@ -64,6 +65,19 @@ class FunctionEqForNull : public IFunction {

bool use_default_implementation_for_nulls() const override { return false; }

ZoneMapFilterResult evaluate_bloom_filter(const BloomFilterEvalContext& ctx,
const VExprSPtrs& arguments) const override {
auto slot_literal = expr_zonemap::extract_bloom_filter_slot_and_literal(arguments);
DORIS_CHECK(slot_literal.has_value());
return expr_zonemap::eval_eq_bloom_filter(ctx, *slot_literal);
}

bool can_evaluate_bloom_filter(const VExprSPtrs& arguments) const override {
// Parquet Bloom filters do not encode null membership, so null-safe equality can only use
// them when its literal is non-null and ordinary equality semantics apply.
return expr_zonemap::can_evaluate_bloom_filter_equality(arguments);
}

Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments,
uint32_t result, size_t input_rows_count) const override {
ColumnWithTypeAndName& col_left = block.get_by_position(arguments[0]);
Expand Down Expand Up @@ -278,4 +292,4 @@ class FunctionEqForNull : public IFunction {
void register_function_comparison_eq_for_null(SimpleFunctionFactory& factory) {
factory.register_function<FunctionEqForNull>();
}
} // namespace doris
} // namespace doris
5 changes: 3 additions & 2 deletions be/src/exprs/function/functions_comparison.h
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ inline ZoneMapFilterResult evaluate_dictionary(const DictionaryEvalContext& ctx,
inline ZoneMapFilterResult evaluate_bloom_filter(const BloomFilterEvalContext& ctx,
const VExprSPtrs& arguments, Op op) {
DORIS_CHECK(op == Op::EQ);
auto slot_literal = expr_zonemap::extract_slot_and_literal(arguments);
auto slot_literal = expr_zonemap::extract_bloom_filter_slot_and_literal(arguments);
DORIS_CHECK(slot_literal.has_value());
return expr_zonemap::eval_eq_bloom_filter(ctx, *slot_literal);
}
Expand Down Expand Up @@ -658,7 +658,8 @@ class FunctionComparison : public IFunction {

bool can_evaluate_bloom_filter(const VExprSPtrs& arguments) const override {
auto op = comparison_zonemap_detail::op_from_name(name);
return op.has_value() && comparison_zonemap_detail::can_evaluate_equality(arguments, *op);
return op == comparison_zonemap_detail::Op::EQ &&
expr_zonemap::can_evaluate_bloom_filter_equality(arguments);
}

/// Get result types by argument types. If the function does not apply to these arguments, throw an exception.
Expand Down
19 changes: 16 additions & 3 deletions be/src/exprs/vectorized_fn_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -671,11 +671,24 @@ bool VectorizedFnCall::is_deterministic() const {
}

bool VectorizedFnCall::is_safe_to_execute_on_selected_rows() const {
static const std::set<std::string> TOTAL_PREDICATE_FUNCTIONS = {
"eq", "ne", "lt", "le", "gt", "ge", "in", "not_in", "is_null_pred", "is_not_null_pred"};
static const std::set<std::string> TOTAL_PREDICATE_FUNCTIONS = {"eq",
"eq_for_null",
"ne",
"lt",
"le",
"gt",
"ge",
"in",
"not_in",
"is_null_pred",
"is_not_null_pred",
"element_at",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Fence constant pruning after a rejected accessor

Adding element_at here also keeps the table-side constant-pruning prefix open. For table items ARRAY<STRUCT<a INT NOT NULL>>, z INT DEFAULT 0, a file containing only items ARRAY<STRUCT<a Nullable(INT)>>, predicates [items[1].a > 10, z = 7], and a row with a=NULL, the mapper correctly leaves the first predicate above TableReader so required-child alignment can report the schema error. But missing z is a constant/default filter entry, so _evaluate_constant_filters() skips the nonconstant first filter, evaluates z=7 as false, and closes the split before any row reaches alignment. Before this accessor was classified safe, the first conjunct closed that prefix. Please propagate a rejected mapping/materialization barrier into constant pruning too, and add a missing/default-column regression for this order.

"struct_element"};
// Selected-row execution may hide data-dependent errors in rows rejected by an earlier
// predicate. Keep function calls unsafe by default and opt in only operations that are total
// for their input domain; child checks then reject expressions such as gt(mod(x, -1), 0).
// for their input domain. Accessors return NULL for absent elements, so admitting them keeps
// nested metadata predicates reachable without crossing an error-producing child such as
// gt(mod(x, -1), 0).
return TOTAL_PREDICATE_FUNCTIONS.contains(_function_name) &&
VExpr::is_safe_to_execute_on_selected_rows();
}
Expand Down
12 changes: 9 additions & 3 deletions be/src/exprs/vin_predicate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,17 @@ Status VInPredicate::_materialize_for_zonemap_filter(VExprContext* context) {
_seg_filter_contains_null = false;
_zonemap_materialized = false;
_direct_filter_set.reset();
if (_children.size() < 2 || !_children[0]->is_slot_ref()) {
if (_children.size() < 2) {
return Status::OK();
}

const auto data_type = remove_nullable(_children[0]->data_type());
auto bloom_probe = expr_zonemap::extract_bloom_filter_probe(_children[0]);
if (!bloom_probe.has_value()) {
return Status::OK();
}
// Materialization is shared by all pruning paths. Their capability checks keep ZoneMap,
// dictionary, and raw evaluation direct-slot-only while Bloom may consume a nested leaf.
const auto data_type = remove_nullable(bloom_probe->value_type);
DORIS_CHECK(data_type != nullptr);
if (is_complex_type(data_type->get_primitive_type())) {
return Status::OK();
Expand Down Expand Up @@ -218,7 +224,7 @@ ZoneMapFilterResult VInPredicate::evaluate_bloom_filter(const BloomFilterEvalCon

bool VInPredicate::can_evaluate_bloom_filter() const {
return _zonemap_materialized && !_is_not_in &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Materialize nested IN probes before checking Bloom capability

This new accessor-aware check still requires _zonemap_materialized, but _materialize_for_zonemap_filter() returns early unless child 0 is a direct slot. A production element_at(s, 'a') IN (2, 4) therefore always reports false here, so the nested-IN part of this feature is unreachable; the test's BloomInExpr masks this by hard-coding capability and values. Please allow valid primitive nested probes to materialize their constant set while retaining the direct-slot gates for ZoneMap/dictionary/raw paths, and add a prepared production VInPredicate test.

std::dynamic_pointer_cast<VSlotRef>(get_child(0)) != nullptr;
expr_zonemap::extract_bloom_filter_probe(get_child(0)).has_value();
}

bool VInPredicate::can_execute_on_raw_fixed_values(const DataTypePtr& data_type,
Expand Down
Loading
Loading