This is an automated email from the ASF dual-hosted git repository.
lzljs3620320 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git
The following commit(s) were added to refs/heads/master by this push:
new 4edc05bc7b [core] Fix retract behavior for FieldProductAgg when
accumulator is null (#4842)
4edc05bc7b is described below
commit 4edc05bc7b459095e7ad2226d9543305d46b6042
Author: xiangyu0xf <[email protected]>
AuthorDate: Thu Jan 9 13:58:59 2025 +0800
[core] Fix retract behavior for FieldProductAgg when accumulator is null
(#4842)
---
docs/content/primary-key-table/merge-engine/aggregation.md | 2 ++
.../mergetree/compact/aggregate/FieldProductAgg.java | 2 +-
.../mergetree/compact/aggregate/FieldAggregatorTest.java | 14 +++++++-------
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/docs/content/primary-key-table/merge-engine/aggregation.md
b/docs/content/primary-key-table/merge-engine/aggregation.md
index 0cc6507f2b..4b3b8c4b5b 100644
--- a/docs/content/primary-key-table/merge-engine/aggregation.md
+++ b/docs/content/primary-key-table/merge-engine/aggregation.md
@@ -360,6 +360,8 @@ If you allow some functions to ignore retraction messages,
you can configure:
The `last_value` and `last_non_null_value` just set field to null when accept
retract messages.
+The `product` will return null for retraction message when accumulator is null.
+
The `collect` and `merge_map` make a best-effort attempt to handle retraction
messages, but the results are not
guaranteed to be accurate. The following behaviors may occur when processing
retraction messages:
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
index 26a0c0c52e..53ccfb94b3 100644
---
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 class FieldProductAgg extends FieldAggregator {
Object product;
if (accumulator == null || inputField == null) {
- product = (accumulator == null ? inputField : accumulator);
+ product = accumulator;
} else {
switch (fieldType.getTypeRoot()) {
case DECIMAL:
diff --git
a/paimon-core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java
b/paimon-core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java
index d32098b80f..cf99a11572 100644
---
a/paimon-core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java
+++
b/paimon-core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java
@@ -208,7 +208,7 @@ public class FieldAggregatorTest {
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)).isNull();
}
@Test
@@ -227,7 +227,7 @@ public class FieldAggregatorTest {
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)).isNull();
}
@Test
@@ -237,7 +237,7 @@ public class FieldAggregatorTest {
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)).isNull();
}
@Test
@@ -265,7 +265,7 @@ public class FieldAggregatorTest {
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)).isNull();
}
@Test
@@ -275,7 +275,7 @@ public class FieldAggregatorTest {
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)).isNull();
}
@Test
@@ -294,7 +294,7 @@ public class FieldAggregatorTest {
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)).isNull();
}
@Test
@@ -313,7 +313,7 @@ public class FieldAggregatorTest {
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))).isNull();
}
@Test
