(iceberg-go) branch main updated: fix(table): safely match binary equality-delete partitions (#1528)

[ze...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git


The following commit(s) were added to refs/heads/main by this push:
     new ef139c7ef fix(table): safely match binary equality-delete partitions (#1528)
ef139c7ef is described below

commit ef139c7ef83866afd100521d71af975f9d3318b6
Author: Minh Vu <vu...@gmail.com>
AuthorDate: Mon Jul 27 22:29:07 2026 +0200

    fix(table): safely match binary equality-delete partitions (#1528)
    
    ## Problem
    
    Equality-delete planning compares data-file and delete-file partition
    maps before attaching a delete to a scan task. The maps store values as
    `any`, and the previous direct `!=` comparison panicked when a supported
    partition value was non-comparable in Go.
    
    Binary and fixed partition values are represented as `[]byte`, so a
    valid table could fail during scan planning with `comparing uncomparable
    type []uint8`.
    
    ## Fix
    
    Compare the maps with `maps.EqualFunc` and `reflect.DeepEqual`.
    
    This keeps the existing matching contract:
    
    - both maps must contain the same field IDs
    - values must have the same concrete type and value
    - independently allocated byte slices with the same bytes match
    - different binary values or type mismatches do not match
    
    It also remains safe if another non-comparable value reaches a custom
    `DataFile` implementation. Comparable primitives retain their previous
    equality semantics, including NaN values remaining unequal.
    
    ## Tests
    
    The tests cover the comparison helper and the real equality-delete
    matching path, including:
    
    - equal and different byte slices
    - binary versus string values
    - comparable multi-field partitions
    - different partition field IDs
    - selecting only the matching delete file without panicking
    
    Validation:
    
    - `go test -count=1 ./...`
    - `go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.8.0
    run --timeout=10m`
    
    Fixes #1349
    
    Signed-off-by: Minh Vu <vu...@gmail.com>
---
 table/scanner.go               | 14 ++------
 table/scanner_internal_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/table/scanner.go b/table/scanner.go
index 91c5e7466..28e5f4b4a 100644
--- a/table/scanner.go
+++ b/table/scanner.go
@@ -22,7 +22,9 @@ import (
 	"context"
 	"fmt"
 	"iter"
+	"maps"
 	"math"
+	"reflect"
 	"slices"
 	"strings"
 	"sync"
@@ -590,17 +592,7 @@ func matchEqualityDeletesToData(dataEntry iceberg.ManifestEntry, eqDeleteEntries
 }
 
 func partitionsMatch(a, b map[int]any) bool {
-	if len(a) != len(b) {
-		return false
-	}
-
-	for k, v := range a {
-		if bv, ok := b[k]; !ok || bv != v {
-			return false
-		}
-	}
-
-	return true
+	return maps.EqualFunc(a, b, reflect.DeepEqual)
 }
 
 // buildDVIndex indexes deletion vectors by the data file path they reference.
diff --git a/table/scanner_internal_test.go b/table/scanner_internal_test.go
index 62cee6dd5..0ca010b2e 100644
--- a/table/scanner_internal_test.go
+++ b/table/scanner_internal_test.go
@@ -53,6 +53,78 @@ func newDeleteManifest(minSeqNum int64) iceberg.ManifestFile {
 		Build()
 }
 
+func TestPartitionsMatchHandlesBinaryValues(t *testing.T) {
+	tests := []struct {
+		name  string
+		left  map[int]any
+		right map[int]any
+		match bool
+	}{
+		{
+			name:  "equal binary values",
+			left:  map[int]any{1000: []byte{0xde, 0xad}},
+			right: map[int]any{1000: append([]byte(nil), 0xde, 0xad)},
+			match: true,
+		},
+		{
+			name:  "different binary values",
+			left:  map[int]any{1000: []byte{0xde, 0xad}},
+			right: map[int]any{1000: []byte{0xbe, 0xef}},
+		},
+		{
+			name:  "binary and string values differ",
+			left:  map[int]any{1000: []byte("value")},
+			right: map[int]any{1000: "value"},
+		},
+		{
+			name:  "comparable values still match",
+			left:  map[int]any{1000: int32(7), 1001: "region"},
+			right: map[int]any{1000: int32(7), 1001: "region"},
+			match: true,
+		},
+		{
+			name:  "different field IDs",
+			left:  map[int]any{1000: []byte{1}},
+			right: map[int]any{1001: []byte{1}},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.NotPanics(t, func() {
+				assert.Equal(t, tt.match, partitionsMatch(tt.left, tt.right))
+			})
+		})
+	}
+}
+
+func TestMatchEqualityDeletesToDataHandlesBinaryPartitions(t *testing.T) {
+	dataSeqNum := int64(1)
+	deleteSeqNum := int64(2)
+	dataFile := &mockDataFile{
+		path:      "data.parquet",
+		partition: map[int]any{1000: []byte{0xde, 0xad}},
+	}
+	matchingDelete := &mockDataFile{
+		path:      "matching-delete.parquet",
+		partition: map[int]any{1000: append([]byte(nil), 0xde, 0xad)},
+	}
+	nonMatchingDelete := &mockDataFile{
+		path:      "non-matching-delete.parquet",
+		partition: map[int]any{1000: []byte{0xbe, 0xef}},
+	}
+
+	dataEntry := iceberg.NewManifestEntry(
+		iceberg.EntryStatusADDED, nil, &dataSeqNum, nil, dataFile)
+	deleteEntries := []iceberg.ManifestEntry{
+		iceberg.NewManifestEntry(iceberg.EntryStatusADDED, nil, &deleteSeqNum, nil, nonMatchingDelete),
+		iceberg.NewManifestEntry(iceberg.EntryStatusADDED, nil, &deleteSeqNum, nil, matchingDelete),
+	}
+
+	assert.Equal(t, []iceberg.DataFile{matchingDelete},
+		matchEqualityDeletesToData(dataEntry, deleteEntries))
+}
+
 func TestMinSequenceNum(t *testing.T) {
 	tests := []struct {
 		name      string