[PR] Ignore rows ingested after planning in H3, JSON_MATCH and null bitmap filters [pinot]

["Tarun-kishore (via GitHub)" <gi...@apache.org>]

Tarun-kishore opened a new pull request, #20043:
URL: https://github.com/apache/pinot/pull/20043

   Part of #20041: this PR covers the H3, JSON_MATCH and null value vector filters. The inverted index part is #PR1_NUMBER.
   
   ### Problem
   
   On a consuming segment, the H3 index, the JSON index and the null value vector hand out bitmaps that can hold rows ingested after `numDocs` was read at planning time. The filters on top of them either flip those bitmaps over `[0, numDocs)`, which turns the new rows into matches, or count them. As a result:
   
   - `ST_Distance(...) > d` and `ST_Within(...) = 0` return rows near the point or inside the polygon.
   - `COUNT(*)` with `JSON_MATCH` or `NOT JSON_MATCH` is off.
   - `IS NOT NULL` can return null rows ingested during planning.
   
   See #20041 for details.
   
   ### Change
   
   Every filter that reads one of these bitmaps now limits it to `[0, numDocs)`, so a leaf filter never hands out a row at or above `numDocs`:
   
   - `FilterOperatorUtils.limitToNumDocs(bitmap, numDocs)` (new): returns the bitmap as is when it holds no row at or above `numDocs`, otherwise a trimmed copy. It never modifies its input, so read-only and memory-mapped bitmaps stay safe. `FilterOperatorUtils.getNullBitmap(dataSource, numDocs)` (new) applies it to the null value vector.
   - `BitmapBasedFilterOperator` limits its rows and its null bitmap. This covers `IS NULL`, `IS NOT NULL`, the always true / always false leaves under null handling, and map filters.
   - `BaseColumnFilterOperator` and the always true / always false leaves in `FilterOperatorUtils` read the null bitmap limited to `numDocs`. A column whose only null rows are new is treated as having no nulls.
   - `H3IndexFilterOperator` ORs the cell bitmaps through one helper that drops the rows at or above `numDocs`, in place of the 5 copies of that loop. `H3InclusionIndexFilterOperator` trims both of its unions.
   - `JsonMatchFilterOperator` limits the JSON index result.
   
   Immutable segments are not affected: their bitmaps never hold a row at or above `numDocs`, so the check is one `isEmpty()` and one `last()` call, and nothing is copied.
   
   ### Tests
   
   - `H3IndexFilterOperatorConsumingSegmentTest` (new): plans each query on a real consuming segment with an H3 index, ingests a row near the point and a far one, then runs it. It covers the distance lower bound and upper bound, and the negative and positive `ST_Within` checks.
   - `JsonMatchFilterOperatorConsumingSegmentTest` (new): covers the rows of `JSON_MATCH` (`=` and `!=`) and `NOT JSON_MATCH` ANDed with a scan filter. It also covers the fast `COUNT(*)` path for `=`, `!=`, `NOT`, an OR, and an AND of `NOT`s.
   - `BitmapBasedFilterOperatorTest` (new): bitmaps with rows at and above `numDocs`, on both sides of exclusive, with and without a null bitmap. It also checks that the input bitmaps are not modified.
   - `InvertedIndexFilterOperatorTest.testNullRowsAtOrAboveNumDocsAreIgnored` (new): a null value vector with rows at and above `numDocs` on an exclusive predicate.
   
   Without the fix, 14 of the new cases fail:
   - the 4 H3 cases (each returns the new near row)
   - 7 of the 8 JSON cases. For example, `COUNT(*)` with `NOT JSON_MATCH` returns 2 instead of 4, and the AND of `NOT`s returns 0 instead of 3. Only the rows of `NOT JSON_MATCH` were already right.
   - the 3 unit tests


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Ignore rows ingested after planning in H3, JSON_MATCH and null bitmap filters [pinot]

["xiangfu0 (via GitHub)" <gi...@apache.org>]

xiangfu0 commented on code in PR #20043:
URL: https://github.com/apache/pinot/pull/20043#discussion_r4376996252


##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/JsonMatchFilterOperator.java:
##########
@@ -124,10 +124,14 @@ private ImmutableRoaringBitmap getMatchingDocIdBitmap() {
     // getMatchingDocIds may return a read-only bitmap backed by the index's (possibly memory-mapped) storage. This
     // operator only iterates it (BitmapDocIdSet / BitmapCollection / getCardinality) within the segment's acquired
     // lifetime and never mutates it, so a borrowed posting list can be consumed directly without a copy.

Review Comment:
   This existing comment still says the borrowed posting list is "consumed directly without a copy," but `limitToNumDocs` below now copies it on a consuming segment when it holds docs >= numDocs. Worth tweaking the wording so the two comments don't contradict each other.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org