gortiz commented on issue #19339: URL: https://github.com/apache/pinot/issues/19339#issuecomment-5479306087
I've opened #19408 implementing option 1, the execution-time push-down. `applyAnd` is generalized from `ScanBasedDocIdIterator` up to `BlockDocIdSet`, so an AND hands the document ids it has matched to its composite children: `OrDocIdSet` unions the restricted branches, `AndDocIdSet` intersects starting from the candidate set, `NotDocIdSet` subtracts. No heuristic, no duplicated index lookups, any nesting depth, and the eager/lazy cliff goes away rather than being routed around. It ships **disabled**. The push-down materializes the filter result instead of streaming it, so a query that stops at its LIMIT ends up doing the full filter work — measured on the existing suite at 21–41% *more* entries scanned for a selection with LIMIT, against 7–19% fewer for aggregations. Hence `andRestrictionPushdownMode`: `NEVER` (default), `AUTO` (only queries that read every matching document, i.e. aggregation and group-by), `ALWAYS`. @alexch2000 — I think this covers your shape, and I'd appreciate a check against your data. Your query is an aggregation with a GROUP BY, so `AUTO` enables it. The 83 segments where the plan is `FILTER_OR → 2× FILTER_FULL_SCAN` should now receive the candidate set from `OR(cohorts) ∩ device_os` instead of evaluating each range predicate over the whole column. Worth spelling out *why* it is so bad today: an OR left lazy is driven through `advance(target)`, and `SVScanDocIdIterator.advance` scans **forward from** the target until it finds a match — so on a segment where the predicate matches nothing it walks to the end. The push-down replaces that with a batched `applyAnd` bounded by the candidate set, which is where your ~441M goes. @richardstartin — I like the `RangeBitmap` context suggestion a lot, and I think this PR is a good short-term fix that yours can be built on top of rather than an alternative to it. Two reasons: 1. Something has to deliver a bitmap to the leaf, and today nothing carries one across an OR boundary — `AndDocIdSet` only pushes into its own scan children, and `StarTreeFilterOperator` and `FilterPlanNode#wirePreFilterForVectorOperators` are both AND-only. This PR is what makes a range index nested under an OR reachable at all. 2. The wiring already exists for another index type. `wirePreFilterForVectorOperators` does exactly the shape you're describing — partition the AND's children, check `FilterAwareVectorIndexReader#supportsPreFilter()`, check the producers can cheaply produce bitmaps, apply a selectivity cost model, then push the bitmap in. A `FilterAwareRangeIndexReader` forwarding to `rangeBitmap.between(min, max, context)` looks like a fairly mechanical generalization of it. The real blocker for the context parameter is one level below this PR: `AndFilterOperator#getTrues()` evaluates every child eagerly, so `BitSlicedRangeIndexReader` has already produced its full bitmap by the time any `BlockDocIdSet` exists. Consuming a context needs the restriction to arrive at the *operator* level, which means ordered rather than eager evaluation of the AND's children. Happy to open that as a separate issue if you think that framing is right — it is also the one that would speed up a plain `AND(a = 1, ts BETWEEN …)` with no OR anywhere, which this PR does nothing for. For completeness on the other option in the description: I don't think the planner rewrite in #19350 is the right primary fix. It only distributes single-column EQ/IN, so it cannot reach @alexch2000's case; it costs an extra index read per branch on the common all-indexed OR, where there is nothing to gain; and it leaves the cliff in place — adding an index to a column inside an OR branch still flips that branch to the eager path, just with `P` inside it now. -- 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]
