zclllyybb commented on issue #65808: URL: https://github.com/apache/doris/issues/65808#issuecomment-5019872700
Breakwater-GitHub-Analysis-Slot: slot_4ee3733d18a7 This content is generated by AI for reference only. Initial code-level analysis: The report looks valid against current `upstream/master` (`52e154530ff72b53d205f77d845a3072e44ea5f1`). I do not see evidence that this depends on IVM or materialized-view refresh; the relevant path is direct Table Stream -> row binlog -> `BlockReader` MIN_DELTA. Mechanism from the current code: 1. `MIN_DELTA` table streams on MOW UNIQUE KEY tables require row binlog with historical values (`OlapTable.checkAsTableStreamBaseTable()`). 2. With `binlog.need_historical_value=true`, `Tablet::_init_context_common_fields()` enables BEFORE-image writing, so `RowBinlogSegmentWriter::append_block()` calls `PrimaryKeyModelRowRetriever::retrieve_historical_row()`. 3. In `historical_row_retriever.cpp`, once `lookup_row_key()` finds the same key, a non-delete incoming row is classified as `ROW_BINLOG_UPDATE` via `_operators.emplace_back(... ROW_BINLOG_UPDATE)`. That branch does not check whether the matched historical physical row is a MOW delete-sign row. 4. `BaseTablet::lookup_row_key()` does filter rows already removed by the delete bitmap, but a MOW delete row itself can remain as the latest physical key-index entry with `__DORIS_DELETE_SIGN__=1`. From the logical table view the key is absent, but the historical lookup can still return that physical tombstone as a key hit. 5. `BlockReader::_min_delta_next_block()` then sees a one-row same-key group whose first and last stored op are both `ROW_BINLOG_UPDATE`. The MIN_DELTA transition matrix maps `UPDATE -> UPDATE` to `UPDATE_BEFORE_AFTER`, and FE normalization displays that as `UPDATE_BEFORE` / `UPDATE_AFTER`. So the suspicious boundary is not stream offset advancement and not FE change-type formatting. The bug is that MIN_DELTA is treating a reinsert whose historical match is a delete tombstone as an update from a visible row. After the prior DELETE has already been consumed by the stream offset, the visible start state for that key is absent, so the next write should be exposed as `APPEND`. This is adjacent to #65800 but not the same failure. #65800 is about the payload chosen when `UPDATE -> DELETE` folds into a DELETE. This issue is about reinsert-after-consumed-delete being encoded/read as an UPDATE pair. Suggested fix direction: 1. Distinguish "matched physical tombstone" from "matched visible row" in the row-binlog historical lookup path. For this case, either emit `ROW_BINLOG_APPEND` at write time or persist enough tombstone-before-state metadata for MIN_DELTA to convert this first-window UPDATE into `APPEND`. 2. Avoid a reader-only heuristic based on NULL BEFORE values. `RowBinlogSegmentWriter::_fill_before_columns()` currently writes BEFORE columns only for visible non-key value columns, not the hidden delete-sign column, and nullable value columns make NULL an unsafe proxy for "deleted row". 3. Review raw row-binlog DETAIL compatibility separately. Current row-binlog regression comments document the physical behavior where insert-after-delete can be recorded as UPDATE. If that raw DETAIL behavior must remain, scope the fix to Table Stream / MIN_DELTA semantics rather than accidentally changing raw binlog consumers. Suggested regression coverage: 1. Add the exact case from this issue to `regression-test/suites/table_stream_p0/test_min_delta_stream.groovy`: insert, delete, consume the stream through `INSERT INTO target SELECT * FROM stream`, then reinsert the same key and assert a single `APPEND` row. 2. Add a variant with nullable value columns and a different reinserted value, so the fix is not tied to the displayed `NULL -> value` shape. 3. Keep adjacent coverage for insert-delete-insert within one unconsumed window, pure DELETE, and UPDATE-then-DELETE, because all of them share the same MIN_DELTA grouping code. Missing information: - The exact Doris build or commit where the reporter reproduced this. - Whether raw `DETAIL` / `@incr(..., "incrementType" = "DETAIL")` should continue exposing the physical tombstone-based UPDATE for this case, or whether the desired logical semantics should also change at the row-binlog layer. That decision affects the cleanest fix boundary. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
