rraulinio opened a new issue, #1265:
URL: https://github.com/apache/iceberg-go/issues/1265
### Apache Iceberg version
main (development)
### Please describe the bug 🐞
# Description
Running the table package under the race detector exposed two independent
races.
## 1. `PartitionToPath` races on lazy partition field name escaping
`PartitionSpec.PartitionToPath()` can be called concurrently by fanout
writers. It builds partition paths like:
```text
bucket=1/day=2026-06-23
```
To avoid escaping the partition field name every time,
`PartitionField.EscapedName()` cached the escaped field name lazily on the
`PartitionField` itself.
That lazy cache write is not safe when multiple goroutines use the same
`PartitionSpec` at the same time. The first concurrent calls can all see an
empty `escapedName` and then write to the same field concurrently.
Observed stack:
```text
WARNING: DATA RACE
Write at ... by goroutine ...:
github.com/apache/iceberg-go.(*PartitionField).EscapedName()
partitions.go:74
github.com/apache/iceberg-go.(*PartitionSpec).PartitionToPath()
partitions.go:541
github.com/apache/iceberg-go/table.(*partitionedFanoutWriter).partitionPath()
table/partitioned_fanout_writer.go:73
github.com/apache/iceberg-go/table.(*partitionedFanoutWriter).processRecord()
table/partitioned_fanout_writer.go:159
```
## 2. Parallel table tests mutate the shared positional delete schema
Some table tests pass the package-level `iceberg.PositionalDeleteSchema`
directly into `MetadataBuilder.AddSchema`.
`AddSchema` assigns a schema ID onto the schema object it receives. That
means the test is not just reading the shared `PositionalDeleteSchema`; it can
mutate it.
This is fine in serial execution, but several table tests run with
`t.Parallel()`. Under the race detector, two tests can touch the same shared
schema pointer at the same time:
- one test writes `schema.ID` through `MetadataBuilder.AddSchema`
- another test reads the same schema while adding/building metadata
# How to reproduce
The table package was not included in the existing CI race-detector target,
so run it manually:
```bash
go test -race ./table/...
```
# Expected behavior
The table package should pass under the race detector.
`PartitionSpec.PartitionToPath()` should be safe to call concurrently
because fanout writers naturally call it from multiple goroutines.
Tests should not mutate package-level shared schemas from parallel subtests.
# Proposed fixes
1. Precompute `PartitionField.escapedName` during
`PartitionSpec.initialize()`, which is already called by partition spec
constructors and JSON unmarshal. Then make `EscapedName()` read-only at call
time: return the precomputed value when present, otherwise compute
`url.QueryEscape(p.Name)` without writing back.
2. Clone `iceberg.PositionalDeleteSchema` before passing it to
`MetadataBuilder.AddSchema` in the affected tests. This keeps the fix
test-local and avoids changing production `MetadataBuilder` behavior.
3. Add `./table/...` to the CI race-detector command so these paths stay
covered.
# Verification
After the fixes:
```text
go test -race ./table/...
ok github.com/apache/iceberg-go/table 27.394s
ok github.com/apache/iceberg-go/table/compaction 48.721s
ok github.com/apache/iceberg-go/table/dv (cached)
ok github.com/apache/iceberg-go/table/internal (cached)
ok github.com/apache/iceberg-go/table/substrait (cached)
```
--
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]
