PDGGK opened a new issue, #2407:
URL: https://github.com/apache/iceberg-go/issues/2407
### Apache Iceberg version
main (development)
### Please describe the bug 🐞
When the smallest value of a string partition field in a manifest is the
empty string, the manifest list stores that field's `lower_bound` as null, next
to `contains_null = false`. A null lower bound means every value of the field
is null (spec, manifest list field 510). So the manifest evaluator skips the
manifest for any predicate on that field, and a filtered scan silently returns
too few rows. It loses every row in the manifest, not only the rows with `""`.
Reproduced on main (c37ed1a) and on v0.7.0:
```go
func TestEmptyStringPartitionBound(t *testing.T) {
ctx := context.Background()
cat, err := hadoop.NewCatalog(ctx, "local", t.TempDir(), nil)
require.NoError(t, err)
require.NoError(t, cat.CreateNamespace(ctx, catalog.ToIdentifier("db"),
nil))
sc := iceberg.NewSchema(0,
iceberg.NestedField{ID: 1, Name: "id", Type:
iceberg.PrimitiveTypes.Int64, Required: true},
iceberg.NestedField{ID: 2, Name: "region", Type:
iceberg.PrimitiveTypes.String},
)
spec, err := iceberg.NewPartitionSpecOpts(
iceberg.AddPartitionFieldByName("region", "region",
iceberg.IdentityTransform{}, sc, nil))
require.NoError(t, err)
tbl, err := cat.CreateTable(ctx, catalog.ToIdentifier("db", "events"),
sc, catalog.WithPartitionSpec(&spec))
require.NoError(t, err)
arrSc, err := table.SchemaToArrowSchema(sc, nil, false, false)
require.NoError(t, err)
b := array.NewRecordBuilder(memory.DefaultAllocator, arrSc)
defer b.Release()
b.Field(0).(*array.Int64Builder).AppendValues([]int64{1, 2}, nil)
b.Field(1).(*array.StringBuilder).AppendValues([]string{"", "us"}, nil)
rec := b.NewRecordBatch()
defer rec.Release()
rdr, err := array.NewRecordReader(arrSc, []arrow.RecordBatch{rec})
require.NoError(t, err)
defer rdr.Release()
tbl, err = tbl.Append(ctx, rdr, nil)
require.NoError(t, err)
fs, err := tbl.FS(ctx)
require.NoError(t, err)
manifests, err := tbl.CurrentSnapshot().Manifests(fs)
require.NoError(t, err)
s := manifests[0].Partitions()[0]
t.Logf("contains_null=%v lower_bound is nil=%v upper_bound=%q",
s.ContainsNull, s.LowerBound == nil, *s.UpperBound)
for _, v := range []string{"us", ""} {
out, err :=
tbl.Scan(table.WithRowFilter(iceberg.EqualTo(iceberg.Reference("region"),
v))).ToArrowTable(ctx)
require.NoError(t, err)
t.Logf("region = %q: %d rows", v, out.NumRows())
out.Release()
}
full, err := tbl.Scan().ToArrowTable(ctx)
require.NoError(t, err)
t.Logf("no filter: %d rows", full.NumRows())
full.Release()
}
```
Output:
```
repro_test.go:52: contains_null=false lower_bound is nil=true
upper_bound="us"
repro_test.go:57: region = "us": 0 rows
repro_test.go:57: region = "": 0 rows
repro_test.go:62: no filter: 2 rows
```
Expected: `lower_bound` is the empty string (zero-length bytes), and each
filter returns its 1 row.
Cause: `StringLiteral.MarshalBinary` returns a nil slice for `""`.
`partitionFieldStats.toSummary` stores a pointer to it, and twmb/avro writes a
pointer to a nil slice as the null branch of the `["null","bytes"]` union.
#1309 hit the same encoder behaviour for the `partitions` list.
Engines that follow the spec read the bound the same way. Java's
`ManifestEvaluator` returns `ROWS_CANNOT_MATCH` when `lowerBound()` is null,
and so does PyIceberg's `_ManifestEvalVisitor`. So they would skip these
manifests in tables written by iceberg-go too.
I have a fix and a regression test ready and would like to work on this.
--
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]
