nssalian commented on code in PR #2375:
URL: https://github.com/apache/iceberg-go/pull/2375#discussion_r4517044230
##########
table/internal/variant_bounds.go:
##########
@@ -267,9 +268,12 @@ func appendLiteralToVariant(b *variant.Builder, it
iceberg.PrimitiveType, lit ic
case iceberg.BooleanType:
return b.AppendBool(lit.Any().(bool))
case iceberg.Int32Type:
- return b.AppendInt(int64(lit.Any().(int32)))
+ // AppendInt picks the smallest width; bounds must keep the
column's width.
+ return b.UnsafeAppendEncoded(binary.LittleEndian.AppendUint32(
+ []byte{byte(variant.PrimitiveInt32) << 2},
uint32(lit.Any().(int32))))
Review Comment:
Done in
https://github.com/apache/iceberg-go/pull/2375/commits/aff19214fa5c3e4642a0f35319ebfd93852ae527.
The header is now built only in the `AppendVariantInt8/16/32/64` helpers. The
tests decode the bounds instead (your other comment), so they don't need them.
##########
table/variant_shredded_write_test.go:
##########
@@ -779,8 +781,8 @@ func TestShreddedVariantWriteLargeDecimal(t *testing.T) {
}
}
-// columnPhysicalType returns the Parquet physical type of a top-level leaf
column.
-func columnPhysicalType(t *testing.T, path, name string) parquet.Type {
+// columnPhysicalType returns the Parquet physical and logical type of a leaf
column.
+func columnPhysicalType(t *testing.T, path, name string) (parquet.Type,
pqschema.LogicalType) {
Review Comment:
Done. `columnPhysicalType` is physical-only again, and the new test uses
`columnLogicalType`.
--
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]
