zeroshade commented on code in PR #1679:
URL: https://github.com/apache/iceberg-go/pull/1679#discussion_r3866030587
##########
exprs.go:
##########
@@ -1116,12 +1128,40 @@ func (u *UnboundTransform) Equals(other UnboundTerm)
bool {
if !ok {
return false
}
+ if u == nil || rhs == nil {
+ return u == rhs
+ }
+
+ leftNil, rightNil := isNilTransform(u.transform),
isNilTransform(rhs.transform)
+ if leftNil || rightNil {
+ if !leftNil || !rightNil || reflect.TypeOf(u.transform) !=
reflect.TypeOf(rhs.transform) {
+ return false
+ }
+ if u.term == nil || rhs.term == nil {
+ return u.term == nil && rhs.term == nil
+ }
+
+ return u.term.Equals(rhs.term)
+ }
return u.transform.Equals(rhs.transform) && u.term.Equals(rhs.term)
Review Comment:
Non-nil pointers to the built-in transforms are valid `Transform` values—the
existing APIs and tests already use forms such as `&BucketTransform{...}`.
However, the built-in `Equals` implementations only accept value forms, so
delegating here breaks equality reflexivity for pointer transforms.
I reproduced this 10/10 times with
`NewUnboundTransform(&BucketTransform{NumBuckets: 16}, ref)`: the unbound term
does not equal itself, its bound predicate does not equal itself, and
projecting that predicate through the identical pointer transform returns `nil`
rather than removing the matching transform.
The typed-nil regressions cover the nil pointer case but miss the adjacent
non-nil pointer case. Please make built-in transform equality pointer/value
aware or normalize built-in pointers, and add regressions for unbound equality,
bound-predicate equality, and same-transform projection.
--
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]
