[Bug] Zone-map pruning trusts placeholder summaries for the hidden TSO and version columns, dropping matching rows

[LuciferYang]

Search before asking

• I had searched in the issues and found no similar issues.

Version

master, 846d9b2.

What's Wrong?

Three hidden columns store a placeholder on disk and get their real value substituted at read time, so their zone maps describe the placeholder rather than the values rows come back with. Zone-map pruning that trusts those summaries drops rows that match.

column	on disk	substituted at read time by
__DORIS_BINLOG_TSO__	all NULL (ColumnNullable::insert_many_defaults, be/src/storage/transform/row_binlog_derive.cpp:126-129)	SegmentIterator::_update_tso_col_if_needed, be/src/storage/segment/segment_iterator.cpp:2476
__DORIS_COMMIT_TSO_COL__	0	a ConstantColumnReader built in Segment::new_iterator / new_column_iterator, be/src/storage/segment/segment.cpp:422-437, :947-966
__DORIS_VERSION_COL__	0 (default ZERO_NUMBER)	SegmentIterator::_replace_version_col_if_needed, be/src/storage/segment/segment_iterator.cpp:2449

The ColumnPredicate paths already handle the two TSO columns. Segment::new_iterator synthesises {min = max = commit_tso, has_not_null = true} for the binlog TSO column and prunes against the real value (be/src/storage/segment/segment.cpp:449-465), the page-level loop skips it (be/src/storage/segment/segment_iterator.cpp:1153), and the commit TSO column is covered by the constant reader above.

The expression zone-map paths have neither guard. build_segment_zonemap_context (be/src/storage/segment/segment.cpp:94-145) and the page-level expression loop (be/src/storage/segment/segment_iterator.cpp:3406-3415) check only Segment::can_apply_predicate_safely, which is about variant target types and returns true here.

For the binlog TSO column the failure is stronger than a wrong bound. Its on-disk zone map has has_not_null = false, and every evaluator treats that as "the comparison is NULL on every row, so no row can be TRUE" and returns kNoMatch before looking at any bound: be/src/exprs/function/functions_comparison.h:337-338, :380-382, and be/src/exprs/expr_zonemap_filter.cpp:498 for IS NOT NULL. So one comparison on that column prunes the whole single-version binlog segment, for any operator and any other operand, even though after substitution the rows carry a non-null TSO.

__DORIS_VERSION_COL__ has no guard on any path, including ColumnPredicate, and it is reachable from plain SQL:

• It exists on effectively every unique-key table: added when Config.enable_hidden_version_column_by_default && keysType == UNIQUE_KEYS && !hasSeqMapping (fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java:1070-1076), and that config defaults to true (fe/fe-common/src/main/java/org/apache/doris/common/Config.java:1500).
• It is invisible, but invisibility only suppresses star expansion (fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzer.java:436-441); binding an explicitly named slot has no visibility check, and it is in the scan output (LogicalOlapScan.computeOutput builds from getBaseSchema(true), fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java:826). Existing regression cases already name it directly.
• On merge-on-write its aggregation is NONE (CreateTableInfo.java:1071), so a predicate on it lands in the storage-layer set rather than the post-merge one (TabletReader::_init_column_predicates, be/src/storage/tablet/tablet_reader.cpp:389-402) and reaches zone-map pruning. On merge-on-read it is REPLACE and stays above the merge.
• The substitution happens before predicate evaluation, so the two disagree: _read_columns_by_index then _replace_version_col_if_needed then _evaluate_vectorization_predicate (be/src/storage/segment/segment_iterator.cpp:2959, :2960, :2978). Row evaluation sees the real version while the zone map says [0, 0].

So on a unique merge-on-write table, WHERE __DORIS_VERSION_COL__ = <n> with n != 0 returns nothing on single-version rowsets, while the rows it asks for exist and row-level evaluation would have matched them.

What You Expected?

A column whose stored value is a placeholder either does not drive zone-map pruning, or drives it through a summary of the value rows will actually have. Pruning and row-level evaluation should not disagree about the same predicate.

How to Reproduce?

The version column case is plain SQL on a unique merge-on-write table with n != 0, comparing results with pruning on and off:

CREATE TABLE t (k INT, v INT) UNIQUE KEY(k) DISTRIBUTED BY HASH(k) BUCKETS 1
PROPERTIES ("replication_num" = "1", "enable_unique_key_merge_on_write" = "true");
INSERT INTO t VALUES (1, 1);
INSERT INTO t VALUES (2, 2);
SELECT k, __DORIS_VERSION_COL__ FROM t;              -- shows the real versions
SELECT count(*) FROM t WHERE __DORIS_VERSION_COL__ = <a version from above>;

The two TSO columns need a pushed common expression on them rather than a ColumnPredicate; IS NULL / IS NOT NULL qualify today (be/src/exprs/function/is_null.h:67, be/src/exprs/function/is_not_null.h:69), and column-vs-column comparison would once #67774 lands, which is what surfaced this.

I have not run the SQL above; it is derived from the code path, and the aggregation-type split means it should reproduce on merge-on-write and not on merge-on-read.

Anything Else?

The substituted values are known exactly where the builders run, so the fix can synthesise a real summary rather than only skip: commit_tso.end_tso() (or 0 when it is -1) for the TSO columns, version.second for the version column, both always non-null. Note that the comment at be/src/storage/segment/segment_iterator.cpp:2478 claims the physical-time part of the commit TSO is used; the code inserts the raw value (:2493, :2500-2503, :2511-2513, :2525), matching the formula the ColumnPredicate path already uses at be/src/storage/segment/segment.cpp:456-461. The comment is stale and misleading.

Routing the binlog TSO column through the constant-reader cache instead would be wrong: its substitution only happens for read_row_binlog reads (be/src/storage/segment/segment_iterator.cpp:2483-2485), and a cached constant reader would then be served to non-binlog reads of the same segment. That cache has a separate problem, filed as #67994.

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[Doris-Breakwater]

Initial maintainer analysis

Triage: This is a high-confidence correctness issue from static analysis of the reported snapshot 846d9b2ebfc595ddfd77579447db16a674c901f7. It is not merely a missed optimization: a physical zone map is being used to decide the truth of predicates over a different, read-time value. The issue currently has no labels or assignee; the appropriate classification is the repository's bug/correctness label plus the BE storage/segment (zone-map) area.

Verified facts

• Expression zone-map pruning is enabled by default (SessionVariable.enableExprZonemapFilter = true; the BE helper also treats an unset query option as enabled).
• build_segment_zonemap_context reads the physical ColumnReader summary, and SegmentIterator::_apply_expr_zonemap_to_row_ranges does the same for page summaries. Neither path recognizes read-time-substituted columns. Their only safety gate is Segment::can_apply_predicate_safely, whose implementation only handles variant target-type compatibility and returns true for these BIGINT columns.
• The reported placeholder/substitution mismatch is present:
  • __DORIS_BINLOG_TSO__ is written as NULL by fill_binlog_system_columns, then replaced with a non-NULL commit_tso.end_tso() (or 0 for -1) for single-version read_row_binlog reads.
  • __DORIS_VERSION_COL__ has default 0, then SegmentIterator::_replace_version_col_if_needed replaces it with version.second for single-version rowsets.
  • __DORIS_COMMIT_TSO_COL__ has default 0; the intended read path requests a ConstantColumnReader carrying the assigned commit TSO for a single-version rowset.
• Row-level evaluation occurs after _replace_version_col_if_needed and _update_tso_col_if_needed, while segment/page zone-map pruning occurs before those substitutions. This establishes the semantic disagreement in the code path.
• An all-NULL binlog-TSO zone map makes comparison evaluators return kNoMatch immediately and makes IS NOT NULL return kNoMatch, so an expression over the read-time non-NULL value can eliminate a whole segment/page.
• The version-column reachability analysis is also supported by the snapshot: the hidden column is enabled by default for UNIQUE KEY tables without sequence mapping, uses aggregation NONE for merge-on-write and REPLACE for merge-on-read, is included in LogicalOlapScan output via getBaseSchema(true), and explicit slot binding does not reject an invisible column. Existing regression suites explicitly reference this column.

There is an additional wrong-result mode beyond the false negatives described in the issue. After creating the iterator, Segment::new_iterator calls ColumnReader::prune_predicates_by_zone_map on cached readers without a placeholder-column guard. For a single-version rowset whose semantic version is positive, the physical [0, 0] summary can mark a range predicate such as __DORIS_VERSION_COL__ < 1 as always true; the predicate is then removed even though it is false for the substituted value. That can admit nonmatching rows. The fix and tests should therefore cover both “no-match” pruning and “always-true” predicate removal.

Scope and related issue

• The unsafe synthesized-value cases are limited by the same guards as their substitutions: single-version rowsets for version/commit TSO, and additionally read_row_binlog for binlog TSO. Range/compaction rowsets and non-binlog reads of __DORIS_BINLOG_TSO__ must continue using physical values.
• The commit-TSO expression path is entangled with [Bug] The column reader cache drops a requested constant value on a cache hit, so a placeholder column can be read as its on-disk placeholder #67994 but this issue is not a duplicate. The expression builder can cache a non-constant reader before new_column_iterator requests the constant, while [Bug] The column reader cache drops a requested constant value on a cache hit, so a placeholder column can be read as its on-disk placeholder #67994 shows that the cache key ignores the requested constant. Fixing only the cache does not make the binlog-TSO or version physical zone maps safe; fixing only the zone-map builders does not repair a previously cached wrong reader.
• A nearby path should be audited before closing this issue: segment_zone_maps_can_answer_agg excludes the commit-TSO ordinal but not the version/binlog-TSO ordinals, and VStatisticsIterator reads MIN/MAX from zone maps. I have not established FE reachability for a pushed MIN/MAX over these hidden columns, so this is a test requirement/hypothesis rather than a confirmed additional symptom.

Missing runtime evidence

The reporter explicitly states that the SQL was not run. Static evidence is strong enough to accept the issue, but the PR should include runtime confirmation with:

1. The exact SQL output using concrete returned version values (replace the <a version from above> placeholder), SHOW CREATE TABLE/full schema output, and the exact build SHA.
2. EXPLAIN VERBOSE proving whether each predicate is a ColumnPredicate or a pushed common expression, plus a query profile containing RowsStatsFiltered, ExprZoneMapFilteredSegments, and ExprZoneMapFilteredPages where applicable.
3. Rowset/segment topology proving the tested data remains in single-version rowsets (avoid a compaction race), and a range/compacted-rowset control case.
4. For the TSO cases, the exact row-binlog read entry point, read_row_binlog state, assigned commit TSO, pushed expression shape, and results with enable_expr_zonemap_filter both on and off.

Note that SET enable_expr_zonemap_filter = false is only a partial diagnostic/workaround: it disables the expression path, not the version column's ColumnPredicate zone-map path or the post-creation predicate-removal path.

Recommended implementation and tests

Centralize the concept of an effective read-time zone map for placeholder-backed columns, with the same conditions used by the substitution code. For an active substitution, expose a non-NULL degenerate summary (min = max = semantic value, has_not_null = true, has_null = false): version.second for the version column and the effective commit TSO (end_tso, or 0 where current read semantics use 0) for the TSO columns. Otherwise use the physical summary. This helper should be consumed consistently by:

• segment-level expression zone-map evaluation;
• page-level expression evaluation (the value is segment-constant, so every page has the same effective summary, or physical page pruning can be conservatively skipped);
• segment/page ColumnPredicate pruning, including the existing binlog-TSO special case and the missing version case; and
• the post-creation prune_predicates_by_zone_map optimization.

Also audit statistics-iterator and any dictionary/bloom/index pruning of these columns so no other physical summary can make a semantic decision about a substituted value.

Tests should include: version equality/range predicates that exercise both false-negative pruning and false-positive predicate removal; expression comparisons and IS NULL/IS NOT NULL; all three placeholder columns; segment and page paths; assigned and -1 TSO; read_row_binlog true/false; single-version versus range rowsets; merge-on-write versus merge-on-read; and cache call ordering coordinated with #67994. Each positive query should compare pruning enabled/disabled where that switch applies and assert returned rows, not only pruning counters.

Breakwater-GitHub-Analysis-Slot: slot_1e4d5197b1b3

[LuciferYang]

Two things in that analysis are additions I had missed, and both change the scope of the fix.

The always-true direction. ColumnReader::prune_predicates_by_zone_map runs on cached readers with no placeholder guard, so a physical [0, 0] summary can prove a range predicate such as __DORIS_VERSION_COL__ < 1 always true and remove it, admitting rows that do not match the substituted value. My report only described the false-negative direction. The fix has to cover predicate removal as well as pruning.

The statistics path. segment_zone_maps_can_answer_agg excludes the commit-TSO ordinal but not the version or binlog-TSO ordinals, so a pushed min()/max() over those columns would answer from the physical summary. I have not established FE reachability for that either; I will check it before claiming it in a PR, and either fix it or say why it cannot be reached.

Agreed on the shape of the fix: one effective-read-time-summary helper gated by exactly the substitution conditions (single-version for version and commit TSO, plus read_row_binlog for binlog TSO), consumed by segment and page expression evaluation, segment and page ColumnPredicate pruning including the existing binlog-TSO branch and the missing version case, and prune_predicates_by_zone_map. Patching the two expression builders alone, which is what I proposed, would leave the version column's ColumnPredicate path and the removal path wrong.

On runtime evidence: the report is static, as it says. I have a local single-BE cluster and will attach the concrete SQL output, EXPLAIN VERBOSE showing which predicates are ColumnPredicate versus pushed common expressions, the profile counters, and a range-rowset control case with the PR rather than guess at them here.

[LuciferYang]

The statistics path is reachable for the version column, and not for binlog TSO. Traced, still static.

max(__DORIS_VERSION_COL__) on a unique-key table returns 0 when enable_pushdown_minmax_on_unique is on (it defaults to false, SessionVariable.java:2379-2381). The chain: AggregateStrategies.pushdownMinMaxOnUniqueTable -> checkWhetherPushDownMinMax (fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java:487-528), which checks only that the original column exists and that its type is not complex/HLL/bitmap, with no visibility check and no synthetic-column check, unlike the generic path at :729-742; then TPushAggOp.MINMAX; then on BE use_statistics_iterator (be/src/storage/segment/segment.cpp:506-521) and segment_zone_maps_can_answer_agg, which skips only the commit-TSO ordinal and disqualifies only on pass_all, so a [0, 0] summary is judged usable; then VStatisticsIterator::next_batch (be/src/storage/iterator/vgeneric_iterators.cpp:63-93) and ColumnReader::next_batch_of_zone_map (be/src/storage/segment/column_reader.cpp:711-732), which report the placeholder. SegmentIterator::_replace_version_col_if_needed never runs on that path.

__DORIS_BINLOG_TSO__ is safe here. It exists only in the row-binlog index schema (fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:2506), reachable only through RowBinlogTableWrapper, which reports DUP_KEYS, so the unique-table rule cannot fire; the generic rule then excludes it explicitly through RowBinlogTableWrapper.isRowBinlogSyntheticColumn (AggregateStrategies.java:729-734).

One shape note for the fix: adding version_ordinal() next to the commit-TSO ordinal in segment_zone_maps_can_answer_agg would be wrong, because continue there means "do not disqualify this column". The gate has to set *usable = false when the read schema carries a substituted column, or produce the effective summary for it.

Not verified: no cluster run of the SQL above, and the merge-on-read variant of the BE path is not fully traced.