Repository navigation
Conversation
With aggregation.remove-record-on-delete enabled, a delete followed by new records for the same key kept aggregating on top of the deleted record's values, because the accumulator row was replaced by the delete's own image and the field aggregators were not reset. Reset the accumulator row and every field aggregator when the first record after such a delete arrives.
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED. Implementation: FINDINGS.
Restarting aggregation after a removing delete fixes a real CDC scenario. Normal JDK 8 reactor verification passes all 7 targeted aggregate/function and table tests. An additional real-table checkpoint-grouping case below still retains the deleted values; the same configuration with DELETE committed separately passes. Temporary reviewer changes were removed.
Current upstream CI also has a JDK 11 core failure in SortBufferWriteBufferTestBase$WithLookupFunctionTest.testCreateReader (expected sequence 6, actual 3). I have not attributed that separate failure to this diff.
| if (currentDeleteRow) { | ||
| // The record was removed by the previous delete, so later records start a new | ||
| // aggregation instead of accumulating onto the deleted record's values. | ||
| currentDeleteRow = false; |
There was a problem hiding this comment.
[P1] Preserve the delete barrier through buffer preaggregation
Resetting this accumulator is insufficient when DELETE and the first replacement INSERT are in the same checkpoint. MergeTreeWriter.flushWriteBuffer invokes this function before persisting that buffer, so getResult() emits an ordinary INSERT and loses the fact that older aggregates must be discarded. I reproduced this by adapting the new table test with supported write-only=true: commit (pt=1,a=1,b=10,c=1), then buffer its DELETE and (1,1,3,7) together before the next prepareCommit, then commit (1,1,4,8). The actual table read returns +I[1,1,17,1] instead of +I[1,1,7,7]; putting DELETE in its own commit passes the identical control. Thus the claimed delete/reinsert fix still depends on checkpoint grouping, and old sum/first_value values survive. Please preserve reset/tombstone semantics through buffer reduction and partial compaction, or avoid collapsing this boundary until older versions participate, and add a same-checkpoint persistence regression.
Purpose
With
aggregation.remove-record-on-deleteenabled, a delete followed by new records for the same key kept aggregating on top of the deleted record's values, because the accumulator row was replaced by the delete's own image and the field aggregators were not reset. Reset the accumulator row and every field aggregator when the first record after such a delete arrives.Tests
Add UT