anoopj commented on code in PR #18604:
URL: https://github.com/apache/iceberg/pull/18604#discussion_r4353571687
##########
core/src/main/java/org/apache/iceberg/StatsUtil.java:
##########
@@ -325,4 +336,181 @@ private static boolean isScalar(Schema schema,
Map<Integer, Integer> parentIndex
return true;
}
+
+ /** Inclusive source column bounds for a partition value. Upper is null when
unbounded. */
+ record PartitionSourceBounds(Object lower, Object upper) {}
+
+ /**
+ * Returns the source column bounds for a partition value, or null for a NaN
identity value.
+ *
+ * @throws IllegalArgumentException if the transform does not preserve order
or the value is null
+ */
+ static PartitionSourceBounds sourceBoundsFor(
+ Schema schema, PartitionField field, Object partitionValue) {
+ Preconditions.checkArgument(
+ field.transform().preservesOrder(),
+ "Cannot derive source column bounds from partition field that does not
preserve order: %s",
+ field);
+ Preconditions.checkArgument(
+ partitionValue != null, "Cannot derive source column bounds from null
partition value");
+ Type sourceType = schema.findType(field.sourceId());
+ Preconditions.checkArgument(
+ sourceType != null, "Cannot find source column for partition field:
%s", field);
+ return PartitionSpecVisitor.visit(
+ schema, field, new PartitionSourceBoundsVisitor(sourceType,
partitionValue));
+ }
+
+ private static class PartitionSourceBoundsVisitor
+ implements PartitionSpecVisitor<PartitionSourceBounds> {
+ private final Type sourceType;
+ private final Object value;
+
+ private PartitionSourceBoundsVisitor(Type sourceType, Object value) {
+ this.sourceType = sourceType;
+ this.value = value;
+ }
+
+ @Override
+ public PartitionSourceBounds identity(int fieldId, String sourceName, int
sourceId) {
+ // convert to the source type, which may have been promoted after the
value was written
+ Object bound =
+ switch (sourceType.typeId()) {
+ case LONG -> ((Number) value).longValue();
+ case DOUBLE -> ((Number) value).doubleValue();
+ default -> value;
+ };
+
+ if (bound instanceof ByteBuffer buffer) {
+ bound = ByteBuffers.copy(buffer);
+ }
+
+ // NaN cannot be used as a bound
+ if (bound instanceof Double d && d.isNaN() || bound instanceof Float f
&& f.isNaN()) {
+ return null;
+ }
+
+ return new PartitionSourceBounds(bound, bound);
+ }
+
+ @Override
+ public PartitionSourceBounds truncate(int fieldId, String sourceName, int
sourceId, int width) {
+ // source values are at most width - 1 larger than the partition value,
unscaled for decimals
+ return switch (sourceType.typeId()) {
+ case INTEGER -> {
+ int lower = (Integer) value;
+ yield new PartitionSourceBounds(
+ lower, (int) Math.min(Integer.MAX_VALUE, (long) lower + width -
1));
+ }
+ case LONG -> {
+ long lower = ((Number) value).longValue();
+ yield new PartitionSourceBounds(
+ lower, lower > Long.MAX_VALUE - (width - 1) ? Long.MAX_VALUE :
lower + width - 1);
+ }
Review Comment:
There is an edge condition when the overflow happens. e.g. if the source
value is `Integer.MIN_VALUE`, the truncate will overflow into a positive value
because java integer arithmetic will wrap around. (which is current behavior).
But I think this will produce the wrong bounds which wont match the original
source value.
--
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]
