ajiteshsingh commented on code in PR #2009:
URL: https://github.com/apache/iceberg-go/pull/2009#discussion_r4020735409


##########
table/evaluators.go:
##########
@@ -790,6 +792,24 @@ type inclusiveMetricsEval struct {
        includeEmptyFiles bool
 }
 
+// intBackedDecimal reports whether a column's statistics are a decimal that
+// Parquet stores in an INT32 or INT64. Parquet's plain encoding for those is
+// little-endian, while an Iceberg bound is big-endian two's complement, so the
+// stat bytes have to be reversed before they can be used as a bound: 659 read
+// as-is would come back as -1828585472 and prune every row group that matches.
+// A FIXED_LEN_BYTE_ARRAY-backed decimal is big-endian in Parquet too, so its
+// bounds are already in Iceberg's form.
+func intBackedDecimal(descr *parquetschema.Column) bool {

Review Comment:
   Yeah, a small helper makes sense to avoid future silent bugs. Pulled the 
EncodeMin/EncodeMax + reverse-if-int-backed step into parquetStatsBounds, with 
a doc comment saying any evaluator populating bounds from raw Parquet stats has 
to go through it. TestRowGroup calls it now, so a future strict or DV stats 
path can just reuse 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]

Reply via email to