arunkumarucet commented on PR #19032: URL: https://github.com/apache/pinot/pull/19032#issuecomment-5128502187
Thanks for the thorough review @KKcorps — since this already merged, I've addressed all three comments in a follow-up: #19128. - **Null rows in negative filters (critical):** `H3IndexFilterOperator` and `H3InclusionIndexFilterOperator` now exclude the column's null doc IDs from the match-all, lower-bound-flip, and negative-check complement results when query null handling is enabled, so skipped null geometries no longer leak into `=false` / lower-bound / match-all results. - **Exception per empty row:** added a `toGeometry()` fast-path for the empty default-null byte array (no per-row `BufferUnderflowException`), and the dictionary reload loop now decodes each dictId at most once instead of per doc. - **Raw path test:** `SegmentPreProcessorTest` now covers `V1/V3 × DICTIONARY/RAW`, and `H3IndexQueriesTest` has a new null-handling query test. One note on the requested query-level comparison against the scan path: the geospatial transform functions throw `BufferUnderflowException` on null/empty geometry input even under null handling, so the scan path can't act as an oracle for all-null rows. The new test asserts the exact non-null count instead (fails pre-fix). If you think the scan path should also tolerate null geometry, I'm happy to take that on in a separate PR. -- 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]
