arunkumarucet commented on PR #18891: URL: https://github.com/apache/pinot/pull/18891#issuecomment-5042126410
@Vamsi-klu thanks for the detailed trace — the `ReverseDocIdSetOperator.getNextBlock()` / `getReverseIntIterator()` path is exactly the reproducer, and your caveat about the dense fast path was spot on. Addressed in a7c1164. Specifically to your points: - **Dense path must stay ascending-specific.** Agreed — a "scatter with true min/max" fix repairs the sparse branch (`docIdToPos` is order-independent) but a descending block is still gap-free, so it would re-enter the dense guard and mis-scatter via `docId - lo`. The ascending fast path now additionally requires `first <= last`. Rather than let descending blocks fall back to the hash-map branch (a per-block `Int2IntOpenHashMap` alloc on a hot path), I added a **symmetric descending fast path** that writes `values[first - docId]` over `[last, first]`, so `ORDER BY ... DESC` keeps the allocation-free path too. - **Sparse fallback** now computes the block's actual min/max, so its scan bounds are correct regardless of ordering. - **`getValuesMV`** — confirmed it was left on the mask path and never regressed; no change there. - **Regression coverage** — added your suggested end-to-end case: a `JsonPathTest` query projecting `json_extract_index(...)` under `ORDER BY <sortedColumn> DESC` on a genuinely sorted column (with `allowReverseOrder=true`), asserting each row's value rather than degrading to null. It throws `Illegal Json Path` against the pre-fix reader and passes now. Also added a reader-level test with hand-built dense `[2,1,0]` and sparse `[2,0]` descending blocks. - **`flattenedToDocIds` / `convertFlattenedDocIdsToDocIds`** — thanks for confirming the ordered `RoaringBitmapWriter` change is valid; left as is. -- 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]
