hcrosse commented on issue #2416: URL: https://github.com/apache/iceberg-go/issues/2416#issuecomment-6635970033
Once https://github.com/apache/arrow-go/pull/1448 is merged and released, fixing this should mostly be an arrow-go bump plus tests. Before I open the PR, I'd like to check what `NotStartsWith` should do with a NULL value in a scan. The translation already compiles it to `not(starts_with(...))`, so with the new kernel a NULL value gives NULL and the scan drops the row. That matches `NotEqual`, and `NotIn` since https://github.com/apache/iceberg-go/pull/2356: https://github.com/apache/iceberg-go/blob/8ef0a092fcbaf70f99aa8935481bc95071eb41b1/table/substrait/substrait.go#L480-L488 The row evaluator keeps the row instead, because it negates `StartsWith`, which is false for NULL: https://github.com/apache/iceberg-go/blob/8ef0a092fcbaf70f99aa8935481bc95071eb41b1/visitors.go#L516-L539 The [expressions spec](https://github.com/apache/iceberg/blob/f12ec1b5c6f1c88a941b0ebd3a9064fb45264efa/format/expressions-spec.md#boolean-logic), added in https://github.com/apache/iceberg/pull/16652, sides with the row evaluator. It says predicates are 2-valued, that `NOT STARTS WITH` is true when the constant is not a prefix of the value, and that `34 != null` is true. Engines that use SQL 3-valued logic must add `IS NULL` and `IS NOT NULL` checks themselves. https://github.com/apache/iceberg/issues/18701 is discussing the same question for Java's `notEq` and `notIn`. I plan to keep the current translation, so scans drop NULL rows for `NotStartsWith` as they do for `NotEqual` and `NotIn`, and to test that. Following the spec would mean treating a NULL `StartsWith` as false before `NOT` is applied, and changing the copy-on-write delete complement to stop keeping NULL rows, so I think it belongs in its own issue covering all the negated predicates. Does that sound right, or would you rather this PR follow the spec for `NotStartsWith`? -- 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]
