Fix retract behavior for FieldProductAgg

herefree · Jan 5, 2025 · b7f5f86 · b7f5f86
1 parent ffb1a71
commit b7f5f86
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 8 deletions.
diff --git a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/FieldProductAgg.java b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/FieldProductAgg.java
@@ -89,7 +89,7 @@ public Object retract(Object accumulator, Object inputField) {
         Object product;
 
         if (accumulator == null || inputField == null) {
-            product = (accumulator == null ? inputField : accumulator);
+            product = (accumulator == null ? inverse(inputField) : accumulator);
         } else {
             switch (fieldType.getTypeRoot()) {
                 case DECIMAL:
@@ -132,4 +132,36 @@ public Object retract(Object accumulator, Object inputField) {
         }
         return product;
     }
+
+    private Object inverse(Object value) {
+        if (value == null) {
+            return null;
+        }
+        switch (fieldType.getTypeRoot()) {
+            case DECIMAL:
+                Decimal decimal = (Decimal) value;
+                return Decimal.fromBigDecimal(
+                        BigDecimal.ONE.divide(decimal.toBigDecimal()),
+                        decimal.precision(),
+                        decimal.scale());
+            case TINYINT:
+                return (byte) ((byte) 1 / (byte) value);
+            case SMALLINT:
+                return (short) ((short) 1 / (short) value);
+            case INTEGER:
+                return 1 / ((int) value);
+            case BIGINT:
+                return 1L / (long) value;
+            case FLOAT:
+                return 1f / (float) value;
+            case DOUBLE:
+                return 1d / (double) value;
+            default:
+                String msg =
+                        String.format(
+                                "type %s not support in %s",
+                                fieldType.getTypeRoot().toString(), this.getClass().getName());
+                throw new IllegalArgumentException(msg);
+        }
+    }
 }
diff --git a/...core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java b/...core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java
@@ -208,7 +208,7 @@ public void testFieldProductIntAgg() {
         assertThat(fieldProductAgg.agg(null, 10)).isEqualTo(10);
         assertThat(fieldProductAgg.agg(1, 10)).isEqualTo(10);
         assertThat(fieldProductAgg.retract(10, 5)).isEqualTo(2);
-        assertThat(fieldProductAgg.retract(null, 5)).isEqualTo(5);
+        assertThat(fieldProductAgg.retract(null, 5)).isEqualTo(0);
     }
 
     @Test
@@ -227,7 +227,7 @@ public void testFieldProductByteAgg() {
         assertThat(fieldProductAgg.agg(null, (byte) 10)).isEqualTo((byte) 10);
         assertThat(fieldProductAgg.agg((byte) 1, (byte) 10)).isEqualTo((byte) 10);
         assertThat(fieldProductAgg.retract((byte) 10, (byte) 5)).isEqualTo((byte) 2);
-        assertThat(fieldProductAgg.retract(null, (byte) 5)).isEqualTo((byte) 5);
+        assertThat(fieldProductAgg.retract(null, (byte) 5)).isEqualTo((byte) 0);
     }
 
     @Test
@@ -237,7 +237,7 @@ public void testFieldProductShortAgg() {
         assertThat(fieldProductAgg.agg(null, (short) 10)).isEqualTo((short) 10);
         assertThat(fieldProductAgg.agg((short) 1, (short) 10)).isEqualTo((short) 10);
         assertThat(fieldProductAgg.retract((short) 10, (short) 5)).isEqualTo((short) 2);
-        assertThat(fieldProductAgg.retract(null, (short) 5)).isEqualTo((short) 5);
+        assertThat(fieldProductAgg.retract(null, (short) 5)).isEqualTo((short) 0);
     }
 
     @Test
@@ -265,7 +265,7 @@ public void testFieldProductLongAgg() {
         assertThat(fieldProductAgg.agg(null, 10L)).isEqualTo(10L);
         assertThat(fieldProductAgg.agg(1L, 10L)).isEqualTo(10L);
         assertThat(fieldProductAgg.retract(10L, 5L)).isEqualTo(2L);
-        assertThat(fieldProductAgg.retract(null, 5L)).isEqualTo(5L);
+        assertThat(fieldProductAgg.retract(null, 5L)).isEqualTo(0L);
     }
 
     @Test
@@ -275,7 +275,7 @@ public void testFieldProductFloatAgg() {
         assertThat(fieldProductAgg.agg(null, (float) 10)).isEqualTo((float) 10);
         assertThat(fieldProductAgg.agg((float) 1, (float) 10)).isEqualTo((float) 10);
         assertThat(fieldProductAgg.retract((float) 10, (float) 5)).isEqualTo((float) 2);
-        assertThat(fieldProductAgg.retract(null, (float) 5)).isEqualTo((float) 5);
+        assertThat(fieldProductAgg.retract(null, (float) 5)).isEqualTo((float) 0.2);
     }
 
     @Test
@@ -294,7 +294,7 @@ public void testFieldProductDoubleAgg() {
         assertThat(fieldProductAgg.agg(null, (double) 10)).isEqualTo((double) 10);
         assertThat(fieldProductAgg.agg((double) 1, (double) 10)).isEqualTo((double) 10);
         assertThat(fieldProductAgg.retract((double) 10, (double) 5)).isEqualTo((double) 2);
-        assertThat(fieldProductAgg.retract(null, (double) 5)).isEqualTo((double) 5);
+        assertThat(fieldProductAgg.retract(null, (double) 5)).isEqualTo(0.2);
     }
 
     @Test
@@ -313,7 +313,7 @@ public void testFieldProductDecimalAgg() {
         assertThat(fieldProductAgg.agg(null, toDecimal(10))).isEqualTo(toDecimal(10));
         assertThat(fieldProductAgg.agg(toDecimal(1), toDecimal(10))).isEqualTo(toDecimal(10));
         assertThat(fieldProductAgg.retract(toDecimal(10), toDecimal(5))).isEqualTo(toDecimal(2));
-        assertThat(fieldProductAgg.retract(null, toDecimal(5))).isEqualTo(toDecimal(5));
+        assertThat(fieldProductAgg.retract(null, toDecimal(5))).isEqualTo(toDecimal(0));
     }
 
     @Test