zhuxiangyi opened a new pull request, #9356:
URL: https://github.com/apache/paimon/pull/9356
### Purpose
`MERGE INTO` with a `DELETE` clause silently corrupts a `partial-update`
primary key table: the
statement succeeds and commits a snapshot, but the table becomes unreadable
afterwards.
```sql
CREATE TABLE t (id INT, a INT, b INT) TBLPROPERTIES (
'primary-key' = 'id', 'bucket' = '1', 'merge-engine' = 'partial-update');
INSERT INTO t VALUES (1, 10, 100), (2, 20, 200);
MERGE INTO t USING s ON t.id = s.id WHEN MATCHED THEN DELETE; -- succeeds,
commits a snapshot
SELECT * FROM t; -- throws
```
```
java.lang.IllegalArgumentException: By default, Partial update can not
accept delete records,
you can choose one of the following solutions:
1. Configure 'ignore-delete' to ignore delete records.
2. Configure 'partial-update.remove-record-on-delete' to remove the whole
row ...
at
org.apache.paimon.mergetree.compact.PartialUpdateMergeFunction.add(PartialUpdateMergeFunction.java:191)
```
The write side never notices; the failure only surfaces at read time, and
only for queries that
actually have to merge the affected key — so the table looks partly healthy,
which makes this hard
to diagnose.
**Root cause.** `PrimaryKeyTableUtils#validatePKUpsertDeletable` already
encodes which merge engines
can consume delete records. It is called by Flink's SQL `DELETE`
(`SupportsRowLevelOperationFlinkTableSink#applyRowLevelDelete` /
`#applyDeleteFilters`) and by
Spark's `DeleteFromPaimonTableCommand`, but **not** by Spark's `MERGE INTO`,
which emits
`RowKind.DELETE` records unconditionally. `RowLevelOp.MergeInto` only checks
that the merge engine
is `deduplicate` or `partial-update`, and `partial-update` is exactly the
one that cannot accept
delete records by default.
**Fix.** Run the same validation in `PaimonMergeInto` when the statement has
a `DELETE` clause and
the target is a primary key table, so all three paths reject the same tables
with the same message.
The check lives in the analysis rule rather than in the command because both
V2 row-level paths
(`SparkTable#supportsV2CopyOnWriteOps` / `#supportsV2DeltaOps`) require
`primaryKeys().isEmpty` —
primary key tables therefore always go through `PaimonMergeInto` on every
supported Spark version,
making it the single choke point. Failing during analysis also means nothing
has been written yet.
**Behavior change.** Such statements now fail with the existing, actionable
message instead of
committing a corrupt snapshot:
> Merge engine partial-update doesn't support batch delete by default. To
support batch delete,
> please set partial-update.remove-record-on-delete to true when there is no
sequence.field or set
> partial-update.remove-record-on-sequence-group.
Scope is narrow — only *primary key table* + *statement contains a DELETE
clause* + *merge engine
cannot accept delete records*:
- `aggregate` / `first-row` are already rejected earlier by
`RowLevelOp.MergeInto`.
- `deduplicate` passes the validation.
- `partial-update` passes once `partial-update.remove-record-on-delete` or
`partial-update.remove-record-on-sequence-group` is set.
- Merges without a `DELETE` clause, and append-only tables, are unaffected.
This also stops the related silent no-op: with `ignore-delete = true`, a
`MERGE ... THEN DELETE`
used to report success while keeping the row, because
`PartialUpdateMergeFunction#add` discards the
delete record at read time. It is now rejected instead.
### Tests
New in `MergeIntoPrimaryKeyTableTest` (`MergeIntoTableTestBase.scala`):
- `reject DELETE clause when the merge engine can not accept deletes` —
covers
`WHEN MATCHED THEN DELETE`, `WHEN MATCHED AND <cond> THEN DELETE` and
`WHEN NOT MATCHED BY SOURCE THEN DELETE`, and asserts the target is left
untouched.
- `allow DELETE clause once the merge engine can accept deletes` —
`partial-update` with
`partial-update.remove-record-on-delete = true` still deletes.
- `a delete-free merge on partial-update is unaffected`.
Existing suites, all green with no changes:
- `MergeInto{PrimaryKeyBucketed,PrimaryKeyNonBucket}TableTest` and their
`V2` variants —
`Tests: succeeded 244, failed 0`
- `MergeIntoAppend{Bucketed,NonBucketed}TableTest` + `V2` variants,
`DeleteFromTableTest` /
`V2DeleteFromTableTest`, `UpdateTableTest` / `V2UpdateTableTest` —
`Tests: succeeded 352, failed 0`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
