findepi commented on PR #11704: URL: https://github.com/apache/iceberg/pull/11704#issuecomment-2583537159
I see benefit of having CharSequenceMap implement Map contract, eg getting all the rich Map interface. In my opinion it's not worth the risk though -- Map interface default implementations cannot be assumed to be correct. I see the point that it's not much different from TreeMap. It's conceptually same problem, the difference is in how the code is developed and vetted. JDK team very much thinks of TreeMap whenever adding a new Map method, while this doesn't hold true for CharSequenceMap. All in all, I am convinced it's better to unimplement Map interface. This seems to me the only way to have a correct implementation of that data structure. The richness of Map interface can be conveniently provided via `asMap` view (using CharSequenceWrapper in the key type). If you still think it's better to keep using an incorrect implementation, for whatever reason, I will close this PR. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
