github-actions[bot] commented on code in PR #68018:
URL: https://github.com/apache/doris/pull/68018#discussion_r4017097815
##########
be/src/storage/segment/column_reader_cache.cpp:
##########
@@ -98,8 +107,12 @@ Status ColumnReaderCache::get_column_reader(int32_t col_uid,
OlapReaderStatistics* stats,
const io::IOContext* source_io_ctx,
std::optional<Field> const_value) {
- // Attempt to find in cache
- if (auto cached = _lookup({col_uid, {}})) {
+ // A caller that passes const_value reads a column whose on-disk value is
a placeholder, so it
+ // must not be served the on-disk reader that a caller without const_value
cached earlier: that
+ // reader would hand back the placeholder both as row data and as a zone
map. Fall through and
+ // build the constant reader, replacing the cached entry so later callers
get the real value too.
+ if (auto cached = _lookup({col_uid, {}});
+ cached != nullptr && (!const_value.has_value() ||
cached->is_constant())) {
Review Comment:
[P1] Keep constant readers out of physical index initialization. With an
inverted index on `__DORIS_COMMIT_TSO_COL__`, an OR such as `tso < 10 OR tso >
20` remains a common expression, so segment expression-zone-map setup first
caches the physical reader. This condition then replaces it with
`ConstantColumnReader` during column-iterator setup; the following bare, eager
index lookup receives that constant and calls non-virtual
`ColumnReader::new_index_iterator` even though its physical type/index state
was never initialized, so the query fails. This is new: for a real TSO greater
than 20, both placeholder 0 and the real value satisfy the OR, so the pre-PR
physical-warmed path returned the correct projected rows. Please skip physical
index creation for constant readers or separate the cache entries, and cover
this ordering in a test.
--
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]
