zeroshade commented on issue #2419: URL: https://github.com/apache/iceberg-go/issues/2419#issuecomment-6637290344
Rejecting at `Bind` with `ErrInvalidArgument` and a pointer to `IsNaN`/`NotNaN` sounds right to me. The constructors can't return an error, and every evaluation path (scan row filters, `ExpressionEvaluator`, the metrics evaluators) binds first. Please add the `releases.md` compatibility note as you suggested. One internal caller needs to change in the same PR: `eqDeletePartitionsToFilter` in `table/conflict_validation.go` builds `LiteralPredicate(OpEQ, col, lit)` from each equality-delete file's partition values with no NaN check, so a NaN identity partition becomes `col = NaN`. That only works today because the partition evaluator's `cmp.Compare` treats NaN as equal to NaN (`TestEqDeleteConflictNaNPartition` relies on it). Once `Bind` rejects NaN literals, serializable conflict validation would fail on those tables. `table/internal/partition_predicate.go` already handles this by emitting `IsNaN(term)` for a NaN partition value; doing the same there should keep that test passing. -- 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]
