Doris-Breakwater commented on issue #68116: URL: https://github.com/apache/doris/issues/68116#issuecomment-5709596393
Initial triage: **confirmed by code inspection; high-severity correctness and availability issue**. The report contains strong deterministic evidence for silent wrong results and a BE crash. I did not independently run the Docker reproduction, but the relevant code at both `ad35a140c7f` (4.1.4) and `c4dee4bd5e82` (reported master) has the ownership violation needed to produce the reported symptoms. ### Code-backed mechanism * `Value::set_value()` in `be/src/exprs/aggregate/aggregate_function_reader_first_last.h` saves only an `IColumn*` plus a row offset. `FirstLastData::insert_result_into()` later reads that pair with `insert_from(*_ptr, _offset)`. `FirstLastData` is used by `first_value`, and `NthValueData` derives from it. * The analytic sink appends arguments into the long-lived `_agg_input_columns`. Before accepting another block, `_remove_unused_rows()` chooses a 256-block prefix and calls `erase(0, remove_rows)` on those same columns. It rebases `_current_row_position`, partition/order boundaries, and queued ends, but it neither copies retained aggregate values nor rebases/invalidates offsets stored inside aggregate state. * The column object itself remains alive; the defect is more specifically a stale or out-of-range saved row offset after the column contents are compacted. For fixed-width columns, `erase()` memmoves later values down, explaining deterministic but incorrect integers. For `ColumnStr`, `erase()` compacts both chars and offsets; a later out-of-range `insert_from()` computes a bogus length/offset pair, which is consistent with the 4 GiB length error and the `memcpy` crash. * `first_value` can keep that reference because `_has_value` makes subsequent calls return without replacing it. The eviction guard added by #67274 protects rows required by the next ROWS-frame calculation, but it does not account for references already retained inside an aggregate state. * The production `block_num = 256` and the `block_num + 1` guard in `_remove_unused_rows()` directly explain why the transition is tied to the number of physical input blocks. Increasing `batch_size` only moves the boundary and is not a correctness guarantee, especially for fragmented input. ### Missing/useful follow-up information The body says a complete reproduction script is attached, but there is currently no attachment or comment. Please upload the deterministic script, including the full DDL (the `nth_value` example uses column `b`, which is absent from the abbreviated two-column DDL), data generator/seed, and load procedure. A complete BE fatal-log excerpt with query ID would also be useful for validating the eventual fix, but this is not a blocker for accepting the bug: the supplied versions, measurements, error, and stack are already sufficient for triage. ### Recommended next steps 1. Add a focused BE regression in `be/test/exec/operator/analytic_sink_operator_test.cpp`. Under `BE_TEST`, the eviction threshold is already one block, so the test can exercise `first_value`/`nth_value` across eviction with only a few blocks. Cover at least fixed-width and string inputs, nullable/ignore-null variants where applicable, and assert that eviction actually occurred. 2. Make the retained window result own/copy its value, or introduce an explicit aggregate-state/eviction contract that preserves and rebases every referenced row. Merely changing the 256-block threshold is not a fix. Copying the selected value is the safer direction for `first_value`/`nth_value`; retaining the entire referenced prefix could defeat the memory-release purpose for unbounded frames. 3. Audit `last_value`, `lead`, and `lag` as well: their window state uses the same `Value`/`BaseValue` pointer-plus-offset representation, although this report does not establish that every one of those functions is reachable with an unsafe lifetime. 4. Validate the fix with the reporter's three oracles, an ASAN build for the string case, default `batch_size` with a partition exceeding the physical-block threshold, and both master and branch-4.1. A 4.1 backport is warranted because 4.1.4 is affected. 5. Until a fix is available, avoid these window shapes on partitions that may span many physical scan blocks. Raising `batch_size` may reduce exposure but should not be documented as a safe workaround. No labels are currently attached. This should be treated as a bug with high correctness/reliability severity due to silent result corruption plus process crash. Breakwater-GitHub-Analysis-Slot: slot_a09751a013c3 -- 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]
