badalprasadsingh commented on code in PR #2438:
URL: https://github.com/apache/iceberg-go/pull/2438#discussion_r4491885791
##########
website/src/releases.md:
##########
@@ -42,6 +42,7 @@ Per-release notes (highlights, breaking changes,
contributors) are published on
### Unreleased compatibility notes
+- **Sort order replacements reuse historical orders.** When a transaction
selects an equivalent order already in metadata, it reuses that order ID and
sends a `set-default-sort-order` update with the concrete ID instead of adding
a duplicate definition.
Review Comment:
main already does this when the order comes from an earlier commit.
what changes: an order added earlier in the same txn: going A, B, then back
to A used to commit B, and now commits A. I'd describe that instead.
##########
schema.go:
##########
@@ -245,7 +245,11 @@ func (s *Schema) lazyNameToIDLower() (map[string]int,
error) {
out := make(map[string]int)
for k, v := range idx {
- out[strings.ToLower(k)] = v
+ lower := strings.ToLower(k)
+ if previous, exists := out[lower]; exists && previous != v {
+ return nil, fmt.Errorf("%w: ambiguous case-insensitive
field name %q", ErrInvalidSchema, lower)
+ }
Review Comment:
P1: with columns `ID` and `id`, case-insensitive scans, Delete, Overwrite,
UpdateSchema and UpdateSpec now fail, even on name, with errors like "could not
bind reference 'name'"
better: name the collision, please add a release note, and return this error
from Reference.Bind, as Select does.
--
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]
