LuciferYang commented on issue #67995:
URL: https://github.com/apache/doris/issues/67995#issuecomment-5675647238

   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.
   


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