nssalian commented on code in PR #2002:
URL: https://github.com/apache/iceberg-go/pull/2002#discussion_r4020191139
##########
table/arrow_utils.go:
##########
@@ -436,7 +436,7 @@ func (c convertToIceberg) Primitive(dt arrow.DataType)
(result iceberg.NestedFie
switch dt.ExtensionName() {
case "arrow.uuid":
result.Type = iceberg.PrimitiveTypes.UUID
- case "parquet.variant":
+ case extensions.VariantExtensionName,
extensions.LegacyVariantExtensionName:
Review Comment:
The repo has no `CHANGELOG.md` (release notes are managed on the GitHub
release). Added the compatibility note to the PR description instead: files
written with the canonical `arrow.parquet.variant` name are read fine by
Java/pqarrow/iceberg-rust; only old iceberg-go readers (still matching
`parquet.variant`) in a mixed-version fleet are affected. No code change (both
names are accepted on read).
##########
table/internal/parquet_files.go:
##########
@@ -711,9 +711,35 @@ func getWriteProperties(writeProps any, arrowSchema
*arrow.Schema) (*parquet.Wri
wp = append(wp, parquet.WithStoreDecimalAsInteger(true))
}
+ // Match Iceberg Java: apply parquet-mr's cost-based dictionary
fallback to every leaf
+ // column so high-cardinality columns fall back to PLAIN rather than
keeping a dictionary.
+ // arrow-go otherwise enables it only for uncompressed columns, so zstd
(our default) would
+ // retain dictionaries on all-distinct columns and roughly double their
size.
+ costFallback, err := dictCostFallbackProps(arrowSchema, wp)
+ if err != nil {
+ return nil, err
+ }
+ wp = append(wp, costFallback...)
+
return parquet.NewWriterProperties(wp...), nil
}
+// dictCostFallbackProps returns a WithDictionaryCostFallbackFor(true)
property for every leaf
+// column of arrowSchema, resolving parquet leaf paths through arrow-go's own
schema conversion.
+func dictCostFallbackProps(arrowSchema *arrow.Schema, base
[]parquet.WriterProperty) ([]parquet.WriterProperty, error) {
+ parquetSchema, err := pqarrow.ToParquet(arrowSchema,
parquet.NewWriterProperties(base...), pqarrow.DefaultWriterProps())
Review Comment:
Done. `dictCostFallbackProps` now walks the arrow schema directly (dotted
field names, extension types unwrapped to storage) and falls back to
`ToParquet` only for list/map schemas.
`TestDictCostFallbackWalkMatchesToParquet` proves the walk's paths equal
`ToParquet`'s for flat, nested-struct, shredded-variant, and decimal schemas.
(Note: the second conversion inside `pqarrow.NewFileWriter` is
arrow-go-internal, so sharing one `parquet.Schema` isn't possible; this removes
our redundant one.)
--
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]
