Revanth14 opened a new pull request, #2139: URL: https://github.com/apache/iceberg-go/pull/2139
Follow-up to #2097, addressing the nits ## Changes - **Length-prefix test** (`literal_set_internal_test.go`): adds a case with two distinct variants whose metadata and value bytes concatenate to the same sequence, using @zeroshade's example (metadata accepts trailing bytes). `addliteral` overwrites on a key match without calling `Equals`, so the length prefix in `variantKey` is what keeps both members. Previously no test failed if the prefix was removed; this one does (1 member instead of 2). - **`variantKey` comment** (`utils.go`): states that it must hash exactly what `VariantLiteral.Equals` compares, why the length prefix matters for correctness, and that a genuine 64-bit collision still drops the earlier member, as for Binary, Fixed, and Geo. - **Subtest rename** (`exprs_test.go`): now `bind to variant column keeps distinct members`, and the `ErrType` assertion stays. With two distinct values, `ErrType` is only reachable if the bind-time set still holds both. If they collapsed to one, binding would take the single-literal path and fail with `ErrInvalidArgument` (I checked this). - **Test helpers:** the `build` closures captured the parent `t`, so a failure inside a subtest produced `subtest may have called FailNow on a parent test`. They now use helpers that take the subtest's `t`: the existing `variantLiteralOf`, and a small wrapper over the existing `scalarVariant`. ## Validation `gofmt`, `go vet .`, and `go test -race .` pass. -- 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]
