mattfaltyn commented on code in PR #1872:
URL: https://github.com/apache/iceberg-go/pull/1872#discussion_r3850768034
##########
exprs_test.go:
##########
@@ -997,6 +997,31 @@ func TestBindAboveBelowIntMax(t *testing.T) {
assert.Equal(t, tt.exp, b)
})
}
+
+ t.Run("set predicates", func(t *testing.T) {
+ tests := []struct {
+ name string
+ pred iceberg.BooleanExpression
+ expected iceberg.BooleanExpression
+ }{
+ {"int in mixed range", iceberg.IsIn(ref, int64(34),
above, below), iceberg.EqualTo(ref, int32(34))},
+ {"int not in mixed range", iceberg.NotIn(ref,
int64(34), above, below), iceberg.NotEqualTo(ref, int32(34))},
+ {"int in outside range", iceberg.IsIn(ref, above,
below), iceberg.AlwaysFalse{}},
+ {"int not in outside range", iceberg.NotIn(ref, above,
below), iceberg.AlwaysTrue{}},
+ {"float in mixed range", iceberg.IsIn(ref2,
float64(34), above2, below2), iceberg.EqualTo(ref2, float32(34))},
+ {"float not in outside range", iceberg.NotIn(ref2,
above2, below2), iceberg.AlwaysTrue{}},
Review Comment:
Thanks — added the two symmetric float cases and a multi-survivor IN case in
1b28a8d, so the filtered bound-set branch is covered too.
##########
exprs_test.go:
##########
@@ -997,6 +997,31 @@ func TestBindAboveBelowIntMax(t *testing.T) {
assert.Equal(t, tt.exp, b)
})
}
+
+ t.Run("set predicates", func(t *testing.T) {
+ tests := []struct {
+ name string
+ pred iceberg.BooleanExpression
+ expected iceberg.BooleanExpression
+ }{
+ {"int in mixed range", iceberg.IsIn(ref, int64(34),
above, below), iceberg.EqualTo(ref, int32(34))},
+ {"int not in mixed range", iceberg.NotIn(ref,
int64(34), above, below), iceberg.NotEqualTo(ref, int32(34))},
+ {"int in outside range", iceberg.IsIn(ref, above,
below), iceberg.AlwaysFalse{}},
+ {"int not in outside range", iceberg.NotIn(ref, above,
below), iceberg.AlwaysTrue{}},
+ {"float in mixed range", iceberg.IsIn(ref2,
float64(34), above2, below2), iceberg.EqualTo(ref2, float32(34))},
+ {"float not in outside range", iceberg.NotIn(ref2,
above2, below2), iceberg.AlwaysTrue{}},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ bound, err := iceberg.BindExpr(sc, tt.pred,
true)
+ require.NoError(t, err)
+ expected, err := iceberg.BindExpr(sc,
tt.expected, true)
Review Comment:
Thanks — renamed the bound expectation to wantBound in 1b28a8d.
##########
table/evaluators_test.go:
##########
@@ -1929,6 +1929,7 @@ func (suite *InclusiveMetricsTestSuite) TestInMetrics() {
{iceberg.IsIn(ref, IntMinValue-1, IntMinValue), true, "should
read: id equal to lower bound"},
{iceberg.IsIn(ref, IntMaxValue-4, IntMaxValue-3), true, "should
read: id between upper and lower bounds"},
{iceberg.IsIn(ref, IntMaxValue, IntMaxValue+1), true, "should
read: id equal to upper bound"},
+ {iceberg.IsIn(ref, int64(IntMaxValue), int64(math.MaxInt32)+1),
true, "should read: ignore value outside int32 range"},
Review Comment:
Thanks — added the NOT IN regression in 1b28a8d under strict metrics, where
NOT IN inspects the set and reproduces the pre-fix sentinel failure. It keeps
two in-range survivors so the fixed path remains a bound NOT IN set.
--
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]
