deepthi912 opened a new pull request, #19009: URL: https://github.com/apache/pinot/pull/19009
## Summary Upsert tables with `deleteRecordColumn` set expose two internal bitmaps: `validDocIds` (latest version per primary key, tombstones **included**) and `queryableDocIds` (same, tombstones **excluded**). Queries only ever see `queryableDocIds` today. This adds an off-by-default query option, `useValidDocIds`, so a query can opt into the `validDocIds` view instead — useful for diagnosing upsert/delete consistency issues (e.g. comparing what a segment considers "valid" vs "queryable" per replica) without the much broader `skipUpsert` bypass (which also surfaces stale/superseded rows, not just tombstones). ```sql SELECT COUNT(*), $hostName FROM my_upsert_table WHERE $segmentName = 'some_segment' OPTION(useValidDocIds='true') GROUP BY $hostName ``` Default (`false`/unset): unchanged, `queryableDocIds`-based filtering as today. ## Design notes - `UpsertViewManager` now maintains a second cached map (`_segmentValidDocIdsMap`), refreshed in the same locked pass (`doBatchRefreshUpsertView`) as the existing `_segmentQueryableDocIdsMap`, so both the query-filter path and segment pruning read a consistent, lock-free snapshot for `SNAPSHOT`-consistency-mode tables — mirroring the existing queryable-docs mechanism rather than inventing a new one. - For `SYNC`-consistency-mode tables, a live per-segment read under the existing read lock is safe and sufficient (mirrors the existing queryable-docs SYNC path). - `IndexSegment` gained a `hasNoValidDocs()` method (mirroring `hasNoQueryableDocs()`) so `SegmentPrunerService` can correctly decide whether to prune a segment under `useValidDocIds=true` based on valid-docs emptiness specifically — a segment that's fully tombstoned (0 queryable docs) but still has valid docs must *not* be pruned in this mode. - `SegmentContext`'s `queryableDocIdsSnapshot` field/getter/setter were renamed to `docIdsSnapshot`, since it can now hold either bitmap depending on the query option — the old name would otherwise be actively misleading. ## Test plan - [x] Unit tests: `QueryOptionsUtilsTest`, `UpsertUtilsTest` (incl. new `hasNoValidDocs` coverage mirroring `hasNoQueryableDocs`), `UpsertViewManagerTest`, `ConcurrentMapTableUpsertMetadataManagerTest` (new — covers the NONE/skipUpsertView direct-read vs. SYNC/SNAPSHOT delegation dispatch), `SegmentPrunerServiceTest`, `FilterPlanNodeTest`, `MetadataAndDictionaryAggregationPlanMakerTest`. - [x] `spotless:apply`, `checkstyle:check`, `license:format`, `license:check` all pass on affected modules. - [ ] No integration/end-to-end test yet exercising `OPTION(useValidDocIds='true')` through a real broker→server query against a table with `deleteRecordColumn` set — flagging this as a known gap for reviewers to weigh in on before merge. -- 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]
