Rishavbhattarai commented on PR #18813: URL: https://github.com/apache/iceberg/pull/18813#issuecomment-6711400628
Thanks @adiranjan25. A predicate-side helper that leaves the bound ordering alone is the fix I had in mind, so I'll review here instead of sending a second patch. Two places still compare with the plain comparator, so `x = 0.0` can still miss `-0.0` rows: 1. `ResidualEvaluator` in `api`. `eq` returns `cmp.compare(ref.eval(struct), lit.value()) == 0 ? alwaysTrue() : alwaysFalse()`, so a float or double identity partition with value `-0.0` gets an `alwaysFalse()` residual for `x = 0.0`. `notEq`, `lt`, `ltEq`, `gt`, `gtEq` and the set predicates in that visitor follow the same pattern. Since it's in the same module, I'd fix it in this PR. 2. `ParquetDictionaryRowGroupFilter` in `parquet`. `eq` and `notEq` build the dictionary set with `lit.comparator()` and call `contains`, so a row group whose dictionary only holds `-0.0` gets skipped for `x = 0.0`. The `lt`/`gt` loops there call `lit.comparator().compare` directly too. This could go in a follow-up if you want to keep this PR to `api`. `ParquetMetricsRowGroupFilter` uses the same comparator, but I don't think it drops rows in practice: parquet-java writes `-0.0` as the min and `+0.0` as the max whenever a zero is present (PARQUET-1222). A test there wouldn't hurt if a committer asks for one. -- 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]
