amogh-jahagirdar commented on code in PR #18604:
URL: https://github.com/apache/iceberg/pull/18604#discussion_r4343053015


##########
core/src/test/java/org/apache/iceberg/TestStatsUtil.java:
##########
@@ -564,6 +571,273 @@ public void testContentStatsWriteSchemaOmitsListAndMap() {
     assertThat(actual.fields()).isEmpty();
   }
 
+  @Test
+  void identityBounds() {
+    assertThat(sourceBounds(Types.StringType.get(), Transforms.identity(), 
"books"))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds("books", "books"));
+  }
+
+  @Test
+  void identityBoundsMatchSourceType() {
+    assertThat(sourceBounds(Types.LongType.get(), Transforms.identity(), 5))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(5L, 5L));
+    assertThat(sourceBounds(Types.DoubleType.get(), Transforms.identity(), 
1.5f))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(1.5d, 1.5d));
+  }
+
+  @Test
+  void identityNaN() {
+    assertThat(sourceBounds(Types.FloatType.get(), Transforms.identity(), 
Float.NaN)).isNull();
+    assertThat(sourceBounds(Types.DoubleType.get(), Transforms.identity(), 
Double.NaN)).isNull();
+  }
+
+  private static Stream<Partition> truncatedNumbers() {
+    Type decimal = Types.DecimalType.of(9, 2);
+    return Stream.of(
+        partition(Types.IntegerType.get(), Transforms.truncate(10), 
20).withExpectedBounds(20, 29),
+        partition(Types.IntegerType.get(), Transforms.truncate(10), -10)
+            .withExpectedBounds(-10, -1),
+        partition(Types.LongType.get(), Transforms.truncate(100), 12_300L)
+            .withExpectedBounds(12_300L, 12_399L),
+        partition(decimal, Transforms.truncate(50), new BigDecimal("12.00"))
+            .withExpectedBounds(new BigDecimal("12.00"), new 
BigDecimal("12.49")),
+        partition(decimal, Transforms.truncate(50), new BigDecimal("-0.50"))
+            .withExpectedBounds(new BigDecimal("-0.50"), new 
BigDecimal("-0.01")));
+  }
+
+  @ParameterizedTest
+  @MethodSource("truncatedNumbers")
+  void truncateNumberBounds(Partition partition) {
+    assertThat(sourceBounds(partition)).isEqualTo(partition.expectedBounds());
+  }
+
+  @Test
+  void truncateNumberNearMax() {
+    assertThat(sourceBounds(Types.IntegerType.get(), Transforms.truncate(10), 
2_147_483_640))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(2_147_483_640, 
Integer.MAX_VALUE));
+    assertThat(sourceBounds(Types.LongType.get(), Transforms.truncate(100), 
Long.MAX_VALUE - 7))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(Long.MAX_VALUE - 7, 
Long.MAX_VALUE));
+    assertThat(
+            sourceBounds(
+                Types.DecimalType.of(9, 2), Transforms.truncate(7), new 
BigDecimal("9999999.94")))
+        .isEqualTo(
+            new StatsUtil.PartitionSourceBounds(
+                new BigDecimal("9999999.94"), new BigDecimal("9999999.99")));
+  }
+
+  @Test
+  void truncateStringBounds() {
+    assertThat(sourceBounds(Types.StringType.get(), Transforms.truncate(3), 
"abc"))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds("abc", "abd"));
+  }
+
+  @Test
+  void truncateShortString() {
+    assertThat(sourceBounds(Types.StringType.get(), Transforms.truncate(3), 
"ab"))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds("ab", "ab"));
+  }
+
+  @Test
+  void truncateShortStringWithSurrogatePair() {
+    String value = "ab" + new String(Character.toChars(0x1F4B0));
+
+    assertThat(sourceBounds(Types.StringType.get(), Transforms.truncate(4), 
value))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(value, value));
+  }
+
+  @Test
+  void truncateStringAtMaxCodePoint() {
+    String maxCodePoint = new 
String(Character.toChars(Character.MAX_CODE_POINT));
+
+    assertThat(sourceBounds(Types.StringType.get(), Transforms.truncate(1), 
maxCodePoint))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(maxCodePoint, null));
+  }
+
+  @Test
+  void truncateBinaryBounds() {
+    ByteBuffer prefix = ByteBuffer.wrap(new byte[] {1, 2});
+
+    assertThat(sourceBounds(Types.BinaryType.get(), Transforms.truncate(2), 
prefix))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(prefix, 
ByteBuffer.wrap(new byte[] {1, 3})));
+  }
+
+  @Test
+  void truncateShortBinary() {
+    ByteBuffer value = ByteBuffer.wrap(new byte[] {1});
+
+    assertThat(sourceBounds(Types.BinaryType.get(), Transforms.truncate(2), 
value))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(value, value));
+  }
+
+  @Test
+  void binaryBoundsDoNotSharePartitionValueBuffers() {
+    ByteBuffer value = ByteBuffer.wrap(new byte[] {1});
+    StatsUtil.PartitionSourceBounds identityBounds =
+        sourceBounds(Types.BinaryType.get(), Transforms.identity(), value);
+    StatsUtil.PartitionSourceBounds truncateBounds =
+        sourceBounds(Types.BinaryType.get(), Transforms.truncate(2), value);
+
+    value.put(0, (byte) 9);
+
+    ByteBuffer original = ByteBuffer.wrap(new byte[] {1});
+    assertThat(identityBounds).isEqualTo(new 
StatsUtil.PartitionSourceBounds(original, original));
+    assertThat(truncateBounds).isEqualTo(new 
StatsUtil.PartitionSourceBounds(original, original));
+  }
+
+  @Test
+  void truncateBinaryAtMaxBytes() {
+    ByteBuffer prefix = ByteBuffer.wrap(new byte[] {(byte) 0xFF, (byte) 0xFF});
+
+    assertThat(sourceBounds(Types.BinaryType.get(), Transforms.truncate(2), 
prefix))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(prefix, null));
+  }
+
+  private static Stream<Partition> timePartitionValues() {
+    Type date = Types.DateType.get();
+    Type timestamp = Types.TimestampType.withZone();
+    Type timestampNano = Types.TimestampNanoType.withZone();
+    return Stream.of(
+        partition(date, Transforms.year(), years("2023-01-01"))
+            .withExpectedBounds(days("2023-01-01"), days("2023-12-31")),
+        partition(date, Transforms.month(), months("2023-02-01"))
+            .withExpectedBounds(days("2023-02-01"), days("2023-02-28")),
+        partition(date, Transforms.day(), days("2023-02-10"))
+            .withExpectedBounds(days("2023-02-10"), days("2023-02-10")),
+        partition(timestamp, Transforms.year(), years("2023-01-01"))
+            .withExpectedBounds(
+                micros("2023-01-01T00:00:00Z"), 
micros("2023-12-31T23:59:59.999999Z")),
+        partition(timestamp, Transforms.month(), months("2023-02-01"))
+            .withExpectedBounds(
+                micros("2023-02-01T00:00:00Z"), 
micros("2023-02-28T23:59:59.999999Z")),
+        partition(timestamp, Transforms.day(), days("2023-02-10"))
+            .withExpectedBounds(
+                micros("2023-02-10T00:00:00Z"), 
micros("2023-02-10T23:59:59.999999Z")),
+        partition(timestamp, Transforms.hour(), hours("2023-02-10T05:00:00Z"))
+            .withExpectedBounds(
+                micros("2023-02-10T05:00:00Z"), 
micros("2023-02-10T05:59:59.999999Z")),
+        partition(timestampNano, Transforms.hour(), 
hours("2023-02-10T05:00:00Z"))
+            .withExpectedBounds(
+                nanos("2023-02-10T05:00:00Z"), 
nanos("2023-02-10T05:59:59.999999999Z")));
+  }
+
+  @ParameterizedTest
+  @MethodSource("timePartitionValues")
+  void timeBounds(Partition partition) {
+    assertThat(sourceBounds(partition)).isEqualTo(partition.expectedBounds());
+  }
+
+  private static Stream<Partition> timePartitionValuesAtOrBefore1970() {
+    Type timestamp = Types.TimestampType.withZone();
+    return Stream.of(
+        partition(timestamp, Transforms.year(), 0)
+            .withExpectedBounds(
+                micros("1969-01-01T00:00:00Z"), 
micros("1970-12-31T23:59:59.999999Z")),
+        partition(timestamp, Transforms.hour(), -1)
+            .withExpectedBounds(
+                micros("1969-12-31T22:00:00Z"), 
micros("1969-12-31T23:59:59.999999Z")),
+        partition(Types.DateType.get(), Transforms.month(), -1)
+            .withExpectedBounds(days("1969-11-01"), days("1969-12-31")),
+        partition(Types.DateType.get(), Transforms.day(), -1)
+            .withExpectedBounds(days("1969-12-31"), days("1969-12-31")));
+  }
+
+  @ParameterizedTest
+  @MethodSource("timePartitionValuesAtOrBefore1970")
+  void timeBoundsAtOrBefore1970(Partition partition) {
+    assertThat(sourceBounds(partition)).isEqualTo(partition.expectedBounds());
+  }
+
+  @Test
+  void dateBoundsNearMax() {
+    // the end of year 5881580 is past the largest date
+    assertThat(sourceBounds(Types.DateType.get(), Transforms.year(), 
years("+5881580-01-01")))
+        .isEqualTo(new StatsUtil.PartitionSourceBounds(days("+5881580-01-01"), 
Integer.MAX_VALUE));
+  }
+
+  @Test
+  void timestampNanoBoundsNearMax() {
+    // the end of 2262 is past the largest nanosecond timestamp
+    assertThat(
+            sourceBounds(
+                Types.TimestampNanoType.withZone(), Transforms.year(), 
years("2262-01-01")))
+        .isEqualTo(
+            new StatsUtil.PartitionSourceBounds(nanos("2262-01-01T00:00:00Z"), 
Long.MAX_VALUE));
+  }
+
+  @Test
+  void nullPartitionValueRejected() {
+    assertThatThrownBy(() -> sourceBounds(Types.TimestampType.withZone(), 
Transforms.hour(), null))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Cannot derive source column bounds from null partition 
value");
+  }
+
+  @Test
+  void nonOrderPreservingTransformsRejected() {
+    assertThatThrownBy(() -> sourceBounds(Types.StringType.get(), 
Transforms.bucket(16), 3))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage(
+            "Cannot derive source column bounds from partition field that does 
not preserve order: 1000: part: bucket[16](1)");
+    assertThatThrownBy(() -> sourceBounds(Types.StringType.get(), 
Transforms.alwaysNull(), null))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage(
+            "Cannot derive source column bounds from partition field that does 
not preserve order: 1000: part: void(1)");
+  }
+
+  private static StatsUtil.PartitionSourceBounds sourceBounds(
+      Type type, Transform<?, ?> transform, Object partitionValue) {
+    Schema schema = new Schema(Types.NestedField.optional(1, "col", type));
+    PartitionField field =
+        PartitionSpec.builderFor(schema).add(1, "part", 
transform).build().fields().get(0);
+    return StatsUtil.sourceBoundsFor(schema, field, partitionValue);
+  }
+
+  private static StatsUtil.PartitionSourceBounds sourceBounds(Partition 
partition) {
+    return sourceBounds(partition.type(), partition.transform(), 
partition.value());
+  }
+
+  private static Partition partition(Type type, Transform<?, ?> transform, 
Object value) {
+    return new Partition(type, transform, value, null, null);
+  }
+
+  private record Partition(
+      Type type,
+      Transform<?, ?> transform,
+      Object value,
+      Object expectedLower,
+      Object expectedUpper) {
+    Partition withExpectedBounds(Object lower, Object upper) {
+      return new Partition(type, transform, value, lower, upper);
+    }
+
+    StatsUtil.PartitionSourceBounds expectedBounds() {
+      return new StatsUtil.PartitionSourceBounds(expectedLower, expectedUpper);
+    }
+  }
+
+  private static int years(String isoDate) {
+    return DateTimeUtil.daysToYears(days(isoDate));
+  }
+
+  private static int months(String isoDate) {
+    return DateTimeUtil.daysToMonths(days(isoDate));
+  }
+
+  private static int days(String isoDate) {
+    return DateTimeUtil.isoDateToDays(isoDate);
+  }
+
+  private static int hours(String isoTimestamp) {
+    return DateTimeUtil.microsToHours(micros(isoTimestamp));
+  }
+
+  private static long micros(String isoTimestamp) {
+    return DateTimeUtil.isoTimestamptzToMicros(isoTimestamp);
+  }
+
+  private static long nanos(String isoTimestamp) {
+    return DateTimeUtil.isoTimestamptzToNanos(isoTimestamp);
+  }

Review Comment:
   I think these helpers make the tests a lot easier to read, but I know we 
don't want a proliferation of too many of these things in tests. It's just that 
inlining the util makes it super dense to read imo



-- 
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