zeroshade commented on code in PR #1675:
URL: https://github.com/apache/iceberg-go/pull/1675#discussion_r3926372642
##########
table/internal/partition_predicate.go:
##########
@@ -151,6 +196,110 @@ func BuildPartitionMatchPredicate(spec
iceberg.PartitionSpec, schema *iceberg.Sc
return result, nil
}
+func partitionTerm(transform iceberg.Transform, name string)
iceberg.UnboundTerm {
+ ref := iceberg.Reference(name)
+ if isIdentityTransform(transform) {
+ return ref
+ }
+ if isVoidTransform(transform) {
+ // Use the value form so binding the null predicate can fold it
to
Review Comment:
**minor** — partitionTerm's void branch is unreachable dead code with a
misleading comment
partitionTerm is called from only two sites (:141 and :155), both reached
only when fr.isVoid is false, and fr.isVoid is set exactly when
isVoidTransform(f.Transform) is true. The void branch at :204-208 can therefore
never execute. Its comment ('Use the value form so binding the null predicate
can fold it to AlwaysTrue') describes an outcome that never occurs, because
void fields never produce a predicate at all — they are skipped at :136-142.
Delete the branch and the comment.
##########
table/internal/partition_predicate_test.go:
##########
@@ -276,38 +290,406 @@ func
TestBuildPartitionMatchPredicate_NoSeparatorCollision(t *testing.T) {
assert.True(t, expr.Equals(want), "distinct tuples must not be deduped,
got %s", expr)
}
-func TestBuildPartitionMatchPredicate_NonIdentityTransformRejected(t
*testing.T) {
+func TestBuildPartitionMatchPredicate_UsesNonIdentityTransform(t *testing.T) {
cases := []struct {
name string
- transform iceberg.Transform
+ field iceberg.PartitionField
+ source string
+ value any
+ wantValue iceberg.Literal
}{
- {"bucket", iceberg.BucketTransform{NumBuckets: 4}},
- {"truncate", iceberg.TruncateTransform{Width: 10}},
- {"day", iceberg.DayTransform{}},
+ {
+ name: "bucket",
+ field: iceberg.PartitionField{SourceIDs: []int{1},
FieldID: 1000, Name: "id_part", Transform: iceberg.BucketTransform{NumBuckets:
4}},
+ source: "id",
+ value: int32(1),
+ wantValue: iceberg.Int32Literal(1),
+ },
+ {
+ name: "truncate",
+ field: iceberg.PartitionField{SourceIDs: []int{2},
FieldID: 1000, Name: "category_part", Transform:
iceberg.TruncateTransform{Width: 3}},
+ source: "category",
+ value: "boo",
+ wantValue: iceberg.StringLiteral("boo"),
+ },
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
- spec := identitySpec(iceberg.PartitionField{SourceIDs:
[]int{1}, FieldID: 1000, Name: "id_part", Transform: tc.transform})
+ spec := specWithFields(tc.field)
- _, err := BuildPartitionMatchPredicate(spec,
dynamicOverwriteSchema(), []map[int]any{{1000: int32(1)}})
- require.Error(t, err)
- assert.ErrorIs(t, err, iceberg.ErrNotImplemented)
+ expr, err := BuildPartitionMatchPredicate(spec,
dynamicOverwriteSchema(), []map[int]any{{1000: tc.value}})
+ require.NoError(t, err)
+
+ want := iceberg.LiteralPredicate(iceberg.OpEQ,
+ iceberg.NewUnboundTransform(tc.field.Transform,
iceberg.Reference(tc.source)),
+ tc.wantValue,
+ )
+ assert.True(t, expr.Equals(want), "want %s, got %s",
want, expr)
})
}
}
+func bucketValueForInt32(t *testing.T, transform iceberg.BucketTransform,
value int32) int32 {
+ t.Helper()
+
+ result := transform.Apply(iceberg.Optional[iceberg.Literal]{
+ Valid: true,
+ Val: iceberg.Int32Literal(value),
+ })
+ require.True(t, result.Valid)
+
+ return result.Val.(iceberg.Int32Literal).Value()
+}
+
+func TestBuildPartitionMatchPredicate_EvaluatesTransforms(t *testing.T) {
+ bucketTransform := iceberg.BucketTransform{NumBuckets: 4}
+ bucketPartition := bucketValueForInt32(t, bucketTransform, 1)
+ bucketNonMatch := int32(2)
+ for bucketValueForInt32(t, bucketTransform, bucketNonMatch) ==
bucketPartition {
+ bucketNonMatch++
+ }
+
+ cases := []struct {
+ name string
+ sourceType iceberg.Type
+ transform iceberg.Transform
+ partitionValue any
+ matchingValues []any
+ nonMatchingVals []any
+ }{
+ {
+ name: "bucket",
+ sourceType: iceberg.PrimitiveTypes.Int32,
+ transform: bucketTransform,
+ partitionValue: bucketPartition,
+ matchingValues: []any{int32(1)},
+ nonMatchingVals: []any{bucketNonMatch},
+ },
+ {
+ name: "truncate",
+ sourceType: iceberg.PrimitiveTypes.String,
+ transform: iceberg.TruncateTransform{Width: 3},
+ partitionValue: "boo",
+ matchingValues: []any{"boo", "books", "booster"},
+ nonMatchingVals: []any{"bar", "science"},
+ },
+ {
+ name: "year",
+ sourceType: iceberg.PrimitiveTypes.Timestamp,
+ transform: iceberg.YearTransform{},
+ partitionValue: int32(50), // 2020 - 1970
+ matchingValues: []any{
+ timestampAtUTC(2020, 1, 1, 0, 0, 0, 0),
+ timestampAtUTC(2020, 12, 31, 23, 59, 59,
999999),
+ },
+ nonMatchingVals: []any{
+ timestampAtUTC(2019, 12, 31, 23, 59, 59,
999999),
+ timestampAtUTC(2021, 1, 1, 0, 0, 0, 0),
+ },
+ },
+ {
+ name: "month",
+ sourceType: iceberg.PrimitiveTypes.Timestamp,
+ transform: iceberg.MonthTransform{},
+ partitionValue: int32(601), // 2020-02, relative to
1970-01
+ matchingValues: []any{
+ timestampAtUTC(2020, 2, 1, 0, 0, 0, 0),
+ timestampAtUTC(2020, 2, 29, 23, 59, 59, 999999),
+ },
+ nonMatchingVals: []any{
+ timestampAtUTC(2020, 1, 31, 23, 59, 59, 999999),
+ timestampAtUTC(2020, 3, 1, 0, 0, 0, 0),
+ },
+ },
+ {
+ name: "day",
+ sourceType: iceberg.PrimitiveTypes.Timestamp,
+ transform: iceberg.DayTransform{},
+ partitionValue: dateAtUTC(2020, 2, 29),
+ matchingValues: []any{
+ timestampAtUTC(2020, 2, 29, 0, 0, 0, 0),
+ timestampAtUTC(2020, 2, 29, 23, 59, 59, 999999),
+ },
+ nonMatchingVals: []any{
+ timestampAtUTC(2020, 2, 28, 23, 59, 59, 999999),
+ timestampAtUTC(2020, 3, 1, 0, 0, 0, 0),
+ },
+ },
+ {
+ name: "hour",
+ sourceType: iceberg.PrimitiveTypes.Timestamp,
+ transform: iceberg.HourTransform{},
+ partitionValue: int32(439714), // 2020-02-29 10:00 UTC,
relative to 1970
+ matchingValues: []any{
+ timestampAtUTC(2020, 2, 29, 10, 0, 0, 0),
+ timestampAtUTC(2020, 2, 29, 10, 59, 59, 999999),
+ },
+ nonMatchingVals: []any{
+ timestampAtUTC(2020, 2, 29, 9, 59, 59, 999999),
+ timestampAtUTC(2020, 2, 29, 11, 0, 0, 0),
+ },
+ },
+ {
+ name: "hour before epoch",
+ sourceType: iceberg.PrimitiveTypes.Timestamp,
+ transform: iceberg.HourTransform{},
+ partitionValue: int32(-1), // 1969-12-31 23:00 UTC
+ matchingValues: []any{
+ timestampAtUTC(1969, 12, 31, 23, 0, 0, 0),
+ timestampAtUTC(1969, 12, 31, 23, 59, 59,
999999),
+ },
+ nonMatchingVals: []any{
+ timestampAtUTC(1969, 12, 31, 22, 59, 59,
999999),
+ timestampAtUTC(1970, 1, 1, 0, 0, 0, 0),
+ },
+ },
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ schema := iceberg.NewSchema(0, iceberg.NestedField{
+ ID: 1, Name: "value", Type: tc.sourceType,
+ })
+ spec := specWithFields(iceberg.PartitionField{
+ SourceIDs: []int{1}, FieldID: 1000, Name:
"value_part", Transform: tc.transform,
+ })
+
+ expr, err := BuildPartitionMatchPredicate(spec, schema,
[]map[int]any{{1000: tc.partitionValue}})
+ require.NoError(t, err)
+
+ eval, err := iceberg.ExpressionEvaluator(schema, expr,
true)
+ require.NoError(t, err)
+
+ for _, value := range tc.matchingValues {
+ matched, err :=
eval(partitionPredicateRow{value})
+ require.NoError(t, err)
+ assert.True(t, matched, "source value %v should
match partition value %v", value, tc.partitionValue)
+ }
+ for _, value := range tc.nonMatchingVals {
+ matched, err :=
eval(partitionPredicateRow{value})
+ require.NoError(t, err)
+ assert.False(t, matched, "source value %v
should not match partition value %v", value, tc.partitionValue)
+ }
+ })
+ }
+}
+
+func TestBuildPartitionMatchPredicate_EvaluatesBucketCollision(t *testing.T) {
+ transform := iceberg.BucketTransform{NumBuckets: 4}
+ valuesByBucket := make(map[int32][]int32)
+ var collisionBucket int32
+ var collisionValues []int32
+
+ for value := range int32(10000) {
+ bucket := bucketValueForInt32(t, transform, value)
+ valuesByBucket[bucket] = append(valuesByBucket[bucket], value)
+ if len(valuesByBucket[bucket]) == 2 {
+ collisionBucket = bucket
+ collisionValues = valuesByBucket[bucket]
+
+ break
+ }
+ }
+ require.Len(t, collisionValues, 2)
+ assert.NotEqual(t, collisionValues[0], collisionValues[1])
+
+ nonMatchingValue := int32(-1)
+ for value := range int32(10000) {
+ if bucketValueForInt32(t, transform, value) != collisionBucket {
+ nonMatchingValue = value
+
+ break
+ }
+ }
+ require.NotEqual(t, int32(-1), nonMatchingValue)
+
+ schema := iceberg.NewSchema(0, iceberg.NestedField{
+ ID: 1, Name: "id", Type: iceberg.PrimitiveTypes.Int32,
+ })
+ spec := specWithFields(iceberg.PartitionField{
+ SourceIDs: []int{1}, FieldID: 1000, Name: "id_bucket",
Transform: transform,
+ })
+ expr, err := BuildPartitionMatchPredicate(spec, schema,
[]map[int]any{{1000: collisionBucket}})
+ require.NoError(t, err)
+
+ eval, err := iceberg.ExpressionEvaluator(schema, expr, true)
+ require.NoError(t, err)
+ for _, value := range collisionValues {
+ matched, err := eval(partitionPredicateRow{value})
+ require.NoError(t, err)
+ assert.True(t, matched, "source value %d should match bucket
%d", value, collisionBucket)
+ }
+ matched, err := eval(partitionPredicateRow{nonMatchingValue})
+ require.NoError(t, err)
+ assert.False(t, matched, "source value %d should not match bucket %d",
nonMatchingValue, collisionBucket)
+}
+
+func TestBuildPartitionMatchPredicate_EvaluatesTransformedNull(t *testing.T) {
+ cases := []struct {
+ name string
+ sourceType iceberg.Type
+ transform iceberg.Transform
+ nonNil any
+ nonNilMatches bool
+ }{
+ {name: "bucket", sourceType: iceberg.PrimitiveTypes.Int32,
transform: iceberg.BucketTransform{NumBuckets: 4}, nonNil: int32(1),
nonNilMatches: false},
+ {name: "truncate", sourceType: iceberg.PrimitiveTypes.String,
transform: iceberg.TruncateTransform{Width: 3}, nonNil: "books", nonNilMatches:
false},
+ {name: "year", sourceType: iceberg.PrimitiveTypes.Timestamp,
transform: iceberg.YearTransform{}, nonNil: timestampAtUTC(2020, 1, 1, 0, 0, 0,
0), nonNilMatches: false},
+ {name: "month", sourceType: iceberg.PrimitiveTypes.Timestamp,
transform: iceberg.MonthTransform{}, nonNil: timestampAtUTC(2020, 1, 1, 0, 0,
0, 0), nonNilMatches: false},
+ {name: "day", sourceType: iceberg.PrimitiveTypes.Timestamp,
transform: iceberg.DayTransform{}, nonNil: timestampAtUTC(2020, 1, 1, 0, 0, 0,
0), nonNilMatches: false},
+ {name: "hour", sourceType: iceberg.PrimitiveTypes.Timestamp,
transform: iceberg.HourTransform{}, nonNil: timestampAtUTC(2020, 1, 1, 0, 0, 0,
0), nonNilMatches: false},
+ {name: "void", sourceType: iceberg.PrimitiveTypes.String,
transform: iceberg.VoidTransform{}, nonNil: "books", nonNilMatches: true},
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ schema := iceberg.NewSchema(0, iceberg.NestedField{
+ ID: 1, Name: "value", Type: tc.sourceType,
+ })
+ spec := specWithFields(iceberg.PartitionField{
+ SourceIDs: []int{1}, FieldID: 1000, Name:
"value_part", Transform: tc.transform,
+ })
+
+ expr, err := BuildPartitionMatchPredicate(spec, schema,
[]map[int]any{{1000: nil}})
+ require.NoError(t, err)
+
+ eval, err := iceberg.ExpressionEvaluator(schema, expr,
true)
+ require.NoError(t, err)
+
+ matched, err := eval(partitionPredicateRow{nil})
+ require.NoError(t, err)
+ assert.True(t, matched, "null source value should match
null partition value")
+
+ matched, err = eval(partitionPredicateRow{tc.nonNil})
+ require.NoError(t, err)
+ assert.Equal(t, tc.nonNilMatches, matched, "unexpected
match for non-null source value")
+ })
+ }
+}
+
+func TestBuildPartitionMatchPredicate_VoidBindsAlwaysTrue(t *testing.T) {
+ spec := specWithFields(iceberg.PartitionField{
Review Comment:
**minor** — TestBuildPartitionMatchPredicate_VoidBindsAlwaysTrue is vacuous
The test builds a void-only spec, calls BindExpr, and asserts the result
equals AlwaysTrue{} — but BuildPartitionMatchPredicate already returns the
concrete value iceberg.AlwaysTrue{} before BindExpr runs, so BindExpr is a
no-op identity and the assertion cannot fail for any reason related to void
handling. The same applies to the {name: "void", ..., nonNilMatches: true} case
at :542, which only asserts that AlwaysTrue matches every row. Either rename to
reflect what is actually covered (a source-less void field contributes no
clause) or drop the BindExpr step.
##########
table/internal/partition_predicate.go:
##########
@@ -151,6 +196,110 @@ func BuildPartitionMatchPredicate(spec
iceberg.PartitionSpec, schema *iceberg.Sc
return result, nil
}
+func partitionTerm(transform iceberg.Transform, name string)
iceberg.UnboundTerm {
+ ref := iceberg.Reference(name)
+ if isIdentityTransform(transform) {
+ return ref
+ }
+ if isVoidTransform(transform) {
+ // Use the value form so binding the null predicate can fold it
to
+ // AlwaysTrue. Pointer forms are accepted by the Transform
interface too.
+ return iceberg.NewUnboundTransform(iceberg.VoidTransform{}, ref)
+ }
+
+ return iceberg.NewUnboundTransform(transform, ref)
+}
+
+func isIdentityTransform(transform iceberg.Transform) bool {
+ switch t := transform.(type) {
+ case iceberg.IdentityTransform:
+ return true
+ case *iceberg.IdentityTransform:
+ return t != nil
+ default:
+ return false
+ }
+}
+
+func isVoidTransform(transform iceberg.Transform) bool {
+ switch t := transform.(type) {
Review Comment:
**minor** — Pointer-form void is accepted here but not by the upstream
IsUnpartitioned guard the doc relies on
The doc comment at :48 leans on 'dynamic partition overwrite rejects
unpartitioned tables upstream', but PartitionSpec.IsUnpartitioned
(partitions.go:756) matches only the value form VoidTransform, while
isVoidTransform here deliberately accepts *VoidTransform too. A spec whose only
field is &VoidTransform{} therefore passes the upstream 'is partitioned' guard
and yields AlwaysTrue from this builder — a whole-table match. Not reachable
from parsed metadata (ParseTransform returns value forms only), so this is
hand-constructed-input territory; either narrow isVoidTransform to the value
form or widen IsUnpartitioned so the two agree.
##########
table/internal/partition_predicate.go:
##########
@@ -39,46 +39,69 @@ import (
// result is an OR across distinct partitions, each clause an AND across the
// spec's fields:
//
-// source == value when the partition value is present
-// IsNaN(source) when the value is a floating-point NaN (x == NaN is
never true)
-// IsNull(source) when the partition value is absent or nil
+// transform(source) == value when the partition value is present
+// IsNaN(transform(source)) when the value is a floating-point NaN (x ==
NaN is never true)
+// IsNull(transform(source)) when the partition value is absent or nil
//
Review Comment:
**minor** — Doc comment contradicts the new fail-closed behaviour for absent
partition keys
Line 44 still documents 'IsNull(transform(source)) when the partition value
is absent or nil', but :127-130 now returns ErrInvalidArgument when the field
id is absent from the tuple — that is the headline behaviour change of commit
28628f7 and the doc was not updated with it. Fix the comment to say absent keys
are rejected.
##########
table/evaluators_test.go:
##########
@@ -3100,6 +3100,21 @@ func TestManifestEvaluatorKeepsTransformedTerms(t
*testing.T) {
assert.True(t, keep)
}
+func TestPrepareBatchFilterRejectsTransformedTerms(t *testing.T) {
+ schema := iceberg.NewSchema(1,
Review Comment:
**nit** — TestPrepareBatchFilterRejectsTransformedTerms sits in the wrong
test file
prepareBatchFilter is defined in table/transaction.go:3356; its test was
added to table/evaluators_test.go. The test itself is good and correctly pins
the limitation the PR description depends on — it just belongs next to the code
it covers.
##########
table/internal/partition_predicate.go:
##########
@@ -151,6 +196,110 @@ func BuildPartitionMatchPredicate(spec
iceberg.PartitionSpec, schema *iceberg.Sc
return result, nil
}
+func partitionTerm(transform iceberg.Transform, name string)
iceberg.UnboundTerm {
+ ref := iceberg.Reference(name)
+ if isIdentityTransform(transform) {
+ return ref
+ }
+ if isVoidTransform(transform) {
+ // Use the value form so binding the null predicate can fold it
to
+ // AlwaysTrue. Pointer forms are accepted by the Transform
interface too.
+ return iceberg.NewUnboundTransform(iceberg.VoidTransform{}, ref)
+ }
+
+ return iceberg.NewUnboundTransform(transform, ref)
+}
+
+func isIdentityTransform(transform iceberg.Transform) bool {
+ switch t := transform.(type) {
+ case iceberg.IdentityTransform:
+ return true
+ case *iceberg.IdentityTransform:
+ return t != nil
+ default:
+ return false
+ }
+}
+
+func isVoidTransform(transform iceberg.Transform) bool {
+ switch t := transform.(type) {
+ case iceberg.VoidTransform:
+ return true
+ case *iceberg.VoidTransform:
+ return t != nil
+ default:
+ return false
+ }
+}
+
+func isTruncateTransform(transform iceberg.Transform) bool {
+ switch t := transform.(type) {
+ case iceberg.TruncateTransform:
+ return true
+ case *iceberg.TruncateTransform:
+ return t != nil
+ default:
+ return false
+ }
+}
+
+func validatePartitionValue(transform iceberg.Transform, resultType
iceberg.Type, lit iceberg.Literal) (iceberg.Literal, error) {
+ normalized, err := lit.To(resultType)
+ if err != nil {
+ return nil, fmt.Errorf("%w: partition value type %s cannot be
converted to transform result type %s: %v",
+ iceberg.ErrInvalidArgument, lit.Type(), resultType, err)
+ }
+
+ switch normalized.(type) {
+ case iceberg.AboveMaxLiteral, iceberg.BelowMinLiteral:
+ return nil, fmt.Errorf("%w: partition value %s is outside
transform result type %s",
+ iceberg.ErrInvalidArgument, normalized, resultType)
+ }
+
+ switch t := transform.(type) {
+ case iceberg.BucketTransform:
+ if err := validateBucketPartitionValue(t.NumBuckets,
normalized); err != nil {
+ return nil, err
+ }
+ case *iceberg.BucketTransform:
+ if t == nil {
+ return nil, fmt.Errorf("%w: bucket transform cannot be
nil", iceberg.ErrInvalidArgument)
+ }
+ if err := validateBucketPartitionValue(t.NumBuckets,
normalized); err != nil {
+ return nil, err
+ }
+ case iceberg.VoidTransform, *iceberg.VoidTransform:
+ return nil, fmt.Errorf("%w: void transform only accepts a nil
partition value", iceberg.ErrInvalidArgument)
Review Comment:
**minor** — Three more unreachable defensive branches in
validatePartitionValue / validateBucketPartitionValue
(a) :271-272 'void transform only accepts a nil partition value' cannot fire
— the caller at :150-153 already returns an error for every void field with a
non-nil value. (b) :265-267 '*BucketTransform cannot be nil' cannot fire —
UnboundTransform.Bind at :95 rejects typed-nil pointers via isNilTransform
first. (c) :291-294 'numBuckets in [1, MaxInt32]' cannot fire —
f.Transform.MarshalText() at :98 already calls
BucketTransform.validateNumBuckets, which rejects the same two conditions. Drop
these and rely on the single checks that already exist, so the reader is not
led to believe there are two independent validation layers.
##########
table/internal/partition_predicate.go:
##########
@@ -151,6 +196,110 @@ func BuildPartitionMatchPredicate(spec
iceberg.PartitionSpec, schema *iceberg.Sc
return result, nil
}
+func partitionTerm(transform iceberg.Transform, name string)
iceberg.UnboundTerm {
+ ref := iceberg.Reference(name)
+ if isIdentityTransform(transform) {
+ return ref
+ }
+ if isVoidTransform(transform) {
+ // Use the value form so binding the null predicate can fold it
to
+ // AlwaysTrue. Pointer forms are accepted by the Transform
interface too.
+ return iceberg.NewUnboundTransform(iceberg.VoidTransform{}, ref)
+ }
+
+ return iceberg.NewUnboundTransform(transform, ref)
+}
+
+func isIdentityTransform(transform iceberg.Transform) bool {
+ switch t := transform.(type) {
+ case iceberg.IdentityTransform:
+ return true
+ case *iceberg.IdentityTransform:
+ return t != nil
+ default:
+ return false
+ }
+}
+
+func isVoidTransform(transform iceberg.Transform) bool {
+ switch t := transform.(type) {
+ case iceberg.VoidTransform:
+ return true
+ case *iceberg.VoidTransform:
+ return t != nil
+ default:
+ return false
+ }
+}
+
+func isTruncateTransform(transform iceberg.Transform) bool {
+ switch t := transform.(type) {
+ case iceberg.TruncateTransform:
+ return true
+ case *iceberg.TruncateTransform:
+ return t != nil
+ default:
+ return false
+ }
+}
+
+func validatePartitionValue(transform iceberg.Transform, resultType
iceberg.Type, lit iceberg.Literal) (iceberg.Literal, error) {
+ normalized, err := lit.To(resultType)
+ if err != nil {
+ return nil, fmt.Errorf("%w: partition value type %s cannot be
converted to transform result type %s: %v",
+ iceberg.ErrInvalidArgument, lit.Type(), resultType, err)
+ }
+
+ switch normalized.(type) {
+ case iceberg.AboveMaxLiteral, iceberg.BelowMinLiteral:
+ return nil, fmt.Errorf("%w: partition value %s is outside
transform result type %s",
+ iceberg.ErrInvalidArgument, normalized, resultType)
+ }
+
+ switch t := transform.(type) {
+ case iceberg.BucketTransform:
+ if err := validateBucketPartitionValue(t.NumBuckets,
normalized); err != nil {
+ return nil, err
+ }
+ case *iceberg.BucketTransform:
+ if t == nil {
+ return nil, fmt.Errorf("%w: bucket transform cannot be
nil", iceberg.ErrInvalidArgument)
+ }
+ if err := validateBucketPartitionValue(t.NumBuckets,
normalized); err != nil {
+ return nil, err
+ }
+ case iceberg.VoidTransform, *iceberg.VoidTransform:
+ return nil, fmt.Errorf("%w: void transform only accepts a nil
partition value", iceberg.ErrInvalidArgument)
+ }
+
+ if (isIdentityTransform(transform) || isTruncateTransform(transform))
&& !isNaN(normalized.Any()) {
+ applied :=
transform.Apply(iceberg.Optional[iceberg.Literal]{Valid: true, Val: normalized})
Review Comment:
**nit** — PR body overstates the impossible-value validation
The description says the change will 'fail closed for missing or impossible
partition values', but the range/fixed-point validation at :259-281 covers only
bucket, truncate, identity and void. Year/month/day/hour partition values get
no plausibility check. This is harmless in practice (an impossible value yields
a predicate that matches nothing, i.e. under-delete rather than over-delete),
but the claim should be scoped to the transforms actually validated.
--
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]
