github-actions[bot] commented on code in PR #63587:
URL: https://github.com/apache/doris/pull/63587#discussion_r3296456798


##########
be/src/service/point_query_executor.cpp:
##########
@@ -472,6 +472,7 @@ Status PointQueryExecutor::_lookup_row_key() {
             if (hit_cache) {
                 _row_read_ctxs[i]._cached_row_data = std::move(cache_handle);
                 ++_profile_metrics.row_cache_hits;
+                _row_hits++;

Review Comment:
   This cache-hit path still skips the column-store backfill needed when the 
row-store column is partial. `RowCache` stores only `ROW_STORE_COL` from 
`BaseTablet::lookup_row_data()` and is keyed by tablet id + primary key, not by 
the requested projection. For a table using `row_store_columns`, a first point 
query can populate the cache with only the row-store columns; a later point 
query for the same key that requests a column outside that row store hits here, 
increments `_row_hits`, and `_lookup_row_data()` deserializes the cached row 
then `continue`s before the `_reusable->missing_col_uids()` column-store read. 
The padding block at the end then inserts defaults for the missing result 
columns, so the query can return incorrect values. Please either bypass/disable 
the row cache when `_reusable->missing_col_uids()` is non-empty, or still 
perform the row-location lookup on cache hits so the cached row can be 
supplemented from column store.



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