LuciferYang commented on PR #68018:
URL: https://github.com/apache/doris/pull/68018#issuecomment-5686901763

   Fixed, with one correction to the framing.
   
   The mechanism is real. `Segment::new_index_iterator` fetches the reader 
without a constant (`be/src/storage/segment/segment.cpp:1069`) and calls 
`reader->new_index_iterator` (`:1108`), which was not virtual; the base 
implementation branches on the raw `_type` / `_meta_type` members that 
`ConstantColumnReader` never sets, so it falls through to 
`INVERTED_INDEX_NOT_SUPPORTED` and the query fails.
   
   It is not new to this PR, though. The predicate loop in 
`Segment::new_iterator` (`:422-437`) already installs the constant reader for 
that column before `SegmentIterator` creates index iterators 
(`be/src/storage/segment/segment_iterator.cpp:1853`, `:1885`), so an ordinary 
`ColumnPredicate` on an indexed `__DORIS_COMMIT_TSO_COL__` reaches the same 
failure today. This PR adds one more ordering, the physically warmed one you 
describe.
   
   I also could not confirm the premise that such an index can exist: 
`processAddIndex` resolves the column with `olapTable.getColumn(col)` and hands 
it to `indexDef.checkColumn(...)`, and I found no visibility rejection on that 
path. So I treated it as plausible rather than proven and fixed it defensively.
   
   The fix is on the reader rather than at the call site: `new_index_iterator` 
is now virtual and `ConstantColumnReader` overrides it to leave the iterator 
unset, exactly as the existing no-reader path does, so the caller falls back to 
reading through the constant. Two reasons for that placement. "I have no index" 
is knowledge the class has and each call site does not, and it also covers the 
pre-existing ordering above. It is unit-testable without a fixture: 
`NewIndexIteratorIsANoOp`, mutation-checked by delegating the override to the 
base, which fails with `[E-6002] Failed to load inverted index: index metadata 
is null`.
   
   The full ordering test you asked for needs a segment carrying an inverted 
index on that hidden column, which I could not build. What I have instead is 
the reader refusing regardless of how it reached index setup.
   
   `ConstantColumnIteratorTest` and `ColumnReaderCacheTest` are 26 for 26 
locally, clang-format 16 clean.
   


-- 
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]

Reply via email to