codeAnqiang-ma opened a new pull request, #1816:
URL: https://github.com/apache/iceberg-go/pull/1816
Fixes #1815
`accumulateSummaryDelta` resolves partition specs two different ways. Added
files use `sp.spec(int(df.SpecID()))`, a by-ID lookup over `GetSpecByID`;
removed data files, delete files and deletion vectors index the slice directly,
`specs[df.SpecID()]`.
Spec ID is an identifier, not a slice position. `RemovePartitionSpecs`
filters the slice without renumbering the survivors, and specs loaded from
metadata keep the `partition-specs` array order (no sort), so positions and IDs
come apart on ordinary tables. Because `specs[df.SpecID()]` is evaluated as a
call argument, it fails before `removeFile` ever runs: any
Delete/Overwrite/RewriteFiles commit that removes files panics with `index out
of range`, or — when the ID stays in bounds but points at the wrong entry —
silently attributes the change to the wrong partition.
`RemovePartitionSpecs([]int{1})` over specs 0/1/2 leaves `specs == [id0,
id2]`. Sending the *same* spec-2 data file down both branches:
| path | result |
| --- | --- |
| add | `partitions.id_trunc=0` |
| remove | `panic: runtime error: index out of range [2] with length 2` |
Same file, same spec, same function; only the branch differs.
The fix calls `sp.spec(...)` in the three removal loops and drops the
now-unused local. Unknown IDs then yield an empty unpartitioned spec instead of
panicking, matching the add path.
<details>
<summary>Test evidence</summary>
Regression test `TestAccumulateSummaryDeltaResolvesRemovedFileSpecByID` in
`table/snapshot_producers_test.go`. It asserts the add path first, so the
failure output itself shows the asymmetry rather than just "remove panics".
Before the fix:
```
=== RUN TestAccumulateSummaryDeltaResolvesRemovedFileSpecByID
--- FAIL: TestAccumulateSummaryDeltaResolvesRemovedFileSpecByID (0.00s)
panic: runtime error: index out of range [2] with length 2 [recovered,
repanicked]
...
github.com/apache/iceberg-go/table.(*snapshotProducer).accumulateSummaryDelta(...)
table/snapshot_producers.go:1023 +0x5e8
github.com/apache/iceberg-go/table.TestAccumulateSummaryDeltaResolvesRemovedFileSpecByID(...)
table/snapshot_producers_test.go:1115 +0x950
FAIL github.com/apache/iceberg-go/table 0.933s
```
After the fix:
```
=== RUN TestAccumulateSummaryDeltaResolvesRemovedFileSpecByID
--- PASS: TestAccumulateSummaryDeltaResolvesRemovedFileSpecByID (0.00s)
ok github.com/apache/iceberg-go/table 0.922s
```
| check | result |
| --- | --- |
| `go test ./...` | all 26 packages with tests ok |
| `go test -race ./codec/... ./table/...` | ok |
| `golangci-lint run` (v2.11.4, whole repo) | 0 issues |
| `gofmt -l` | clean |
Not run: the Docker-gated integration suite (`go test -tags integration
./...`) and the s390x cross-compile job. The change is confined to in-memory
summary accounting, with no catalog, IO or platform-specific code involved.
</details>
Assisted by Cursor; I reproduced the panic locally and reviewed every line
of the change.
--
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]
