You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "PyRSA (via GitHub)" <gi...@apache.org> on 2026/06/28 07:41:43 UTC
[PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
PyRSA opened a new pull request, #8374:
URL: https://github.com/apache/paimon/pull/8374
### Purpose
[Feature] Add `fields.{fieldName}.nested-key-null-strategy` configuration for `nested_update` function
[Feature] Support nested-key null handling strategies in FieldNestedUpdateAgg operator to control behavior when nested-key does not satisfy primary key semantics
Expected behavior when nested-key contains null values or does not satisfy primary key semantics:
In the following examples, `nested-key` is defined as `k0,k1`.
| **case** | **Input A** | **Input B** | **Current Behavior** | **Expected Behavior** |
| ---------------------------- | ------------------- | ---------------- | -------------------- | ----------------------------- |
| Default behavior (no config) | [(0, null, "A", 1)] | [(0, 1, "B", 2)] | Merge | Merge |
| MERGE strategy | [(0, null, "A", 1)] | [(0, 1, "B", 2)] | Merge | Merge |
| IGNORE strategy | [(0, null, "A", 1)] | [(0, 1, "B", 2)] | Merge ❌ | Ignore invalid nested-key row |
| ERROR strategy | [(0, null, "A", 1)] | [(0, 1, "B", 2)] | Merge ❌ | Throw exception |
---
### Behavior Definition
- `merge`
Merge rows even if nested-key does not satisfy primary key semantics.
This is equivalent to not configuring `nested-key-null-strategy`.
- `ignore`
Ignore rows whose nested-key does not satisfy primary key semantics.
- `error`
Throw an exception when nested-key does not satisfy primary key semantics.
---
### Tests
- org.apache.paimon.mergetree.compact.aggregate.FieldAggregatorTest#testFieldNestedUpdateAggWithNestedKeyNullStrategyArgumentCheck
- org.apache.paimon.mergetree.compact.aggregate.FieldAggregatorTest#testFieldNestedUpdateAggWithoutNestedKeyNullStrategy
- org.apache.paimon.mergetree.compact.aggregate.FieldAggregatorTest#testFieldNestedUpdateAggWhenNestedKeyNullUseMergeStrategy
- org.apache.paimon.mergetree.compact.aggregate.FieldAggregatorTest#testFieldNestedUpdateAggWhenNestedKeyNullUseIgnoreStrategy
- org.apache.paimon.mergetree.compact.aggregate.FieldAggregatorTest#testFieldNestedUpdateAggWhenNestedKeyNullUseThrowErrorStrategy
- org.apache.paimon.mergetree.compact.aggregate.FieldAggregatorTest#testFieldNestedUpdateAggWithCountLimitWhenNestedKeyNullUseMergeStrategy
- org.apache.paimon.mergetree.compact.aggregate.FieldAggregatorTest#testFieldNestedUpdateAggWithCountLimitWhenNestedKeyNullUseIgnoreStrategy
- org.apache.paimon.mergetree.compact.aggregate.FieldAggregatorTest#testFieldNestedUpdateAggWithCountLimitWhenNestedKeyNullUseThrowErrorStrategy
---
### API and Format
No Changes.
---
### Documentation
docs/docs/primary-key-table/merge-engine/aggregation.mdx
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "PyRSA (via GitHub)" <gi...@apache.org>.
PyRSA commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3490057411
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/FieldNestedUpdateAgg.java:
##########
@@ -243,6 +253,11 @@ private void addNestedRows(
InternalRow row = array.getRow(i, nestedFields);
BinaryRow key = keyProjection.apply(row).copy();
+
+ if (!applyNestedKeyNullStrategy(key)) {
Review Comment:
Thanks for catching this! I've updated FieldNestedUpdateAgg.retract() to apply nested-key-null-strategy consistently when both rebuilding the accumulator map and processing retract records. I also added corresponding test cases covering the IGNORE, and ERROR behaviors for the retract path.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3489341669
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/FieldNestedUpdateAgg.java:
##########
@@ -243,6 +253,11 @@ private void addNestedRows(
InternalRow row = array.getRow(i, nestedFields);
BinaryRow key = keyProjection.apply(row).copy();
+
+ if (!applyNestedKeyNullStrategy(key)) {
Review Comment:
This applies the new strategy only while merging rows in `agg()`. The `retract()` path still builds/removes keys with `keyProjection` directly, so with `nested-key-null-strategy=error` a retraction containing a null nested key will not throw, and with `ignore` it can still process a null-key retraction instead of ignoring it. Both `AggregateMergeFunction` and `PartialUpdateMergeFunction` route retract records through `FieldAggregator.retract(...)`, so the new option should be enforced there as well (or explicitly documented/tested as agg-only).
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3519469859
##########
paimon-core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java:
##########
@@ -795,6 +799,116 @@ public void testFieldNestedAppendAgg() {
.containsExactlyInAnyOrderElementsOf(Collections.singletonList(row(0, 1, "B")));
}
+ @Test
+ public void testFieldNestedUpdateAggFactoryWithSequenceFieldPrerequisite() {
+ DataType elementRowType =
+ DataTypes.ROW(
+ DataTypes.FIELD(0, "k0", DataTypes.INT()),
+ DataTypes.FIELD(1, "k1", DataTypes.INT()),
+ DataTypes.FIELD(2, "v", DataTypes.STRING()),
+ DataTypes.FIELD(3, "seq", DataTypes.INT()));
+
+ org.assertj.core.api.Assertions.assertThatThrownBy(
+ () ->
+ new FieldNestedUpdateAggFactory()
+ .create(
+ DataTypes.ARRAY(elementRowType),
+ CoreOptions.fromMap(
+ ImmutableMap.of(
+ "fields.fieldName.nested-sequence-field",
+ "seq")),
+ "fieldName"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Option 'fields.<field-name>.nested-sequence-field' requires "
+ + "'fields.<field-name>.nested-key' to be configured.");
+
+ FieldNestedUpdateAgg agg1 =
+ new FieldNestedUpdateAggFactory()
+ .create(
+ DataTypes.ARRAY(elementRowType),
+ CoreOptions.fromMap(new HashMap<>()),
+ "fieldName");
+ org.assertj.core.api.Assertions.assertThat(agg1).isNotNull();
+
+ FieldNestedUpdateAgg agg2 =
+ new FieldNestedUpdateAggFactory()
+ .create(
+ DataTypes.ARRAY(elementRowType),
+ CoreOptions.fromMap(
+ ImmutableMap.of("fields.filedName.nested-key", "k0,k1")),
Review Comment:
These positive factory cases use `fields.filedName...` while `create(..., "fieldName")` reads options under `fields.fieldName...`, so the configured `nested-key`, `nested-sequence-field`, and `nested-key-null-strategy` values are ignored. The assertions still pass with the default empty configuration, which means the tests do not cover the successful configured paths or lowercase enum parsing. Please fix the option key typo and assert behavior that depends on the configured options.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "PyRSA (via GitHub)" <gi...@apache.org>.
PyRSA commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3512619524
##########
paimon-api/src/main/java/org/apache/paimon/CoreOptions.java:
##########
@@ -2943,6 +2945,13 @@ public List<String> fieldNestedUpdateAggNestedKey(String fieldName) {
return Arrays.stream(keyString.split(",")).map(String::trim).collect(Collectors.toList());
}
+ public NestedKeyNullStrategy fieldNestedUpdateAggNestedKeyNullStrategy(String fieldName) {
+ return options.get(
+ key(FIELDS_PREFIX + "." + fieldName + "." + NESTED_KEY_NULL_STRATEGY)
+ .enumType(NestedKeyNullStrategy.class)
+ .noDefaultValue());
Review Comment:
Hi @JingsongLi, I’ve already replied above regarding the default-value approach. When you have a chance, could you please take another look? Thanks!
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #8374:
URL: https://github.com/apache/paimon/pull/8374#issuecomment-4877226653
+1
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "PyRSA (via GitHub)" <gi...@apache.org>.
PyRSA commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3504452326
##########
paimon-api/src/main/java/org/apache/paimon/CoreOptions.java:
##########
@@ -2943,6 +2945,13 @@ public List<String> fieldNestedUpdateAggNestedKey(String fieldName) {
return Arrays.stream(keyString.split(",")).map(String::trim).collect(Collectors.toList());
}
+ public NestedKeyNullStrategy fieldNestedUpdateAggNestedKeyNullStrategy(String fieldName) {
+ return options.get(
+ key(FIELDS_PREFIX + "." + fieldName + "." + NESTED_KEY_NULL_STRATEGY)
+ .enumType(NestedKeyNullStrategy.class)
+ .noDefaultValue());
Review Comment:
> Why not create a default value?
Thanks for the suggestion! I think using a default value also makes sense.
The only issue is that once `MERGE` becomes the default, `FieldNestedUpdateAgg` can no longer distinguish between an unspecified option and an explicitly configured `MERGE` in its constructor, so the dependency validation cannot be performed there anymore.
Instead, I'm thinking of moving the validation to `FieldNestedUpdateAggFactory.create()`, where the dependency can be validated before creating the aggregator by checking whether `nested-key-null-strategy` was explicitly configured. This still provides `fail-fast` validation during table creation and avoids allowing table options that have no effect.
If this approach sounds reasonable, I'll update the implementation accordingly.
```java
boolean strategyConfigured =
options.toConfiguration()
.containsKey(...);
checkArgument(
!strategyConfigured || !nestedKey.isEmpty(),
...);
```
Something along these lines.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "PyRSA (via GitHub)" <gi...@apache.org>.
PyRSA commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3520149808
##########
paimon-core/src/test/java/org/apache/paimon/mergetree/compact/aggregate/FieldAggregatorTest.java:
##########
@@ -795,6 +799,116 @@ public void testFieldNestedAppendAgg() {
.containsExactlyInAnyOrderElementsOf(Collections.singletonList(row(0, 1, "B")));
}
+ @Test
+ public void testFieldNestedUpdateAggFactoryWithSequenceFieldPrerequisite() {
+ DataType elementRowType =
+ DataTypes.ROW(
+ DataTypes.FIELD(0, "k0", DataTypes.INT()),
+ DataTypes.FIELD(1, "k1", DataTypes.INT()),
+ DataTypes.FIELD(2, "v", DataTypes.STRING()),
+ DataTypes.FIELD(3, "seq", DataTypes.INT()));
+
+ org.assertj.core.api.Assertions.assertThatThrownBy(
+ () ->
+ new FieldNestedUpdateAggFactory()
+ .create(
+ DataTypes.ARRAY(elementRowType),
+ CoreOptions.fromMap(
+ ImmutableMap.of(
+ "fields.fieldName.nested-sequence-field",
+ "seq")),
+ "fieldName"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Option 'fields.<field-name>.nested-sequence-field' requires "
+ + "'fields.<field-name>.nested-key' to be configured.");
+
+ FieldNestedUpdateAgg agg1 =
+ new FieldNestedUpdateAggFactory()
+ .create(
+ DataTypes.ARRAY(elementRowType),
+ CoreOptions.fromMap(new HashMap<>()),
+ "fieldName");
+ org.assertj.core.api.Assertions.assertThat(agg1).isNotNull();
+
+ FieldNestedUpdateAgg agg2 =
+ new FieldNestedUpdateAggFactory()
+ .create(
+ DataTypes.ARRAY(elementRowType),
+ CoreOptions.fromMap(
+ ImmutableMap.of("fields.filedName.nested-key", "k0,k1")),
Review Comment:
Thanks for pointing this out! Fixed the option key typo and updated the tests to cover the configured paths and option behaviors.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "PyRSA (via GitHub)" <gi...@apache.org>.
PyRSA commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3503741702
##########
paimon-api/src/main/java/org/apache/paimon/CoreOptions.java:
##########
@@ -2943,6 +2945,13 @@ public List<String> fieldNestedUpdateAggNestedKey(String fieldName) {
return Arrays.stream(keyString.split(",")).map(String::trim).collect(Collectors.toList());
}
+ public NestedKeyNullStrategy fieldNestedUpdateAggNestedKeyNullStrategy(String fieldName) {
+ return options.get(
+ key(FIELDS_PREFIX + "." + fieldName + "." + NESTED_KEY_NULL_STRATEGY)
+ .enumType(NestedKeyNullStrategy.class)
+ .noDefaultValue());
Review Comment:
Thanks for the suggestion! I originally avoided a default value because I wanted to distinguish between an unspecified option and an explicitly configured `MERGE`, so that I could validate that `nested-key-null-strategy` is only configured when `nested-key` is present. With a default value, these two cases become indistinguishable. The default behavior is still preserved by falling back to `MERGE` in `FieldNestedUpdateAgg`.
I'm happy to adjust this if you think consistency with other options is more important.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "PyRSA (via GitHub)" <gi...@apache.org>.
PyRSA commented on PR #8374:
URL: https://github.com/apache/paimon/pull/8374#issuecomment-4873836845
Updated based on the review comments.
Changes:
- Added `MERGE` as the default value for `nested-key-null-strategy`.
- Moved the dependency validation for `nested-key-null-strategy` and `nested-sequence-field` to `FieldNestedUpdateAggFactory` to ensure both options require `nested-key`.
Test:
- mvn test -pl paimon-core -Dtest=FieldAggregatorTest
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "PyRSA (via GitHub)" <gi...@apache.org>.
PyRSA commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3490057411
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/FieldNestedUpdateAgg.java:
##########
@@ -243,6 +253,11 @@ private void addNestedRows(
InternalRow row = array.getRow(i, nestedFields);
BinaryRow key = keyProjection.apply(row).copy();
+
+ if (!applyNestedKeyNullStrategy(key)) {
Review Comment:
Thanks for catching this! I've updated FieldNestedUpdateAgg.retract() to apply nested-key-null-strategy consistently when both rebuilding the accumulator map and processing retract records. I also added corresponding test cases covering the IGNORE and ERROR behaviors for the retract path.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8374:
URL: https://github.com/apache/paimon/pull/8374#discussion_r3503563171
##########
paimon-api/src/main/java/org/apache/paimon/CoreOptions.java:
##########
@@ -2943,6 +2945,13 @@ public List<String> fieldNestedUpdateAggNestedKey(String fieldName) {
return Arrays.stream(keyString.split(",")).map(String::trim).collect(Collectors.toList());
}
+ public NestedKeyNullStrategy fieldNestedUpdateAggNestedKeyNullStrategy(String fieldName) {
+ return options.get(
+ key(FIELDS_PREFIX + "." + fieldName + "." + NESTED_KEY_NULL_STRATEGY)
+ .enumType(NestedKeyNullStrategy.class)
+ .noDefaultValue());
Review Comment:
Why not create a default value?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support nested-key-null-strategy in FieldNestedUpdateAgg operator [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #8374:
URL: https://github.com/apache/paimon/pull/8374
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org