MDEV-40376 Protocol::end_statement assertion when window function fills HEAP tmp table#5406
Open
arcivanov wants to merge 1 commit into
Open
MDEV-40376 Protocol::end_statement assertion when window function fills HEAP tmp table#5406arcivanov wants to merge 1 commit into
arcivanov wants to merge 1 commit into
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
|
…ills HEAP tmp table `save_window_function_values()` stores computed window function values back into the sorted tmp table with `ha_update_row()`. When the result column is a blob (expressions wider than 512 characters are promoted to TEXT in tmp tables), each update allocates a blob continuation record in the HEAP tmp table. Once this crosses `max_heap_table_size`, the update fails with `HA_ERR_RECORD_FILE_FULL`, which was returned as a plain `true` without calling `handler::print_error()` and without any overflow-to-Aria handling. The statement then failed with an **empty diagnostics area**: debug builds hit `Assertion '0'` in `Protocol::end_statement()`, release builds send a bare OK packet after the result set metadata, which the client mis-parses (appears as a hang / lost connection). The write path into window tmp tables already converts HEAP to Aria on overflow (`create_internal_tmp_table_from_heap()`); the update path performed during window function computation had no such handling. The fix has two layers: 1. **Error exposure**: `save_window_function_values()` now calls `print_error()` for any `ha_update_row()` failure it does not recover from; the previously ignored `ha_rnd_pos()` return values in `save_window_function_values()` and `compute_window_func()` are checked and reported; `compute_window_func()` distinguishes a read error from EOF (positive `read_record()` return) and stops the row scan as soon as the per-row loop fails instead of continuing to rewrite the remaining rows after an error or `KILL`. 2. **Overflow recovery**: `HA_ERR_RECORD_FILE_FULL` from a HEAP tmp table is propagated up (via a new `out_error` parameter through `compute_window_func()` and `Window_func_runner::exec()`) to `Window_funcs_sort::exec()`, which converts the table to Aria with `create_internal_tmp_table_from_heap()` and re-runs the whole sort step. Converting at the point of failure is not possible: the filesort result and the frame cursors hold raw HEAP row positions that the conversion invalidates, so the filesort result is discarded and rebuilt after the conversion. Re-running the computation is safe because it reads only the source columns (preserved by the conversion) and unconditionally recomputes and overwrites the window function result columns. The re-run also re-evaluates compound expressions containing window functions (`items_to_copy`) for every row, which is only correct for side-effect-free expressions. The retry is therefore refused -- the original "table is full" error is reported instead -- when any such expression is non-deterministic (`RAND_TABLE_BIT`: user variable assignments, non-deterministic stored functions). A captured engine error that does not qualify for the retry is reported via `print_error()` so no path can leave the diagnostics area empty. `create_internal_tmp_table_from_heap()` gains a `copy_pending_row` parameter (defaults to `true`, preserving all existing call sites): the write-overflow paths append the pending `record[0]` that failed to be written, but here the row whose update failed already exists in the table and must not be appended again. Note: a spilling statement reports sort-related aggregate warnings (e.g. `max_sort_length` truncation counts) for both sort passes. This is consistent with the existing statement-wide accumulation semantics of `THD::num_of_strings_sorted_on_truncated_length` -- the sort really does run twice, as it already does in multi-sort statements. Add `heap.blob_window_overflow` test: window function computation over a HEAP tmp table with a blob result column overflows mid-computation and must transparently spill to Aria -- a single window over all rows, a partitioned window, and refusal of the retry for a side-effecting compound window expression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
MDEV-40376
save_window_function_values()stores computed window function values back into the sorted tmp table withha_update_row(). When the result column is a blob (expressions wider than 512 characters are promoted to TEXT in tmp tables), each update allocates a blob continuation record in the HEAP tmp table. Once this crossesmax_heap_table_size, the update fails withHA_ERR_RECORD_FILE_FULL, which was returned as a plaintruewithouthandler::print_error()and without any overflow-to-Aria handling. The statement then failed with an empty diagnostics area: debug builds hitAssertion '0'inProtocol::end_statement(), release builds send a bare OK packet after the result set metadata, which the client mis-parses (appears as a hang / lost connection).The write path into window tmp tables already converts HEAP to Aria on overflow; the update path performed during window function computation had no such handling.
Fix
Error exposure:
save_window_function_values()callsprint_error()for anyha_update_row()failure it does not recover from; previously ignoredha_rnd_pos()return values are checked and reported;compute_window_func()distinguishes a read error from EOF and stops the row scan as soon as the per-row loop fails (instead of continuing to rewrite remaining rows after an error orKILL).Overflow recovery:
HA_ERR_RECORD_FILE_FULLfrom a HEAP tmp table is propagated (newout_errorparameter) up toWindow_funcs_sort::exec(), which converts the table to Aria viacreate_internal_tmp_table_from_heap()and re-runs the whole sort step. Converting at the point of failure is not possible: the filesort result and frame cursors hold raw HEAP row positions that the conversion invalidates, so the filesort result is discarded and rebuilt. The re-run is safe because it reads only the source columns (preserved by the conversion) and unconditionally recomputes the window function result columns.RAND_TABLE_BIT: user variable assignments, non-deterministic stored functions), since the re-run would re-apply side effects for rows already processed.create_internal_tmp_table_from_heap()gains acopy_pending_rowparameter (defaulttrue, all existing call sites unchanged): the row whose update failed already exists in the table and must not be appended as an extra row.Testing
New
heap.blob_window_overflowtest: single window over all rows, partitioned window, and retry refusal for a side-effecting compound window expression. Reproduced the assertion pre-fix; post-fix the heap suite (28), allmain.win*tests, and the tmp-table spill-path tests (group_by,union,derived,subselect_mat,user_var, etc.) pass. gcov against the heap suite confirms every functional path of the change is exercised (spill-retry taken, retry refused,copy_pending_rowboth values); uncovered lines are fault-injection-class defensive error paths only.Release Notes
Window functions computed over an in-memory tmp table that fills up mid-computation now transparently spill to disk instead of failing with an empty diagnostics area (debug assertion / apparent client hang).
How can this PR be tested?
mysql-test/mtr heap.blob_window_overflowBasing the PR against the correct MariaDB version
mainbranch. (Preview branchpreview-13.1-preview, which carries the HEAP blob feature this fixes.)PR quality check