tanmayrauth commented on PR #2444: URL: https://github.com/apache/iceberg-go/pull/2444#issuecomment-6750483829
Thanks @zeroshade for digging into this, especially checking the types the tests didn't cover. Good catch on the non-primitive columns: named columns that are unknown, non-primitive, or nested (like s.a, which was also being dropped silently) now return ErrInvalidArgument with Java's wording, and the doc and a test are updated. I also added TestComputeNDVsMatchesJavaFixtures, which writes the long and string fixture rows into a table and compares the result against long.sk / string.sk; I changed one value to make sure it fails when it should. On perf, agreed, building a literal per value is wasteful. I'd like to do the Arrow-buffer hashing in a follow-up so this PR stays on the MarshalBinary path the fixtures validate, and I'm happy to open an issue for it. -- 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]
