From 6538ce6e4bc20d83018d5ac4d3e8f30aa57984d5 Mon Sep 17 00:00:00 2001 From: Aihua Xu Date: Sun, 27 Jul 2025 12:27:08 -0700 Subject: [PATCH 1/2] Use decimal precision to determine variant decimal type --- .../iceberg/variants/TestSerializedPrimitives.java | 4 ++-- .../java/org/apache/iceberg/variants/Variants.java | 11 ++++++----- .../apache/iceberg/variants/TestPrimitiveWrapper.java | 8 ++++---- .../apache/iceberg/parquet/TestVariantReaders.java | 8 ++++---- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java b/api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java index c9ad94875472..b173b7e74252 100644 --- a/api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java +++ b/api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java @@ -185,7 +185,7 @@ public void testDecimal4() { new byte[] {primitiveHeader(8), 0x04, (byte) 0xD2, 0x02, (byte) 0x96, 0x49}); assertThat(value.type()).isEqualTo(PhysicalType.DECIMAL4); - assertThat(value.get()).isEqualTo(new BigDecimal("123456.7890")); + assertThat(value.get()).isEqualTo(new BigDecimal("12345.6789")); } @Test @@ -218,7 +218,7 @@ public void testDecimal8() { }); assertThat(value.type()).isEqualTo(PhysicalType.DECIMAL8); - assertThat(value.get()).isEqualTo(new BigDecimal("1234567890.987654321")); + assertThat(value.get()).isEqualTo(new BigDecimal("123456789.987654321")); } @Test diff --git a/core/src/main/java/org/apache/iceberg/variants/Variants.java b/core/src/main/java/org/apache/iceberg/variants/Variants.java index 5591145ca603..ab538dae44b5 100644 --- a/core/src/main/java/org/apache/iceberg/variants/Variants.java +++ b/core/src/main/java/org/apache/iceberg/variants/Variants.java @@ -186,16 +186,17 @@ public static VariantPrimitive ofIsoTimestampntz(String value) { } public static VariantPrimitive of(BigDecimal value) { - int bitLength = value.unscaledValue().bitLength(); - if (bitLength < 32) { + int precision = value.precision(); + + if (precision >= 1 && precision <= 9) { return new PrimitiveWrapper<>(PhysicalType.DECIMAL4, value); - } else if (bitLength < 64) { + } else if (precision >= 10 && precision <= 18) { return new PrimitiveWrapper<>(PhysicalType.DECIMAL8, value); - } else if (bitLength < 128) { + } else if (precision <= 38) { return new PrimitiveWrapper<>(PhysicalType.DECIMAL16, value); } - throw new UnsupportedOperationException("Unsupported decimal precision: " + value.precision()); + throw new UnsupportedOperationException("Unsupported decimal precision: " + precision); } public static VariantPrimitive of(ByteBuffer value) { diff --git a/core/src/test/java/org/apache/iceberg/variants/TestPrimitiveWrapper.java b/core/src/test/java/org/apache/iceberg/variants/TestPrimitiveWrapper.java index 108132087a4e..5af3dbfb5f47 100644 --- a/core/src/test/java/org/apache/iceberg/variants/TestPrimitiveWrapper.java +++ b/core/src/test/java/org/apache/iceberg/variants/TestPrimitiveWrapper.java @@ -52,10 +52,10 @@ public class TestPrimitiveWrapper { Variants.ofIsoTimestamptz("1957-11-07T12:33:54.123456+00:00"), Variants.ofIsoTimestampntz("2024-11-07T12:33:54.123456"), Variants.ofIsoTimestampntz("1957-11-07T12:33:54.123456"), - Variants.of(new BigDecimal("123456.7890")), // decimal4 - Variants.of(new BigDecimal("-123456.7890")), // decimal4 - Variants.of(new BigDecimal("1234567890.987654321")), // decimal8 - Variants.of(new BigDecimal("-1234567890.987654321")), // decimal8 + Variants.of(new BigDecimal("12345.6789")), // decimal4 + Variants.of(new BigDecimal("-12345.6789")), // decimal4 + Variants.of(new BigDecimal("123456789.987654321")), // decimal8 + Variants.of(new BigDecimal("-123456789.987654321")), // decimal8 Variants.of(new BigDecimal("9876543210.123456789")), // decimal16 Variants.of(new BigDecimal("-9876543210.123456789")), // decimal16 Variants.of(ByteBuffer.wrap(new byte[] {0x0a, 0x0b, 0x0c, 0x0d})), diff --git a/parquet/src/test/java/org/apache/iceberg/parquet/TestVariantReaders.java b/parquet/src/test/java/org/apache/iceberg/parquet/TestVariantReaders.java index ed2a8b61be77..ffefbc3ce90d 100644 --- a/parquet/src/test/java/org/apache/iceberg/parquet/TestVariantReaders.java +++ b/parquet/src/test/java/org/apache/iceberg/parquet/TestVariantReaders.java @@ -122,10 +122,10 @@ public class TestVariantReaders { Variants.ofIsoTimestamptz("1957-11-07T12:33:54.123456+00:00"), Variants.ofIsoTimestampntz("2024-11-07T12:33:54.123456"), Variants.ofIsoTimestampntz("1957-11-07T12:33:54.123456"), - Variants.of(new BigDecimal("123456.7890")), // decimal4 - Variants.of(new BigDecimal("-123456.7890")), // decimal4 - Variants.of(new BigDecimal("1234567890.987654321")), // decimal8 - Variants.of(new BigDecimal("-1234567890.987654321")), // decimal8 + Variants.of(new BigDecimal("12345.6789")), // decimal4 + Variants.of(new BigDecimal("-12345.6789")), // decimal4 + Variants.of(new BigDecimal("123456789.987654321")), // decimal8 + Variants.of(new BigDecimal("-123456789.987654321")), // decimal8 Variants.of(new BigDecimal("9876543210.123456789")), // decimal16 Variants.of(new BigDecimal("-9876543210.123456789")), // decimal16 Variants.of(ByteBuffer.wrap(new byte[] {0x0a, 0x0b, 0x0c, 0x0d})), From b9137ffa8104104473ac4ed0db0f94cda0bd6b5f Mon Sep 17 00:00:00 2001 From: Aihua Xu Date: Mon, 28 Jul 2025 11:24:02 -0700 Subject: [PATCH 2/2] Update tests --- .../variants/TestSerializedPrimitives.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java b/api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java index b173b7e74252..37ae2085fdd0 100644 --- a/api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java +++ b/api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java @@ -182,7 +182,7 @@ public void testNegativeDouble() { public void testDecimal4() { VariantPrimitive value = SerializedPrimitive.from( - new byte[] {primitiveHeader(8), 0x04, (byte) 0xD2, 0x02, (byte) 0x96, 0x49}); + new byte[] {primitiveHeader(8), 0x04, (byte) 0x15, (byte) 0xCD, (byte) 0x5B, 0x07}); assertThat(value.type()).isEqualTo(PhysicalType.DECIMAL4); assertThat(value.get()).isEqualTo(new BigDecimal("12345.6789")); @@ -208,13 +208,13 @@ public void testDecimal8() { primitiveHeader(9), 0x09, // scale=9 (byte) 0xB1, - 0x1C, - 0x6C, - (byte) 0xB1, - (byte) 0xF4, - 0x10, - 0x22, - 0x11 + (byte) 0xFA, + 0x52, + (byte) 0xE0, + (byte) 0x4B, + (byte) 0x9B, + (byte) 0xB6, + 0x01 }); assertThat(value.type()).isEqualTo(PhysicalType.DECIMAL8);