Skip to content

[core] Fix retract behavior for FieldProductAgg when accumulator is null #4842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JingsongLi merged 1 commit into from
Jan 9, 2025
Merged

[core] Fix retract behavior for FieldProductAgg when accumulator is null #4842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/content/primary-key-table/merge-engine/aggregation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
2 changes: 1 addition & 1 deletion paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/FieldProductAgg.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
} else {
switch (fieldType.getTypeRoot()) {
case DECIMAL:
Expand Down
14 changes: 7 additions & 7 deletions ...core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)).isNull();
}

@Test
Expand All @@ -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)).isNull();
}

@Test
Expand All @@ -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)).isNull();
}

@Test
Expand Down Expand Up @@ -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)).isNull();
}

@Test
Expand All @@ -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)).isNull();
}

@Test
Expand All @@ -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)).isNull();
}

@Test
Expand All @@ -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))).isNull();
}

@Test
Expand Down
Loading