[Bug] Read-time hidden columns return placeholders in direct reads and use incorrect pruning/statistics

[HappenLee]

Search before asking

Related issues #67994 and #67995 describe reader-cache and ZoneMap failures. This report tracks the broader consistency problem across hidden-column pruning, statistics, and direct reads, including short-circuit point queries and two-phase row-ID fetches that do not use the normal scan materialization path.

Version

Apache Doris master. The existing reader/cache and pruning reports describe baseline 846d9b2ebfc595ddfd77579447db16a674c901f7.

Related proposed implementation: #68125, inspected at 0022410d4b3cddbb48c548c47bc22b4a7e99d5d8. Its regression scenarios are referenced below. No new cluster reproduction or test execution was performed when filing this report.

What's Wrong?

Some hidden columns store placeholders in segment files and obtain their logical values from the owning rowset during a read:

• __DORIS_VERSION_COL__: a single-version rowset can supply its version instead of the physical 0.
• __DORIS_COMMIT_TSO_COL__: a published single-version rowset can supply its assigned commit TSO instead of the physical 0.
• __DORIS_BINLOG_TSO__: the row-binlog read path can materialize a TSO instead of its physical placeholder, subject to the row-binlog read context.

These semantics are not interchangeable with reading the physical column or its indexes. Several paths can bypass or precede normal hidden-column materialization:

1. Pruning: physical segment/page ZoneMaps, Bloom filters, or inverted indexes can reject rows based on placeholders rather than the logical values that row evaluation will see.
2. Statistics: a pushed-down MIN/MAX path can consume placeholder statistics rather than the logical hidden-column values.
3. Reader reuse: a reader cache keyed only by column identity can return a physical reader when a logical constant is required, or reuse a context-dependent constant where a physical read is intended.
4. Direct reads: short-circuit point queries and lazy two-phase row-ID fetches can read physical column values or deserialize a JSONB row-store payload without performing the replacement used by a normal SegmentIterator scan. A JSONB row cache also lacks the owning rowset context needed to derive the logical value.

As a result, the returned hidden value or matching row set can depend on the execution path, cache warmup order, or whether row storage is enabled.

What You Expected?

Logical projection, predicate evaluation, index pruning, and pushed-down statistics must agree on the hidden-column values derived from the exact owning rowset.

Point queries and two-phase reads must return the same logical VERSION/COMMIT_TSO values as normal scans. Multi-version rowsets, unpublished internal reads, deliberately physical accesses, and row-binlog-specific BINLOG_TSO handling must retain their respective semantics; they must not receive an unconditional constant replacement.

How to Reproduce?

The following point-query scenario is adapted from the public regression case in #68125. It is a proposed comparison for an affected build; it has not been rerun for this report.

CREATE TABLE hidden_column_point_read (
    k INT,
    v INT
) UNIQUE KEY(k)
DISTRIBUTED BY HASH(k) BUCKETS 1
PROPERTIES (
    "replication_num" = "1",
    "enable_unique_key_merge_on_write" = "true",
    "light_schema_change" = "true",
    "store_row_column" = "true",
    "disable_auto_compaction" = "true"
);

INSERT INTO hidden_column_point_read VALUES (1, 10);
SET show_hidden_columns = true;

SELECT __DORIS_VERSION_COL__
FROM hidden_column_point_read WHERE k = 1;

SELECT /*+ SET_VAR(enable_short_circuit_query=false) */
       __DORIS_VERSION_COL__
FROM hidden_column_point_read WHERE k = 1;

Verify that the first query actually uses the short-circuit path. Both queries should return the same positive published version, rather than one path returning the physical 0. Repeat with partial row storage and with an ordinary-column point query first to warm the row cache.

Additional validation should cover:

• Two singleton rowsets whose sort keys interleave in a lazy TopN result. Assert that the plan contains PhysicalLazyMaterialize, and compare each fetched row's hidden version with its source rowset. Exercise both column storage and full row storage; replacement must apply only to the newly appended rows.
• COMMIT_TSO projection and predicates on a ROW-binlog-enabled base table, using source rowset metadata as an independent oracle.
• Ordinary predicates and common expressions at both segment and page levels, with reader caches initially cold and warm.
• MIN/MAX with execution-plan assertions proving that the statistics pushdown path is active. A query that silently falls back to a normal scan does not validate that optimization.
• BINLOG_TSO through the actual row-binlog scan context, plus controls for physical/internal and multi-version reads.

Anything Else?

The proposed direction in #68125 is to centralize hidden-column identification and read-time value resolution, use logical summaries where available, and avoid pruning from physical indexes that do not describe those values. Context-dependent logical constants should not pollute the shared physical reader cache. Direct reads must retain the exact owning rowset and resolve VERSION/COMMIT_TSO accordingly; BINLOG_TSO materialization needs the row-binlog scan context.

Statistics fast paths should be used only when the reader can supply the correct logical result; otherwise they should fall back to normal iteration.

Public implementation and test references:

• Hidden-column value resolution.
• Point-query and interleaved lazy-fetch regression scenarios.
• [fix](be) Handle read-time hidden columns in reads and pruning #68125 addresses the broader hidden-column read contract.
• [fix](be) Resolve commit TSO before accessing physical column readers #68097 addresses the COMMIT_TSO reader-entry-point and direct-read subset.
• [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 and [Bug] Zone-map pruning trusts placeholder summaries for the hidden TSO and version columns, dropping matching rows #67995 contain the earlier cache and ZoneMap analyses.

[Doris-Breakwater]

Breakwater-GitHub-Analysis-Slot: slot_f821e4f614d8

Initial assessment

This is a credible data-correctness bug on master, not just a performance/optimization issue. Static inspection of the cited baseline 846d9b2ebfc595ddfd77579447db16a674c901f7 confirms the underlying contract mismatch: VERSION, COMMIT_TSO, and row-binlog BINLOG_TSO may be logical rowset/context-derived values, while some consumers still see physical segment placeholders. That can produce both wrong projected values and false-negative pruning. The issue currently has no labels, assignee, milestone, or linked development item; it should be linked to #68125 and labeled as a storage/read-path data-correctness bug according to the repository's label taxonomy.

Code-confirmed facts

• The normal SegmentIterator path explicitly replaces a singleton rowset's physical VERSION placeholder and performs BINLOG_TSO replacement only when read_row_binlog is set. This proves that the physical value and the logical scan value intentionally differ in those contexts.
• At the cited baseline, segment/page predicate pruning and expression ZoneMap evaluation can obtain summaries from the physical ColumnReader. The COMMIT_TSO constant-reader support is also vulnerable to cache order because ColumnReaderCache is keyed by column UID/path and returns a cached reader before considering the requested constant.
• Segment::seek_and_read_by_rowid only reads the selected physical column through a ColumnIterator; it has no post-read VERSION/COMMIT_TSO replacement. Both short-circuit point queries and lazy two-phase row-ID fetches call this entry point with read options that do not carry the owning rowset's logical value.
• The point-query full-row-store path decodes JSONB directly. A row-cache hit is keyed by tablet/key and bypasses row-location/rowset lookup, so that cached payload cannot independently derive the owning rowset's VERSION or COMMIT_TSO.
• The lazy row-ID path does retain the exact BetaRowset, which is sufficient to repair the result per batch, but the baseline does not use that metadata after either JSONB decoding or column reads.
• MIN/MAX pushdown can use the statistics iterator and physical ZoneMaps instead of normal scan materialization. VERSION therefore needs a normal-iteration fallback unless the statistics reader can expose the logical singleton; COMMIT_TSO can retain the fast path only when its constant reader is correct and isolated from the physical-reader cache.

These facts explain the reported execution-path and cache-warmup dependence. They also show why an unconditional replacement would be wrong: multi-version rowsets, unassigned COMMIT_TSO, intentionally physical/internal reads, and BINLOG_TSO outside a row-binlog read do not share the same semantics.

Assessment of the proposed fix

At the inspected and current #68125 head, 0022410d4b3cddbb48c548c47bc22b4a7e99d5d8, the design is directionally consistent with the required contract:

• hidden-column identification and value resolution are centralized;
• segment predicates/common-expression ZoneMaps use a synthetic logical singleton;
• physical page ZoneMap, Bloom-filter, and inverted-index pruning are bypassed when a read-time constant applies;
• context-dependent constant readers do not enter the UID-only physical reader cache;
• MIN/MAX falls back to SegmentIterator for logical constants not supported by the statistics reader, while COMMIT_TSO retains its constant-reader fast path;
• point reads resolve the owning rowset, avoid context-free row-cache hits when a read-time hidden column is projected, and replace only the newly decoded row;
• two-phase row-ID reads replace only the newly appended suffix using the exact batch rowset; and
• direct reads intentionally do not synthesize BINLOG_TSO, leaving that context-dependent behavior in SegmentIterator.

This is a static assessment, not runtime verification. I did not build Doris or run a cluster/test suite during this triage. The issue itself also states that its reproduction was not rerun.

Missing evidence / validation gaps

Before closing this as fixed, please attach results from an affected baseline and from the proposed fix head:

1. Exact outputs for the two point queries, plus EXPLAIN or profile evidence that the first query used the short-circuit path. Use the source rowset version as the expected-value oracle rather than only asserting > 0.
2. The same point test for full and partial row storage, both with a cold row cache and after an ordinary-column point query warms it.
3. Lazy TopN plan output containing PhysicalLazyMaterialize, with interleaved singleton rowsets and exact per-row VERSION expectations for both column-store and full-row-store fetches.
4. COMMIT_TSO projection and predicates with the owning rowset metadata as the oracle, including direct reads and both physical-reader-first and logical-reader-first cache order.
5. Segment- and page-level ordinary predicates/common expressions with cold and warm caches, and explicit coverage of ZoneMap, Bloom-filter, and inverted-index behavior.
6. MIN/MAX plan/profile evidence proving which queries use VStatisticsIterator and which deliberately fall back; result equality alone does not validate the optimization path.
7. Actual row-binlog BINLOG_TSO coverage, plus negative controls for multi-version rowsets, unpublished/unassigned COMMIT_TSO, non-row-binlog reads, and intentionally physical internal reads.

The added tests in #68125 cover many important mechanics, including exact 2,3,2,3 lazy-fetch VERSION values, both reader-cache orders, VERSION statistics fallback, and COMMIT_TSO statistics use. The main remaining acceptance gaps are end-to-end path proof for short-circuit and MIN/MAX, row-cache warmup, direct COMMIT_TSO, and the positive/negative row-binlog context matrix.

Recommended next step

Use #68125 as the primary fix, run its targeted BE test (SegmentIteratorExprZonemapTest.*) and point-query regression on both the baseline and PR head, then add the missing assertions above before merge. If those tests pass, this issue can serve as the umbrella tracker for #67994/#67995 and the direct-read/statistics extensions; no separate speculative fix is warranted. If any path still disagrees, capture the query profile, exact commit SHA, table properties, rowset version/commit TSO metadata, cache warmup order, and storage mode before assigning a new root cause.