You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "jackylee-ch (via GitHub)" <gi...@apache.org> on 2026/09/24 11:53:09 UTC
[PR] [python] Support ignore-delete on partial-update tables [paimon]
jackylee-ch opened a new pull request, #10161:
URL: https://github.com/apache/paimon/pull/10161
### Purpose
A `partial-update` table with `ignore-delete` set — the documented first choice for handling deletes in partial-update — was rejected by pypaimon. Both the read-side guard (`TableRead` → `merge_engine_support.check_supported`) and the shared merge dispatch listed `ignore-delete` / `partial-update.ignore-delete` as unsupported, so such a table could be neither read nor written (`NotImplementedError`).
It is now handled the same way `FirstRowMergeFunction` already does: `PartialUpdateMergeFunction` takes an `ignore_delete` flag and skips DELETE / UPDATE_BEFORE rows instead of raising, and `build_merge_function` derives the flag from the table options. Both unsupported lists drop the two `ignore-delete` keys; `sequence-group`, per-field aggregator override and `partial-update.remove-record-on-*` remain unsupported and still raise. Without `ignore-delete`, a retract row still raises, so plain partial-update is unchanged.
### Tests
- `test_partial_update_merge_function`: retract rows are skipped, and a retract-only group yields nothing, when `ignore_delete` is set.
- `test_merge_engine_dispatch`: the flag is wired from options; `remove-record-on-delete` is still flagged.
- `test_partial_update_e2e`: an `ignore-delete` table is accepted and still merges last-non-null (the write path cannot emit a DELETE row without `rowkind.field`, so retract-skipping is covered as a unit).
Written with Claude Code; verification is mine.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi closed pull request #10161: [python] Support ignore-delete on partial-update tables
URL: https://github.com/apache/paimon/pull/10161
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on code in PR #10161:
URL: https://github.com/apache/paimon/pull/10161#discussion_r4180202992
##########
paimon-python/pypaimon/read/reader/partial_update_merge_function.py:
##########
@@ -92,16 +100,19 @@ def reset(self) -> None:
def add(self, kv: KeyValue) -> None:
row_kind_byte = kv.value_row_kind_byte
if not RowKind.is_add_byte(row_kind_byte):
- # DELETE / UPDATE_BEFORE require ignore-delete or
- # partial-update.remove-record-on-delete to be enabled,
- # and neither option is implemented in pypaimon yet, so
- # refuse the row rather than silently swallow it.
+ if self._ignore_delete:
+ # ignore-delete: drop the retract row and keep merging the
+ # rest of the group, as Java PartialUpdateMergeFunction does.
+ return
Review Comment:
Fixed in 1bc644521. add() now seeds the accumulator from the first retract before the ignore-delete short-circuit, mirroring Java initRow()/notNullColumnFilled, so a persisted DELETE's value survives a later partial INSERT that leaves the field unset. A retract-only group still yields no row: a new _meet_insert flag gates get_result(), matching Java's dropped RowKind.DELETE when !meetInsert. Added unit tests plus an end-to-end test over two overlapping level-0 KV files (retract-first then partial add) reading value=100 where the old code read null; reverting the seed reproduces the null.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #10161:
URL: https://github.com/apache/paimon/pull/10161#issuecomment-5996393657
Java is included solely for compatibility with version 0.7.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on PR #10161:
URL: https://github.com/apache/paimon/pull/10161#issuecomment-5911596708
Added the end-to-end read you asked for (`test_python_read_of_persisted_retractions`) and rebased onto master. Since the Python writer only emits INSERT, the fixture is persisted via the native writer with `rowkind.field`, then read back with `read.native.enabled=false` so pypaimon's SortMergeReader / merge dispatch handles the persisted `-D` / `-U` rows: the retraction on an existing key is skipped (key 1 keeps its insert) and a retract-only key (key 2) is absent, across first-row / partial-update / aggregation.
Verified locally against `paimon-rust` main; reverting the merge-engine change makes the partial-update case raise `NotImplementedError`, so it's non-vacuous. It runs in the Native CI job, which the earlier INSERT-only test could not reach.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #10161:
URL: https://github.com/apache/paimon/pull/10161#issuecomment-5832070923
Reviewed the current head across option parsing, read/write merge dispatch, partial-update row-kind handling, and split planning. This has a real cross-engine use case: a Python reader should be able to consume an `ignore-delete` partial-update table containing Java-written retract rows. The implementation skips DELETE/UPDATE_BEFORE in `PartialUpdateMergeFunction`, keeps the default non-ignore path rejecting them, and now uses the same canonical boolean parser as `CoreOptions`. I also checked that the primary-key scan's raw-convertible path requires no delete rows, so a file with recorded retracts should enter the merge path.
Local verification: 71 partial-update, merge-function, dispatch, and adjacent aggregation tests passed; `git diff --check` passed. The head's Python version matrix and Native CI are green. The earlier `ignore-delete=yes` parser discrepancy is addressed.
**Production verification gap:** the new real-table test writes only INSERT rows, and the DELETE/UPDATE_BEFORE behavior is tested only by calling the merge function directly. Python's current writer always emits INSERT row kinds, so that test does not exercise the actual use case through manifest planning, file reading, and merge dispatch. Please add a Java-written (or equivalent persisted-file) DELETE/UPDATE_BEFORE fixture with `ignore-delete=true`, then assert the Python table read retains the correct merged row; also cover a retract-only key. I would hold production merge until this end-to-end path is verified.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #10161:
URL: https://github.com/apache/paimon/pull/10161#issuecomment-5952248525
Reviewed head 706417553f. The partial-update implementation has end-to-end value, and I did not find an introduced read-semantic defect. I verified actual Java-produced Parquet and Avro fixtures containing both DELETE and UPDATE_BEFORE: each manifest records two retract rows, Python uses the merge path, retains the earlier insert, drops a retract-only key, and merges later non-null fields exactly like Java. Full reads, projections, and canonical Python boolean spellings passed. Disabling only the new retract-skipping branch makes these fixtures fail.
**[P2] The new persisted-retraction test still removes the retracts on the write side** (`native_write_capabilities_test.py:264–276`). It writes with `ignore-delete=true`. The exact Native CI dependency (`paimon-rust` 6267e9c0) generates row kinds and then applies `RowKindFilter` before persisting; DELETE/UPDATE_BEFORE are discarded. Consequently the Python reader only sees the insert, and the assertion does not exercise the new `PartialUpdateMergeFunction.add` branch. Reverting the option-support guard and getting `NotImplementedError` proves table admission, but does not prove retract handling. The same physical filtering also explains why the aggregation variant passes.
Please persist a legacy/equivalent fixture whose physical `_VALUE_KIND` contains 1 and 3, assert positive manifest `deleteRowCount`, and read it with Python. Include the existing-key and retract-only-key cases, and verify that removing only the new skip branch fails the test. Modern Java/native writers also filter these rows, so simply switching writer languages while retaining `ignore-delete=true` will not create that fixture.
Validation: 71 relevant Python tests and 47 Java partial-update tests passed; 25 native-extension cases were skipped locally, while the exact-head Native CI is green. The separate persisted-file cross-engine checks above passed; configured Python lint, Python 3.6 grammar, and diff checks passed.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on code in PR #10161:
URL: https://github.com/apache/paimon/pull/10161#discussion_r4111426340
##########
paimon-python/pypaimon/common/merge_engine_dispatch.py:
##########
@@ -110,6 +110,7 @@ def build_merge_function(
value_field_names=(
list(value_field_names)
if value_field_names is not None else None),
+ ignore_delete=_ignore_delete_from_options(raw_options),
Review Comment:
Good catch — fixed in e4db05d54. `_option_is_truthy` now delegates to `OptionsUtils.convert_to_boolean` (the parser behind `CoreOptions.ignore_delete()`), so `ignore-delete=yes` / `1` / `on` are honored consistently instead of being silently downgraded to `False`. The dispatch tests were updated to cover those spellings.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on PR #10161:
URL: https://github.com/apache/paimon/pull/10161#issuecomment-5967554182
You're right — the old test wrote with `ignore-delete=true`, so the native `RowKindFilter` (and the Python writer's `_VALUE_KIND=0` hardcode + merge buffer) stripped the retracts before persist; the file was all-INSERT and the skip branch never ran.
Replaced it with a hand-crafted fixture (`partial_update_persisted_retraction_test.py`): a level-0 KV data file whose physical `_VALUE_KIND` carries DELETE(3) and UPDATE_BEFORE(1), described by a `DataFileMeta` with `delete_row_count=2`, committed through the normal commit path (real manifest + snapshot). The positive `delete_row_count` is load-bearing: it makes the split non-raw-convertible so the read routes through `SortMergeReaderWithMinHeap` → `PartialUpdateMergeFunction` rather than `RawFileSplitRead`. Covers both the existing key (1: INSERT then DELETE → keeps the insert) and the retract-only key (2: UPDATE_BEFORE → absent).
Non-vacuity: disabling only the skip branch makes the first retract raise `NotImplementedError` (proving the retract actually reaches the merge function). Pinned to the Python lane so native CI lanes don't force it onto the native backends. Pushed in c903f8423.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on PR #10161:
URL: https://github.com/apache/paimon/pull/10161#issuecomment-5968850125
Rebased onto `cd34f029d` and force-pushed (no content change to the fix above).
Heads-up on an unrelated upstream breakage I hit while getting this green: current `master` does not test-compile `paimon-core`. `GlobalIndexer.create` (paimon-common) now takes `List<DataField>`, but `IndexQuerySplitTest.java:921` added in #10201 still passes a single `DataField`:
```
IndexQuerySplitTest.java:[921,79] incompatible types:
org.apache.paimon.types.DataField cannot be converted to java.util.List<DataField>
```
Because the Python CI lane test-compiles `paimon-core`, every PR rebuilt on current master fails before any Python test runs — which is what reddened this PR. I verified it reproduces on a clean rebase onto `master` HEAD and is gone at `cd34f029d` (the commit just before #10201). This PR is pure Python, so I based it there to get a representative green run; it stays `MERGEABLE` and can be fast-forwarded once master is fixed. Local: 57 partial-update tests pass, `paimon-core` test-compiles, flake8 clean.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on PR #10161:
URL: https://github.com/apache/paimon/pull/10161#issuecomment-5826602960
Thanks @Akash3121, fixed in 7a829d960. `_option_is_truthy` now delegates to `OptionsUtils.convert_to_boolean` (`CoreOptions.ignore_delete()`'s parser) instead of a second strict parser, so `true`/`1`/`yes`/`on` are true and `false`/`0`/`no`/`off` false; an unrecognized spelling raises. This covers both the ignore-delete wiring (`ignore-delete=yes` now builds `ignore_delete=True`, skipping retracts instead of raising) and the unsupported-option guard. Tests exercise every canonical spelling for both, plus that a canonical-true `remove-record-on-delete` stays flagged. PTAL.
--
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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #10161:
URL: https://github.com/apache/paimon/pull/10161#discussion_r4179991406
##########
paimon-python/pypaimon/read/reader/partial_update_merge_function.py:
##########
@@ -92,16 +100,19 @@ def reset(self) -> None:
def add(self, kv: KeyValue) -> None:
row_kind_byte = kv.value_row_kind_byte
if not RowKind.is_add_byte(row_kind_byte):
- # DELETE / UPDATE_BEFORE require ignore-delete or
- # partial-update.remove-record-on-delete to be enabled,
- # and neither option is implemented in pypaimon yet, so
- # refuse the row rather than silently swallow it.
+ if self._ignore_delete:
+ # ignore-delete: drop the retract row and keep merging the
+ # rest of the group, as Java PartialUpdateMergeFunction does.
+ return
Review Comment:
[P2] Preserve Java initialization when the first legacy record is a retract
Java PartialUpdateMergeFunction initializes the accumulator from the first retract before applying ignore-delete (add lines 155–164 / initRow), specifically for retracts persisted by 0.7-era writers. This branch returns without that initialization. If a legacy DELETE has value=100 and a later INSERT/UPDATE_AFTER leaves that nullable field unset, Java preserves 100 but Python now returns null. I reproduced this through two valid overlapping level-0 KV files (one record per key in each file), normal CommitMessage/manifest/snapshot commit, and actual table reads: both engines read the same committed table; Java returns {id:1,value:100}, this head returns {id:1,value:null}. A retract-only key remains absent in both. A source-free Python control adding only first-retract accumulator initialization restores the Java result. Please mirror Java initialization while still keeping retract-only groups absent, and cover retract-first plus later partial-add across separate overlapping files.
The new fixture covers add-then-retract and retract-only, which cannot reveal the lost field 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] [python] Support ignore-delete on partial-update tables [paimon]
Posted by "Akash3121 (via GitHub)" <gi...@apache.org>.
Akash3121 commented on code in PR #10161:
URL: https://github.com/apache/paimon/pull/10161#discussion_r4095842086
##########
paimon-python/pypaimon/common/merge_engine_dispatch.py:
##########
@@ -110,6 +110,7 @@ def build_merge_function(
value_field_names=(
list(value_field_names)
if value_field_names is not None else None),
+ ignore_delete=_ignore_delete_from_options(raw_options),
Review Comment:
` _ignore_delete_from_options` ultimately uses `_option_is_truthy` , which recognizes only the string `"true"` . That conflicts with PyPaimon's canonical `OptionsUtils.convert_to_boolean` / `CoreOptions.ignore_delete()` , where `"1"` , `"yes"` , and `"on"` are also true. Consequently, a valid table option such as `ignore-delete=yes` now passes the unsupported-option guard but constructs this merge function with ignore_delete=False ; the first retract then raises `NotImplementedError` . Please resolve this through `CoreOptions.ignore_delete()` or `OptionsUtils.convert_to_boolean` rather than maintaining a second boolean parser, and test every canonical true/false spelling.
--
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