github-actions[bot] commented on code in PR #67019:
URL: https://github.com/apache/doris/pull/67019#discussion_r3842763448
##########
be/src/storage/segment/segment.cpp:
##########
@@ -414,47 +425,65 @@ Status Segment::new_iterator(ReadSchemaSPtr schema, const
StorageReadOptions& re
// Placeholder tso column on a single-version binlog segment: its
zonemap reflects the
// NULL placeholder (replaced with commit_tso at read time), so skip
pruning by
// zonemap (min == max == commit_tso) and reuse the predicate's own
zonemap matching:
- // evaluate_and() returns false iff no value in [min, max] can satisfy
the predicates,
- // i.e. commit_tso fails them and the whole segment can be pruned.
Predicates that don't
- // support zonemap return true (conservative: not pruned, row-level
eval handles them).
- if (read_options.col_id_to_predicates.contains(column_id) &&
- is_tso_placeholder_col(column_id, *schema, read_options)) {
+ // kNoMatch means no value in [min, max] can satisfy the predicates,
i.e. commit_tso fails
+ // them and the whole segment can be pruned. Predicates that don't
support zonemap report
+ // kMayMatch (conservative: not pruned, row-level eval handles them).
+ if (is_tso_placeholder_col(column_id, *schema, read_options)) {
const Int64 commit_tso =
read_options.commit_tso.end_tso() == -1 ? 0 :
read_options.commit_tso.end_tso();
ZoneMap zone_map;
zone_map.min_value = Field::create_field<TYPE_BIGINT>(commit_tso);
zone_map.max_value = Field::create_field<TYPE_BIGINT>(commit_tso);
zone_map.has_not_null = true;
- if (!entry.second->evaluate_and(zone_map)) {
+ if (col_predicates->evaluate_zonemap_filter(zone_map) ==
+ ZoneMapFilterResult::kNoMatch) {
// any condition not satisfied, return.
*iter = std::make_unique<EmptySegmentIterator>(*schema);
read_options.stats->filtered_segment_number++;
return Status::OK();
}
+ // The zone map above is made up rather than read from disk, so it
must not drop
+ // a predicate either.
continue;
}
- if (read_options.col_id_to_predicates.contains(column_id) &&
- can_apply_predicate_safely(column_id, *schema,
-
read_options.target_cast_type_for_variants, read_options)) {
- bool matched = true;
- RETURN_IF_ERROR(reader->match_condition(entry.second.get(),
&matched));
- if (!matched) {
- // any condition not satisfied, return.
- *iter = std::make_unique<EmptySegmentIterator>(*schema);
- read_options.stats->filtered_segment_number++;
- read_options.stats->rows_stats_filtered += num_rows();
- return Status::OK();
+ if (!can_apply_predicate_safely(column_id, *schema,
+
read_options.target_cast_type_for_variants, read_options)) {
+ continue;
+ }
+
+ ZoneMap zone_map;
+ RETURN_IF_ERROR(reader->get_segment_zone_map(&zone_map));
+ if (col_predicates->evaluate_zonemap_filter(zone_map) ==
ZoneMapFilterResult::kNoMatch) {
+ // any condition not satisfied, return.
+ *iter = std::make_unique<EmptySegmentIterator>(*schema);
+ read_options.stats->filtered_segment_number++;
+ read_options.stats->rows_stats_filtered += num_rows();
+ return Status::OK();
+ }
+ // The group above answers for the column as a whole. Dropping needs
to know which single
+ // predicate the zone map proves redundant, which only a per-predicate
answer can tell.
+ if (can_drop_predicate) {
+ for (const auto& predicate : read_options.column_predicates) {
+ if (predicate->column_id() == column_id &&
+ predicate->evaluate_zonemap_filter(zone_map) ==
+ ZoneMapFilterResult::kAllMatch) {
+ always_true_predicates.insert(predicate.get());
Review Comment:
[P1] Do not remove predicates using stored placeholders for
read-time-replaced hidden columns. This direct path special-cases commit TSO
but not `__DORIS_VERSION_COL__`: on a single-version rowset with real version
1000 and stored 0, `__DORIS_VERSION_COL__ = 0` is newly marked all-match here
and erased before `_replace_version_col_if_needed()` can test 1000. The
common-expression context also reads raw placeholders for VERSION and
commit/binlog TSO, so `< 500 OR k > 10` can be removed on the same false
evidence. Please centralize synthetic zone-map values for every
read-time-replaced column, or forbid both direct and common all-match removal
for them, with VERSION and TSO regressions.
##########
be/src/storage/predicate/like_column_predicate.h:
##########
@@ -69,6 +69,10 @@ class LikeColumnPredicate final : public ColumnPredicate {
}
PredicateType type() const override { return PredicateType::LIKE; }
+
+ // A LIKE pattern matches on the value itself, so the min/max of a zone
says nothing about it.
+ bool support_zonemap() const override { return false; }
Review Comment:
[P2] Preserve delete-condition page pruning for LIKE/BF queries.
`SegmentIterator::_get_row_ranges_from_conditions()` skips this column's
page-zone-map walk when this returns false, before it considers the independent
`del_predicates_for_zone_map`. Previously LIKE inherited true, its query-side
range check conservatively kept the page, and
`ColumnReader::_get_filtered_pages()` could still skip a page wholly covered by
(for example) `DELETE WHERE s = 'x'`. After this override, `WHERE s LIKE 'x%'`
reads and row-filters those pages instead; the BF override has the same issue.
Please gate the walk on query support *or* delete predicates and treat only the
unsupported query side as may-match.
--
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]
