perf(table): deduplicate equality delete metadata setup by fallintoplace · Pull Request #2025 · apache/iceberg-go

fallintoplace · 2026-09-18T14:36:14Z

What

Deduplicate equality-delete metadata during scan setup.
Avoid repeating setup when tasks share delete files.

Why

Many scan tasks can reference the same delete file.
Re-reading its metadata adds setup time and allocations.

Implementation

Deduplicate file metadata by path and fast-path repeated file pointers.
Check schema history using the unique delete files.
Return ErrConflictingEqualityDeleteMetadata when one path has a different format or equality field ID set.
Keep the public getter fallback for custom DataFile implementations.

Benchmark

BenchmarkLazyEqualityDeleteMetadataSetup; 10,000 task references per case. Median of 7 runs, 1 second each, -cpu=1. Go 1.26.3, Apple M1 Pro (darwin/arm64). Base: 21e85b6; head: 68837f1.

Command: go test ./table -run '^$' -bench '^BenchmarkLazyEqualityDeleteMetadataSetup$' -benchmem -benchtime=1s -count=7 -cpu=1

Unique files	Before: time / bytes / allocs	After: time / bytes / allocs	Speedup
1	297.0 us / 80,480 B / 10,004	61.9 us / 504 B / 5	4.80x
100	363.3 us / 96,680 B / 10,112	243.0 us / 17,496 B / 212	1.49x
1,000	591.6 us / 285,144 B / 11,023	477.1 us / 213,160 B / 2,023	1.24x

laskoviymishka

Nice, the dedup is the right call, and the BenchmarkLazyEqualityDeleteMetadataSetup numbers make the case on their own. Collapsing the metadata setup to once-per-unique-path is a clean win.

I'd hold it before merging for couple things.

My concern is that the tests don't yet prove the fast paths preserve the old behavior. needsSchemaHistory() is the part doing the real work here, it's what lets us skip the second task walk while still loading schema history when a delete file references a dropped column, and its positive path has no test. If it regressed to always returning false, every existing test would stay green while delete files against dropped columns silently stopped resolving. That's the one I'd really want a guard on.

The new dedup test has a related gap: countingEqualityFieldDataFile embeds the DataFile interface, so it only ever exercises the EqualityFieldIDs() fallback. The DataFileCollectionsRef path that every real manifest-entry scan takes is never touched, and the "read once per path" assertion doesn't actually cover it.

A few things I'd want before merge:

a test that drives needsSchemaHistory() true (field ID absent from the current schema but present in a historical one), assigns the schemas, and loads the file successfully, plus the negative case
a dedup test double that actually implements DataFileCollectionsRef, so the borrow path is what gets counted
clone the slice returned from dataFileEqualityFieldIDs (or document the retain-with-DataFile contract); the public getter already clones, and this one gets stored for the whole scan

Nothing structural, the shape is right. Once those land I'm happy to take another pass and approve.

laskoviymishka · 2026-09-21T15:26:23Z

 	require.ErrorContains(t, err, "empty-equality-fields.parquet")
 }

+func TestEqualityDeleteMetadataIsReadOncePerPath(t *testing.T) {


needsSchemaHistory() is the part of this change doing the real work, and I don't think anything exercises its positive path. The pre-existing TestLazyEqualityDeleteLoaderRejectsFieldAbsentFromSchemaHistory passes tableSchemas in directly, so it skips the method entirely.

The failure mode that worries me: if needsSchemaHistory() regressed to always returning false, every test here would still pass, but in production a delete file referencing a dropped column would never load its historical schema and would silently stop resolving.

Could we add a test that builds a loader with a field ID absent from the current schema but present in a historical one, asserts needsSchemaHistory() returns true, assigns the schemas, and loads the file cleanly, plus the negative case where all IDs are present? That's the regression guard I'd want on this before merge. wdyt?

laskoviymishka · 2026-09-21T15:26:23Z

 	return internal.BorrowedDataFileCollections(file)
 }

+func dataFileEqualityFieldIDs(file iceberg.DataFile) []int {


This returns the concrete file's internal equality-IDs slice directly, but the public EqualityFieldIDs() getter clones (slices.Clone), and the Borrowed* contract on the siblings says these values must not escape the current operation. Here the returned slice does escape: callers store it in lazyEqualityDeleteFile.fieldIDs and deleteFileInfo.fieldIDs for the whole scan.

It's safe today only because the lazyEqualityDeleteFile keeps the *dataFile alive, so the backing array can't be recycled. But the moment someone adds pooling or a Reset() that reuses that backing array, every retained fieldIDs across in-flight scans would alias recycled memory and delete-key comparisons would go wrong, with nothing at the call site hinting the slice was borrowed.

I'd slices.Clone on the fast-path return here. It's one small alloc per unique file, amortized by the dedup you just added. Failing that, a doc comment spelling out the retain-with-DataFile contract (the siblings all have one, this function has none). wdyt?

laskoviymishka · 2026-09-21T15:26:24Z

+	loader, err := newLazyEqualityDeleteLoader(iceio.NewMemFS(), schema, nil, nil, tasks)
+	require.NoError(t, err)
+	assert.Len(t, loader.files, 1)
+	assert.Equal(t, 1, deleteFile.equalityFieldIDsCalls)


This assertion only proves the fallback path. countingEqualityFieldDataFile embeds iceberg.DataFile (the interface), and Go won't promote DataFileCollectionsRef through an embedded interface, so *countingEqualityFieldDataFile never satisfies it and dataFileEqualityFieldIDs always takes the EqualityFieldIDs() branch here.

For a real manifest-entry data file it's the other way around: DataFileCollectionsRef is satisfied, the borrow path is taken, and EqualityFieldIDs() is never called, so this counter would read 0. So the "read once per path" claim is verified only for the path production never uses, and a bug in the DataFileCollectionsRef branch (wrong IDs, or the borrow called more than once per path) wouldn't be caught.

Could we use a double that actually implements DataFileCollectionsRef (or a concrete manifest-entry-sourced file) and count borrow invocations instead? That's what exercises the hot path. wdyt?

laskoviymishka · 2026-09-21T15:26:24Z

+
+	for _, file := range l.files {
+		for _, fieldID := range file.fieldIDs {
+			if _, found := l.tableSchema.FindColumnName(fieldID); !found {


Small one: the old check used FindFieldByID, and I'd keep it here. It's equivalent today since both return false for an absent ID, but FindFieldByID is the "does a field with this ID exist?" question this is actually asking, and it matches schemaForEqualityFields and the rest of the file.

There's also a sharper edge: FindColumnName goes through IndexNameByID, which panics on duplicate field names, where FindFieldByID's IndexByID just overwrites. So a hand-crafted or corrupted schema with dup names that used to degrade gracefully (treat IDs as absent, load history) would now crash the scan. Switching back to FindFieldByID sidesteps that. wdyt?

laskoviymishka · 2026-09-21T15:26:24Z

+				return nil, fmt.Errorf("%w: equality delete file %s", ErrEmptyEqualityFieldIDs, path)
 			}

 			hasAny = true


After the reorder, hasAny is set only once a new entry is about to be inserted, so it's now exactly equivalent to len(uniqueDeletes) > 0. It reads as extra state to trace across three continue paths, and if someone later slips a continue between hasAny = true and the insert the invariant breaks silently.

I'd drop hasAny and gate the return on len(uniqueDeletes) == 0 instead.

laskoviymishka · 2026-09-21T15:26:24Z

+				}
+
+				loader.files = make(map[string]*lazyEqualityDeleteFile, 2)
+				firstFile.id = 0


firstFile is built with id already at its zero value, so this firstFile.id = 0 (and the matching one at line 421) is a no-op. It makes a reader stop to check whether id could have been something else. I'd drop both; if the "first file is always id 0" invariant is worth stating, a one-line comment says it more clearly than the self-assignment.

zeroshade

REQUEST_CHANGES on the dedup identity; detail inline.

Commit 4f20e7f741 addresses all six earlier threads: the borrowed slice is cloned, the DataFileCollectionsRef fast path is exercised, both schema-history directions are covered, FindFieldByID is restored, and the redundant state and assignments are gone. I found no cache-lifetime, concurrency, or Arrow-release problem — the shared-loader test passes under -race and the checked-allocator reader tests pass.

zeroshade · 2026-09-21T19:13:19Z

+					}
+
+					continue
+				}


The dedup identity is the file path alone, and the skip happens before this entry's equality IDs are ever read:

if path == firstPath { continue }

Two equality-delete entries at the same path carrying different ordered equality field IDs — say [1] and [2] — collapse to the first. dataFileEqualityFieldIDs never observes field 2, so needsSchemaHistory and addFieldIDs omit it, and a task referencing the second entry resolves through the same path-keyed map and applies the first entry's delete set. That is a wrong result, not a missed optimization. A focused regression expecting field 2 to trigger schema-history loading fails at this head.

Key the map on an end-to-end metadata identity — the path plus the ordered equality field IDs, plus anything else with read semantics such as format — or explicitly reject same-path entries whose metadata conflicts.

zeroshade · 2026-09-21T19:13:19Z

@@ -514,30 +556,32 @@ func readAllEqualityDeleteFiles(ctx context.Context, fs iceio.IO, schema *iceber
 	}

 	uniqueDeletes := make(map[string]deleteFileInfo)


Whatever identity you settle on above needs to carry through here and through the per-task lookup, since both resolve entries by path today. Worth one regression with the same path and differing ordered equality IDs that covers the lazy and eager paths together.

laskoviymishka

the three from last round are all in and tested now: the borrowed-slice clone, the DataFileCollectionsRef double, and both needsSchemaHistory() directions. the file-format conflict subtest is a nice add too, that's a real edge the old test missed. CI's green as well.

the accept path is the one gap I'd still want closed. we've got two reject subtests now but nothing that builds two distinct DataFile objects at the same path with matching format and IDs and asserts the dedup actually succeeds. since it's table-driven now it's basically a wantErr flag plus one row, and it's the branch @zeroshade's identity thread is really about. worth noting IsReadOncePerPath reuses a single pointer, so today it's asserting read-once-per-pointer, not per-path.

on @zeroshade's dedup-identity thread: agreed that's the thing to settle, and I added one point there. slices.Equal is order-sensitive, so [1,2] vs [2,1] for the same physical file is a hard conflict, which is stricter than the spec (set predicate) and than Java/PyIceberg, both of which tolerate permutations. root cause is our own key encoding; the clean fix is to canonicalize field-ID order in the encoding and comparison, or at minimum document why we reject. left the detail inline, along with the small sentinel-error and nil-guard bits.

this has come a long way since the first pass. get the accept row in and settle the identity/order question on @zeroshade's thread and I think we're there.

laskoviymishka · 2026-09-30T21:41:14Z

+	existingFile iceberg.DataFile,
+	existingFieldIDs []int,
+) error {
+	if dataFile != nil && reflect.TypeOf(dataFile).Comparable() && dataFile == existingFile {


The dataFile != nil check only guards the reflect.TypeOf call in this fast path. If a nil interface ever reached here it short-circuits past the fast path and then panics two lines down at dataFileEqualityFieldIDsRef(dataFile) and dataFile.FilePath(), so the guard reads like it protects the whole function when it only covers the if. Callers all check ContentType first so it can't happen today, but I'd either drop the != nil sub-expression or add a real top-of-function nil check so the contract is honest.

While we're here: this identity fast path only fires when two tasks share the exact same pointer, which is a test-only shape. Real manifest entries are distinct allocations, so in production this always falls through to the structural compare. Not wrong, but a one-line comment (or scoping the check to Kind() == reflect.Ptr) would save the next reader the double-take.

laskoviymishka · 2026-09-30T21:41:14Z

+	if len(fieldIDs) == 0 {
+		return fmt.Errorf("%w: equality delete file %s", ErrEmptyEqualityFieldIDs, dataFile.FilePath())
+	}
+	if dataFile.FileFormat() == existingFile.FileFormat() && slices.Equal(fieldIDs, existingFieldIDs) {


REQUEST_CHANGES on the dedup identity

agreed this is the piece to settle. the one thing I'd add: slices.Equal(fieldIDs, existingFieldIDs) is order-sensitive, so the same physical path with equality_ids = [1,2] vs [2,1] is a hard conflict, and the new subtest now locks that in as intended. that's stricter than the spec and both other clients. equality-delete matching is a set predicate (a row matches if all equality fields match), Java's DeleteFileIndex does no path dedup or cross-entry check (first-wins), and PyIceberg dedups via set(), so a table they read fine would error here.

root cause is our own key encoding: readEqualityDeleteFile builds composite keys in fieldIDs slice order, so the strictness is an artifact of the encoding, not the spec. the clean fix is to canonicalize (sort ascending by field ID) when we build the key set and when we compare, so order stops mattering and we're spec-correct; at minimum a comment on why we reject permutations. the intent (avoid silent under-deletion) is right and the practical risk is low, so I don't think this blocks on its own. since it's your thread, your call on whether to canonicalize now or just document.

laskoviymishka · 2026-09-30T21:41:14Z

+		return nil
+	}
+
+	return fmt.Errorf(


ErrEmptyEqualityFieldIDs and ErrAmbiguousEqualityColumn are %w-wrapped sentinels, but this conflict is a bare fmt.Errorf, so the test has to ErrorContains on a substring and scan-layer callers can't errors.Is it apart from an I/O or schema failure. I'd give it a sentinel and wrap it:

var ErrConflictingEqualityDeleteMetadata = errors.New("conflicting equality delete metadata")

then %w it here and switch the test to require.ErrorIs.

laskoviymishka · 2026-09-30T21:41:14Z

+
+	tests := []struct {
+		name   string
+		second iceberg.DataFile


Now that this is table-driven, I'd add a wantErr flag and one accept row: two distinct DataFile objects at the same path, matching format and field IDs, asserting the dedup succeeds.

{ name: "accept: distinct objects, matching format and IDs", second: newEqualityDeleteSetAssemblyTestFile(t, path, []int{1, 2}), wantErr: false, },

Right now the accept branch (FileFormat match plus slices.Equal returning nil) has zero coverage: every test either reuses one pointer so the identity shortcut fires, or expects the reject error. A future change that inverts the condition or flips the slices.Equal args wouldn't be caught. Related: TestEqualityDeleteMetadataIsReadOncePerPath reuses a single pointer, so it's really asserting read-once-per-pointer, not per-path. The accept row is what covers the per-path structural case, and it's exactly the branch @zeroshade's thread is asking about.

Cover successful same-path deduplication for distinct files in lazy and eager loaders. Restrict the identity shortcut to pointers to avoid panics from comparable wrappers containing non-comparable values. Expose metadata conflicts through an errors.Is-compatible sentinel and document the conservative ordered-ID contract.

zeroshade

My identity concern (r4065519322 / r4065519330) is addressed: conflicting same-path metadata is now rejected before any I/O in both loaders, with a zero-I/O regression test. I will resolve those threads. The round-2 items from @laskoviymishka (accept row, sentinel error, nil guard) are in.

Before merge:

reflect.Ptr at equality_delete_reader.go:318 fails govet in all four lint-and-test jobs. Lint runs first, so make test-race never ran on c24c10c (it passes locally).
Compare same-path IDs as sets (ruling on r4149615004, inline) and drop the duplicate conflict test.
PR body: the "after" numbers predate the clone added during review (head is 5 / 212 / 2,023 allocs for 1 / 100 / 1,000 files, +1 alloc per unique file), and the 4.48x / 48% / 46% timings did not reproduce for me; only the one-file case was consistently faster. Please refresh them, and mention ErrConflictingEqualityDeleteMetadata in the title or body, since scans can now fail on conflicting duplicates.

Follow-up, not this PR: scanner.go:1840 guards memo[file] with Comparable(), which has the same panic your comment at equality_delete_reader.go:316-317 describes. I will approve after a green run that includes test-race.

zeroshade · 2026-10-01T17:44:59Z

+	// Callers have already inspected ContentType, so both files must be non-nil.
+	// Only skip validation for the same immutable file pointer: a comparable
+	// struct may still contain an interface holding a non-comparable value.
+	if reflect.TypeOf(dataFile).Kind() == reflect.Ptr && dataFile == existingFile {


Blocker: reflect.Ptr fails the govet inline check (Constant reflect.Ptr should be inlined). That is the only failure in all four red lint-and-test jobs, and because lint runs before make test-race, CI ran no tests on this commit. go test -race ./table passes locally. Use reflect.Pointer.

Keep the same-pointer shortcut itself. @laskoviymishka, r4149614992 calls this a test-only shape, but real scans take it constantly: planning hands every matching task the same *dataFile for a delete entry (appendEqualityDeletesAfter appends entry.entry.DataFile(), and dataFilesWithoutColumnStats memoizes one projected copy per original). This is the common path, not the fallback.

zeroshade · 2026-10-01T17:44:59Z

+	// This intentionally rejects reordered IDs, although equality matching itself
+	// is order-independent. Keep this dedup change aligned with the existing
+	// order-sensitive key encoding/grouping; canonicalization is a separate change.
+	if dataFile.FileFormat() == existingFile.FileFormat() && slices.Equal(fieldIDs, existingFieldIDs) {


@laskoviymishka, this is my ruling on r4149615004: compare same-path IDs as sets, not with slices.Equal.

Each path gets one file set, and the fieldIDs of that set drive both the delete-file read (loadFile, :500 and :508) and row matching (processEqualityDeletesColumnarForFile, :1294). Tasks resolve the set by path (:543), so nothing after construction reads the order of a later entry. The same path with [1,2] and then [2,1] therefore scans correctly on main today, and this check turns it into ErrConflictingEqualityDeleteMetadata. Java groups equality deletes by Sets.newHashSet(delete.equalityFieldIds()) for the same reason.

Fix: require the same length and that every ID in each slice appears in the other (a nested loop over a few IDs, no allocation). [1] vs [2] must still be rejected. Update the comment above, and flip the "reordered IDs" row in TestEqualityDeleteMetadataConflictErrors (equality_delete_metadata_validation_test.go:143) to expect success; the other reordered row goes away with the duplicate test. Normalizing the key encoding across different paths stays out of scope.

zeroshade · 2026-10-01T17:44:59Z

+func TestEqualityDeleteMetadataRejectsConflictingSamePath(t *testing.T) {
+	t.Parallel()
+
+	schema := iceberg.NewSchema(0,
+		iceberg.NestedField{ID: 1, Name: "id", Type: iceberg.PrimitiveTypes.Int64, Required: true},
+		iceberg.NestedField{ID: 2, Name: "data", Type: iceberg.PrimitiveTypes.Int64, Required: true},
+	)
+	path := "mem://metadata-conflict/delete.parquet"
+	first := newEqualityDeleteSetAssemblyTestFile(t, path, []int{1, 2})
+
+	avroBuilder, err := iceberg.NewDataFileBuilder(
+		*iceberg.UnpartitionedSpec,
+		iceberg.EntryContentEqDeletes,
+		path,
+		iceberg.AvroFile,
+		nil,
+		nil,
+		nil,
+		1,
+		128,
+	)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name   string
+		second iceberg.DataFile
+	}{
+		{
+			name:   "equality field IDs",
+			second: newEqualityDeleteSetAssemblyTestFile(t, path, []int{2, 1}),
+		},
+		{
+			name:   "file format",
+			second: avroBuilder.EqualityFieldIDs([]int{1, 2}).Build(),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tasks := []FileScanTask{
+				{EqualityDeleteFiles: []iceberg.DataFile{first}},
+				{EqualityDeleteFiles: []iceberg.DataFile{tt.second}},
+			}
+
+			_, err := newLazyEqualityDeleteLoader(iceio.NewMemFS(), schema, nil, nil, tasks)
+			require.ErrorContains(t, err, "conflicting equality delete metadata")
+			require.ErrorContains(t, err, path)
+
+			_, err = readAllEqualityDeleteFiles(t.Context(), iceio.NewMemFS(), schema, nil, tasks, 1)
+			require.ErrorContains(t, err, "conflicting equality delete metadata")
+			require.ErrorContains(t, err, path)
+		})
+	}
+}


This is fully covered by TestEqualityDeleteMetadataConflictErrors (equality_delete_metadata_validation_test.go:127-167): the same reordered-IDs and Avro rows, plus ErrorIs, the different- and empty-ID rows, and a zero-I/O assertion. This one still matches on the error string, which r4149615008 asked to replace. Please delete it. Also consider moving the new 167-line file into this one, where the rest of the equality-delete loader tests live.

zeroshade · 2026-10-01T17:44:59Z

+func TestEqualityDeleteMetadataIsReadOncePerPath(t *testing.T) {
+	t.Parallel()
+
+	schema := iceberg.NewSchema(0,
+		iceberg.NestedField{ID: 1, Name: "id", Type: iceberg.PrimitiveTypes.Int64, Required: true},
+	)
+
+	base := newEqualityDeleteSetAssemblyTestFile(t, "mem://metadata-dedup/delete.parquet", []int{1})
+	deleteFile := &countingEqualityFieldDataFile{DataFile: base}
+	tasks := make([]FileScanTask, 100)
+	for i := range tasks {
+		tasks[i] = FileScanTask{EqualityDeleteFiles: []iceberg.DataFile{deleteFile}}
+	}
+
+	loader, err := newLazyEqualityDeleteLoader(iceio.NewMemFS(), schema, nil, nil, tasks)
+	require.NoError(t, err)
+	assert.Len(t, loader.files, 1)
+	assert.Equal(t, 1, deleteFile.borrowedEqualityFieldIDsCalls)
+	assert.Zero(t, deleteFile.equalityFieldIDsCalls)
+
+	fs := iceio.NewMemFS()
+	path := "mem://metadata-dedup/eager-delete.parquet"
+	writeEqualityDeleteParquetToMemFS(t, fs, path, `[{"id": 1}]`)
+	base = newEqualityDeleteSetAssemblyTestFile(t, path, []int{1})
+	deleteFile = &countingEqualityFieldDataFile{DataFile: base}
+	tasks = make([]FileScanTask, 100)
+	for i := range tasks {
+		tasks[i] = FileScanTask{EqualityDeleteFiles: []iceberg.DataFile{deleteFile}}
+	}
+
+	perTask, err := readAllEqualityDeleteFiles(t.Context(), fs, schema, nil, tasks, 1)
+	require.NoError(t, err)
+	assert.Len(t, perTask, len(tasks))
+	assert.Equal(t, 1, deleteFile.borrowedEqualityFieldIDsCalls)
+	assert.Zero(t, deleteFile.equalityFieldIDsCalls)
+}


Nit: both loaders here see one shared pointer, so after the first reference this counts the same-pointer shortcut in validateEqualityDeleteMetadata, not dedup by path. It will break on harmless changes to that shortcut. TestEqualityDeleteMetadataAcceptsDistinctSamePathFiles already covers the behavior through I/O counts, and its "borrowed metadata" rows exercise the DataFileCollectionsRef path with distinct objects. @laskoviymishka, this is the double you asked for in r4063623041; I am fine with dropping or keeping it.

zeroshade · 2026-10-01T19:02:22Z

there's a linting issue that needs fixing

zeroshade

Everything from my last review is fixed at 68837f1, and the perf commits added since keep the existing behavior. LGTM from my side. Two optional nits are inline.

I'll wait for @laskoviymishka to re-review and merge, since his change request is still open.

This review was drafted by an AI-assisted tool and
confirmed by an Apache Iceberg maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Iceberg handles maintainer review:
CONTRIBUTING.md.

zeroshade · 2026-10-02T17:32:00Z

 		}

-		return []*equalityDeleteSet{fileSet.equalityDeleteSet}, nil
+		return file.singleSet[:], nil


Nit: every task whose only equality delete is this file now shares the slice load returns. That's safe for today's only consumer, which just ranges over it, but nothing on load says the slice is shared. Either document that callers must not modify it, or drop singleSet. It saves one 8-byte allocation per data file.

zeroshade · 2026-10-02T17:32:00Z


 	for _, t := range tasks {
 		for _, d := range t.EqualityDeleteFiles {
+			if hasFirst && uniqueDeletes == nil &&


Nit: readAllEqualityDeleteFiles is only called from tests and benchmarks, so this copied first-path fast path only speeds up tests and is a second copy of the lazy loader's logic. I'd revert the eager side to the plain map loop, keeping the cached hasPointerIdentity.

fallintoplace requested a review from zeroshade as a code owner September 18, 2026 14:36

laskoviymishka requested changes Sep 21, 2026

View reviewed changes

zeroshade requested changes Sep 21, 2026

View reviewed changes

fallintoplace added 7 commits September 23, 2026 21:59

perf(table): deduplicate equality delete metadata setup

239ff92

perf(table): streamline equality delete setup

791b661

perf(table): reduce equality delete setup branching

77c2a93

style(table): fix gofumpt benchmark formatting

40acb8d

fix(table): preserve equality delete metadata safety

797776a

fix(table): reject conflicting equality delete metadata

b811c42

style(table): clarify equality delete metadata helpers

de2123c

fallintoplace force-pushed the perf/deduplicate-equality-delete-metadata branch from 24eb462 to de2123c Compare September 23, 2026 20:00

test(table): cover equality delete format conflicts

e72afab

laskoviymishka requested changes Sep 30, 2026

View reviewed changes

zeroshade requested changes Oct 1, 2026

View reviewed changes

fallintoplace added 14 commits October 1, 2026 20:03

fix(table): compare equality delete field IDs as sets

4c5adea

test(table): accept reordered equality delete IDs

5bdf2f1

test(table): drop duplicate equality metadata conflict coverage

397248a

style(table): satisfy equality metadata lint

a5166b8

style(table): satisfy metadata validation lint

1a7b914

perf(table): cache equality delete pointer identity

f88d835

perf(table): skip repeated equality delete identity work

50d45ae

style(table): format pointer identity cache

2fd4b79

perf(table): fast-path repeated eager equality deletes

becdbf8

perf(table): skip repeated delete type checks

b594be7

perf(table): bypass path lookup for single equality delete

5d3b243

perf(table): reuse single equality delete result slice

efde05d

perf(table): group resolved equality deletes directly

775c0d9

perf(table): skip unused equality schema resolution

0b0d75a

style(table): fix equality delete reader lint

68837f1

zeroshade approved these changes Oct 2, 2026

View reviewed changes

perf(table): simplify equality delete fast paths

beca7bb

		@@ -514,30 +556,32 @@ func readAllEqualityDeleteFiles(ctx context.Context, fs iceio.IO, schema *iceber
		}

		uniqueDeletes := make(map[string]deleteFileInfo)

Conversation

fallintoplace commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Implementation

Benchmark

Uh oh!

laskoviymishka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeroshade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laskoviymishka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeroshade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeroshade commented Oct 1, 2026

Uh oh!

zeroshade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fallintoplace commented Sep 18, 2026 •

edited

Loading