Shruthirdy opened a new issue, #2540:
URL: https://github.com/apache/iceberg-go/issues/2540
### Apache Iceberg version
None
### Please describe the bug 🐞
`Schema.UnmarshalJSON` accepts a schema where two fields have ID `0`, but
rejects the same schema when both fields have ID `1`.
That's because it uses `checkDuplicateFieldIDs`, which skips IDs ≤ 0. Those
IDs are allowed as placeholders while a schema is being built (#879), but a
schema read from metadata JSON is already complete, so duplicate 0s shouldn't
get through.
After parsing, lookups by ID on that schema can return the wrong field. For
example, `FindColumnName(0)` returns `created_at` or `event` depending on Go's
map iteration order. That's the same lookup behind #2393. #2475 fixes the write
path (`NewMetadata`), but the read path still accepts these schemas.
For comparison, Java rejects duplicate field IDs, including 0, when it
builds a `Schema`.
### Repro
```go
for _, id := range []int{0, 1} {
data := fmt.Sprintf(`{"type":"struct","schema-id":0,"fields":[
{"id":%d,"name":"created_at","required":false,"type":"timestamptz"},
{"id":%d,"name":"event","required":false,"type":"string"}]}`,
id, id)
var sc iceberg.Schema
err := json.Unmarshal([]byte(data), &sc)
fmt.Printf("id=%d err=%v\n", id, err)
}
```
Output:
```
id=0 err=<nil>
id=1 err=invalid schema: multiple fields for id 1: created_at and event
```
### Expected
Duplicate field IDs, including 0, are caught instead of silently accepted,
as they are for positive IDs. Java's `SchemaParser` builds a `Schema`, so it
already rejects these on read.
### Possible fixes
This was left out of #2475 because it changes the read path, where being
stricter can stop tables that load today from loading.
1. **Reject in `UnmarshalJSON`**: check every ID, including 0 and negatives.
This is the simplest and matches Java, but such tables would fail to load.
2. **Reject only when it matters**: fail on load only if a partition spec,
sort order or identifier field refers to a duplicated ID, since that's when
lookups go wrong. Other tables keep loading.
3. **Keep loading, reject on write**: leave parsing as it is, and reject
duplicate IDs wherever a schema is committed, so such tables can still be read
but not updated.
I'm happy to work on whichever approach is preferred.
--
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]
