wombatu-kun commented on code in PR #17010:
URL: https://github.com/apache/iceberg/pull/17010#discussion_r3497723597
##########
api/src/main/java/org/apache/iceberg/types/IndexByName.java:
##########
@@ -76,9 +82,15 @@ public Map<String, Integer> byName() {
* @return a map from field ID to name
*/
public Map<Integer, String> byId() {
+ // a builder is used to swap key and value
ImmutableMap.Builder<Integer, String> builder = ImmutableMap.builder();
nameToId.forEach((key, value) -> builder.put(value, key));
- return builder.build();
+
+ if (useShortNames) {
+ shortNameToId.forEach((key, value) -> builder.put(value, key));
+ }
+
+ return builder.buildKeepingLast();
Review Comment:
Switching `byId()` from `build()` to `buildKeepingLast()` also changes the
two existing callers of `byId()`, `TypeUtil.indexNameById` and
`indexQuotedNameById`. `Schema`'s constructor relies on `indexNameById` to
reject duplicate field IDs: it calls `lazyIdToName()` -> `indexNameById()` ->
`byId()`, and the `ImmutableMap` keyed by field ID used to throw on a duplicate
key. With `buildKeepingLast()` a schema with two fields sharing an ID is now
silently accepted, which is why `TestNameMapping.testFailsDuplicateId` fails on
both JDK 17 and 21 ("Expecting code to raise a throwable").
The short-name-overrides-full-name behavior is only needed for the new
`indexStatsNames` path, so gating `buildKeepingLast()` behind `useShortNames`
keeps the existing validation intact:
```suggestion
if (useShortNames) {
// short names intentionally override the full name for the same field
ID
shortNameToId.forEach((key, value) -> builder.put(value, key));
return builder.buildKeepingLast();
}
return builder.build();
```
--
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]
