JingsongLi commented on code in PR #10665:
URL: https://github.com/apache/paimon/pull/10665#discussion_r4345790126
##########
paimon-core/src/main/java/org/apache/paimon/index/DynamicBucketIndexMaintainer.java:
##########
@@ -72,7 +80,11 @@ public void notifyNewRecord(KeyValue record) {
if (!(key instanceof BinaryRow)) {
throw new IllegalArgumentException("Unsupported key type: " +
key.getClass());
}
- boolean changed = hashcode.add(key.hashCode());
+ // only DELETE removes a key: -U is the first half of an update, and
+U / +I write the key
+ boolean changed =
+ removeDeletedKeys && record.valueKind() == RowKind.DELETE
+ ? hashcode.remove(key.hashCode())
Review Comment:
[P1] Keep routing for live keys that share a deleted key hash
The persisted set contains 32-bit hashes, not distinct primary keys.
Removing a hash on DELETE also removes the routing of any other live key with
that hash. I reproduced this with BIGINT keys 187054415718944 and
562722320285643 (both hash to -1755412784): write both into bucket 1, delete
the first plus a filler in bucket 0, commit and reopen the assigner/writer,
then update the surviving key. With this option on, the survivor moves to
bucket 0 and the table returns both its stale value 20 and new value 99. The
option-off control retains bucket 1 and returns only 99. This is primary-key
data corruption beyond the documented approximate row count. Preserve
collision-safe routing across deletes/recovery, or use sufficient key/refcount
information to prove no live key owns the hash before removing it.
##########
paimon-core/src/main/java/org/apache/paimon/table/sink/RowPartitionKeyExtractor.java:
##########
@@ -48,4 +49,10 @@ public BinaryRow partition(InternalRow record) {
public BinaryRow trimmedPrimaryKey(InternalRow record) {
return trimmedPrimaryKeyProjection.apply(record);
}
+
+ /** Only {@link RowKind#DELETE} deletes a key: {@code -U} is the first
half of an update. */
+ @Override
+ public boolean isDelete(InternalRow record) {
+ return record.getRowKind() == RowKind.DELETE;
Review Comment:
[P2] Use the writer effective row kind for slot accounting
`rowkind.field` is allowed with this option, but TableWriteImpl derives the
actual kind using RowKindGenerator after the assigner, while this method only
reads the incoming row metadata. With default local merge disabled, `+I(id=1,
op='-D')` is deleted by the real writer yet is not recognized here, so a later
key opens bucket 2 instead of reusing bucket 0. Conversely a metadata DELETE
with op='+I' frees a still-live slot. This was reproduced with current branch
extractor/assigner/writer calls and committed readback. Apply the same
effective-kind logic as the writer, or reject this configuration until it is
supported.
--
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]
