badalprasadsingh commented on code in PR #2438:
URL: https://github.com/apache/iceberg-go/pull/2438#discussion_r4526966311
##########
schema.go:
##########
@@ -243,9 +243,22 @@ func (s *Schema) lazyNameToIDLower() (map[string]int,
error) {
return nil, err
}
- out := make(map[string]int)
- for k, v := range idx {
- out[strings.ToLower(k)] = v
+ names := make([]string, 0, len(idx))
+ for name := range idx {
+ names = append(names, name)
+ }
+ slices.Sort(names)
+ out := make(map[string]int, len(idx))
+ originalNames := make(map[string]string, len(idx))
+ for _, name := range names {
+ id := idx[name]
+ lower := strings.ToLower(name)
+ if previous, exists := out[lower]; exists && previous != id {
+ return nil, fmt.Errorf("%w: ambiguous case-insensitive
field names %q and %q",
+ ErrInvalidSchema, originalNames[lower], name)
Review Comment:
+1 to zeroshade, with ID and id, case-insensitive scans, Delete, Overwrite
and schema changes now fail even on name.
better: fail only for the ambiguous name and update the release note. note:
iceberg-java rejects the whole schema (mp since 1.7.0)
--
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]