perf(table): cache equality delete field groups

[fallintoplace]

What changed

• Cache the equality field-group key once per delete file.
• Avoid allocating the per-task grouping map when all delete files use one field group.
• Keep the existing grouping path for tasks with multiple field groups.
• Reuse the existing assembly coverage and benchmark setup.

Why

Most tasks have one equality field group. The old path formatted the same fieldIDs slice and allocated a grouping map for every task, even when there was nothing to split.

Benchmark

Run on an Apple M1 Pro with:

go test ./table -run '^$' -bench '^BenchmarkEqualityDeleteSetAssembly/(shared-single-file|shared-four-file-union)/shared$' -benchmem -benchtime=2s -count=5

These are medians over five runs. Before is upstream/main and after is this change.

Case	Before	After
1 shared delete file, 1 group	295.826 µs/op, 216,383 B/op, 5,022 allocs/op	109.775 µs/op, 176,328 B/op, 2,022 allocs/op
4 shared delete files, 1 group	909.692 µs/op, 525,542 B/op, 17,045 allocs/op	324.757 µs/op, 365,408 B/op, 5,045 allocs/op

Testing

• go test ./table -count=1
• go test ./table -race -count=1
• go vet ./table

[laskoviymishka]

Nice change!

Almost there. The one thing I'd want before merge is test coverage for the new branches. The rewrite introduces a couple of subtle paths that nothing currently exercises: multiple files accumulating into groupFiles before the map is promoted, and a file arriving with the original key after the transition (the [A(g1), B(g2), C(g1)] layout, where C gets appended back onto the migrated key). Both are correct as written, but equality-delete mis-grouping is a silent data-correctness bug where rows that should be deleted just come back, so I'd rather have these pinned by a test before we merge than discover a future refactor quietly broke them.

Two smaller things I left inline, neither blocking. The double if groups == nil reads as two independent guards when it's really one sequentially-coupled decision; an if/else if/else makes the three per-iteration paths obvious and lets us sever the groupFiles alias while we're at it. And since newEqualityDeleteFileSet is now the single place the group key gets computed, it's a good spot to note (or fix) that fmt.Sprint(fieldIDs) is order-sensitive where Java keys on a Set<Integer>.

Once the transition-path tests are in, happy to take another pass and approve.

[laskoviymishka]

Caching the key here so it's computed once per unique delete file is the right call.

One thing worth flagging while this is the single place the key gets built: fmt.Sprint(fieldIDs) is order-sensitive, so [1 2] and [2 1] land in separate groups, whereas Java keys on a Set<Integer> (via TypeUtil.selectInIdOrder) and treats any permutation of the same IDs as one group. Two delete files covering the same equality columns but storing equality_ids in different orders would merge under Java and split here. It's pre-existing, not something this PR introduces, but since the key is now canonicalized to one spot, sorting fieldIDs ascending before the Sprint (and before colNames) would close the gap cheaply and match Java. At minimum a comment that the format has to stay collision-free for []int would help. wdyt?

[laskoviymishka]

These two consecutive if groups == nil blocks read like independent guards, but they aren't. The first can flip groups from nil to a live map, and the second observes that flip to decide where the current file goes. The transition branch deliberately doesn't append dk itself; it relies on fall-through to the second check to do it, which is the hardest part to see, and a stray continue or a hoisted block would silently break the invariant that each file is added exactly once.

An if/else if/else makes the three per-iteration paths explicit, and lets us null out groupFiles right after it's handed to the map so nothing can later append into the shared backing array:

if groups != nil {
    groups[dk.groupKey] = append(groups[dk.groupKey], dk)
} else if len(groupFiles) == 0 {
    groupKey = dk.groupKey
    groupFiles = append(groupFiles, dk)
} else if dk.groupKey != groupKey {
    groups = make(map[string][]*equalityDeleteFileSet, 2)
    groups[groupKey] = groupFiles
    groupFiles = nil
    groups[dk.groupKey] = append(groups[dk.groupKey], dk)
} else {
    groupFiles = append(groupFiles, dk)
}

wdyt?

[laskoviymishka]

This path, a file with the original key arriving after the map's been promoted, has no test behind it. Neither existing test builds a task where groupFiles holds more than one file at the moment of promotion, or where a later file lands back on the already-migrated key.

I'd add two cases to TestBuildEqualityDeleteSetsPerTaskKeepsFieldGroupsSeparate: [A=[1], B=[1], C=[2]] (two group-1 files accumulate, then groups["[1]"] = groupFiles assigns a 2-element slice), and [A=[1], B=[2], C=[1]] (promotion fires on B, then C appends back through this line into the migrated [1] group). Assert the [1] group ends up with both A and C in the second case.

That second layout is the one I care about most. It's the only path where the migrated key takes a further append, and it's exactly what a future edit to the promotion block could silently break.

[laskoviymishka]

This new early-continue (a task whose delete files are all absent from perFile) isn't hit by either existing test, since both always match. A one-line case with a task whose delete paths aren't keys in perFile, asserting perTask has no entry for that index, would lock it in. Fine as a follow-up.