claudevdm commented on code in PR #39720:
URL: https://github.com/apache/beam/pull/39720#discussion_r3760608893
##########
sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/AddFiles.java:
##########
@@ -650,6 +650,24 @@ static String getPartitionFromMetrics(Metrics metrics,
InputFile inputFile, Tabl
"Min and max transformed values were not equal, for column: " +
field.name());
}
+ // Equal transformed bounds only cover the values in between for
+ // order-preserving transforms. For bucket, which hashes the value mod
N
+ // , min and max can land in the same bucket while intermediate values
+ // land in others. The void transform maps every value to null and
needs
+ // no check.
+ if (!transform.preservesOrder()
+ && !transform.isVoid()
+ && !Objects.deepEquals(
+ Conversions.fromByteBuffer(type, lowerBytes),
+ Conversions.fromByteBuffer(type, upperBytes))) {
Review Comment:
But we cannot guarantee from summary statistics bucket transform is valid
unless max == min. Should we consider making it safe by default and have a flag
for opting into unchecked bucketing?
Users might think "oh neat I can apply bucket transforms on any column
safely?". Instead they should read the fine print and assert that "my files are
already clustered by this spec's buckets, trust them.".
--
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]
