zeroshade commented on code in PR #2097:
URL: https://github.com/apache/iceberg-go/pull/2097#discussion_r4168426256
##########
exprs_test.go:
##########
@@ -1100,6 +1100,46 @@ func TestVariantBoundLiteralRejectionMessage(t
*testing.T) {
assert.ErrorContains(t, err, "ordered predicates are not supported on
variant fields")
}
+func TestVariantSetPredicate(t *testing.T) {
+ build := func(v any) variant.Value {
+ var b variant.Builder
+ require.NoError(t, b.Append(v))
+ val, err := b.Build()
+ require.NoError(t, err)
+
+ return val
+ }
+
+ one, two := build(int64(1)), build(int64(2))
+ ref := iceberg.Reference("payload")
+
+ t.Run("in", func(t *testing.T) {
+ pred := iceberg.IsIn(ref, one, two, build(int64(1)))
+ require.Implements(t, (*iceberg.UnboundPredicate)(nil), pred)
+ assert.Equal(t, iceberg.OpIn, pred.Op())
+ // The duplicate is a separate buffer, so this only holds if
the set dedups by content.
+ assert.True(t, pred.Equals(iceberg.IsIn(ref, one, two)))
+ })
+
+ t.Run("not in", func(t *testing.T) {
+ pred := iceberg.NotIn(ref, one, two)
+ require.Implements(t, (*iceberg.UnboundPredicate)(nil), pred)
+ assert.Equal(t, iceberg.OpNotIn, pred.Op())
+ assert.True(t, pred.Negate().Equals(iceberg.IsIn(ref, one,
two)))
+ })
+
+ t.Run("bind to variant column", func(t *testing.T) {
+ sc := iceberg.NewSchema(0,
+ iceberg.NestedField{ID: 1, Name: "payload", Type:
iceberg.VariantType{}, Required: false},
+ )
+
+ // Set predicates on variant columns are not supported yet;
this only
+ // guards that binding rejects them instead of panicking.
+ _, err := iceberg.BindExpr(sc, iceberg.IsIn(ref, one, two),
true)
+ require.Error(t, err)
Review Comment:
This passes for any bind failure, not just the variant rejection. Two
distinct values reach the set-predicate fallthrough (`type error: invalid bound
type for set predicate - variant`), so the class can be pinned:
```suggestion
require.ErrorIs(t, err, iceberg.ErrType)
```
##########
exprs_test.go:
##########
@@ -1100,6 +1100,46 @@ func TestVariantBoundLiteralRejectionMessage(t
*testing.T) {
assert.ErrorContains(t, err, "ordered predicates are not supported on
variant fields")
}
+func TestVariantSetPredicate(t *testing.T) {
+ build := func(v any) variant.Value {
+ var b variant.Builder
+ require.NoError(t, b.Append(v))
+ val, err := b.Build()
+ require.NoError(t, err)
+
+ return val
+ }
+
+ one, two := build(int64(1)), build(int64(2))
+ ref := iceberg.Reference("payload")
+
+ t.Run("in", func(t *testing.T) {
+ pred := iceberg.IsIn(ref, one, two, build(int64(1)))
+ require.Implements(t, (*iceberg.UnboundPredicate)(nil), pred)
+ assert.Equal(t, iceberg.OpIn, pred.Op())
+ // The duplicate is a separate buffer, so this only holds if
the set dedups by content.
+ assert.True(t, pred.Equals(iceberg.IsIn(ref, one, two)))
+ })
+
+ t.Run("not in", func(t *testing.T) {
+ pred := iceberg.NotIn(ref, one, two)
+ require.Implements(t, (*iceberg.UnboundPredicate)(nil), pred)
+ assert.Equal(t, iceberg.OpNotIn, pred.Op())
+ assert.True(t, pred.Negate().Equals(iceberg.IsIn(ref, one,
two)))
+ })
+
+ t.Run("bind to variant column", func(t *testing.T) {
+ sc := iceberg.NewSchema(0,
+ iceberg.NestedField{ID: 1, Name: "payload", Type:
iceberg.VariantType{}, Required: false},
+ )
+
+ // Set predicates on variant columns are not supported yet;
this only
+ // guards that binding rejects them instead of panicking.
+ _, err := iceberg.BindExpr(sc, iceberg.IsIn(ref, one, two),
true)
+ require.Error(t, err)
+ })
Review Comment:
#2088 is on `main` now, so the end-to-end case from #2093 can be covered
here. It needs a rebase: at this head the subtest fails with `could not cast
value: VariantLiteral to long`; merged with `main` it passes.
```suggestion
})
t.Run("bind to primitive column", func(t *testing.T) {
sc := iceberg.NewSchema(0,
iceberg.NestedField{ID: 1, Name: "payload", Type:
iceberg.PrimitiveTypes.Int64, Required: false},
)
bound, err := iceberg.BindExpr(sc, iceberg.IsIn(ref, one, two),
true)
require.NoError(t, err)
require.Implements(t, (*iceberg.BoundSetPredicate)(nil), bound)
lits := bound.(iceberg.BoundSetPredicate).Literals()
assert.Equal(t, 2, lits.Len())
assert.True(t, lits.Contains(iceberg.Int64Literal(1)))
assert.True(t, lits.Contains(iceberg.Int64Literal(2)))
})
```
--
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]
