LuciferYang opened a new issue, #67994:
URL: https://github.com/apache/doris/issues/67994
### Search before asking
- [X] I had searched in the issues and found no similar issues.
### Version
master, 846d9b2ebfc.
### What's Wrong?
`__DORIS_COMMIT_TSO_COL__` stores a `0` placeholder on disk in a
single-version segment; the real value is the rowset's `commit_tso`, supplied
at read time by building a `ConstantColumnReader` instead of the on-disk
reader. That substitution goes through a per-segment reader cache that ignores
the constant:
```cpp
Status ColumnReaderCache::get_column_reader(int32_t col_uid, ...,
std::optional<Field> const_value) {
// Attempt to find in cache
if (auto cached = _lookup({col_uid, {}})) {
*column_reader = cached;
return Status::OK();
}
```
`be/src/storage/segment/column_reader_cache.cpp:96-105`. On a hit the cached
reader is returned and `const_value` is dropped, so whichever caller populates
the entry first decides what every later caller gets. The cache is owned by the
`Segment` and outlives a query.
The expression zone-map context builder is a caller that never passes a
constant:
```cpp
Status st = segment->get_column_reader(*tablet_column, &reader,
read_options.stats,
&read_options.io_ctx);
```
`be/src/storage/segment/segment.cpp:129-130`, and the page-level builder
does the same at `be/src/storage/segment/segment_iterator.cpp:3414-3415`. The
segment-level builder runs at `be/src/storage/segment/segment.cpp:485-488`,
inside `Segment::new_iterator`, so it executes before the `SegmentIterator`
exists and therefore before any `Segment::new_column_iterator` call, which is
where the constant would otherwise be installed
(`be/src/storage/segment/segment.cpp:947-966`).
The consequence is not limited to pruning.
`ConstantColumnReader::new_iterator` returns a `ConstantColumnIterator` over
the real value (`be/src/storage/segment/column_reader.h:1093-1096`); the
on-disk reader returns the placeholder. Once the builder has cached the on-disk
reader for that column, every subsequent read of that column on that segment
yields `0` as row data, for as long as the segment stays cached, including for
other queries.
Reaching it needs a pushed common expression that references the column and
is zone-map evaluable, with no `ColumnPredicate` on the same column in the same
query (a `ColumnPredicate` makes `Segment::new_iterator` install the constant
first, at `be/src/storage/segment/segment.cpp:422-437`). `IS NULL` and `IS NOT
NULL` already qualify (`be/src/exprs/function/is_null.h:67`,
`be/src/exprs/function/is_not_null.h:69`), and a column-vs-column comparison
would qualify once expression zone maps support that shape, which is proposed
in #67774; such a comparison can never fall back to a `ColumnPredicate`, since
those are column-vs-literal.
### What You Expected?
A caller that asks for a constant-backed reader gets one, whatever another
caller cached earlier, and a column whose on-disk value is a placeholder never
returns that placeholder as row data.
### How to Reproduce?
Not reproduced end to end; this is a static reading of the cache lookup, the
two builder call sites, and their order inside `Segment::new_iterator`. A unit
test on `ColumnReaderCache` is the direct check: request a column reader
without `const_value`, then request the same column with one, and assert the
second call returns a `ConstantColumnReader`.
### Anything Else?
Correcting the two builder call sites is not sufficient on its own. Other
callers build a bare `StorageReadOptions` without a tablet schema and so cannot
pass a constant either, which the comment at
`be/src/storage/segment/segment.cpp:947-952` already notes for the MOW
partial-update row fetch. The lookup itself is the place where the constant is
dropped, so that is where the fix belongs.
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of
Conduct](https://www.apache.org/foundation/policies/conduct)
--
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]
