MDEV-40376 Protocol::end_statement assertion when window function fills HEAP tmp table - #5406
Open
arcivanov wants to merge 2 commits into
Open
MDEV-40376 Protocol::end_statement assertion when window function fills HEAP tmp table#5406arcivanov wants to merge 2 commits into
arcivanov wants to merge 2 commits 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.
gkodinov
added
the
External Contribution
gkodinov
approved these changes
Jul 20, 2026
gkodinov
left a comment
gkodinov
left a comment
Member
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
LGTM, pending the target branch clarification.
Please stand by for the final review.
Contributor
Author
|
This also needs to go into preview-13.1 as it's a fix for the feature introduced there (MDEV-38975) |
`save_window_function_values()` stores the computed window function values back into the sorted tmp table with `ha_update_row()`. When that fills the table, the conversion to Aria invalidates the row positions that the filesort result and the frame cursors are built on, which is why the computation was re-run from the start of the sort step. Translate the positions instead, so that nothing has to be re-run. The filesort result of a window sort is a sequence of row positions that covers every row of the table exactly once (the sort is set up without a limit) and is the only place where the positions are kept: the row scan and all frame cursors read the rows through it. The conversion therefore copies the rows in the order of that sequence, which makes the new position of a row known as soon as the row has been written, and stores it back into the slot the old position was read from. A position in the converted table is never longer than a position in the HEAP table, which is a pointer, so a sequence held in memory is rewritten in place. It is compacted while it is rewritten, because everything that reads it derives its layout from `handler::ref_length`, which the conversion shortens. A sequence held in a temporary file is written and read through an `IO_CACHE`, which encrypts the file when tmp file encryption is enabled, so the file can not be rewritten in place: the old sequence is streamed out of its cache and the translated sequence into a new cached temporary file, which then replaces the old one under the cache. The frame cursors' slave caches stay linked to the master through `next_file_user` across the replacement, and are re-created from the new master afterwards. Should re-creating one fail, the cursor forgets its already-released cache, so that it is not released a second time when the cursor is destroyed, and reports the failure. The row whose update did not fit is written from `record[0]`, which holds its new image, instead of being read from the table, so the conversion applies the update that failed. Blob values of `record[0]` that were read from the HEAP table can point into the handler's shared blob reassembly buffer (`hp_read_blobs()`, blob values that span multiple allocation blocks), which reading the other rows overwrites, so they are first given memory of their own. When the window sort was set up with a deferred filter (a HAVING clause deferred to the final ORDER BY sort), the sequence omits the rows the filter rejected and the conversion drops them: every later reader of the table applies the same filter, either directly or by reading the table through a sort that does, so such rows can never reach the result. `create_internal_tmp_table_from_heap()` gains a `Tmp_table_row_copier` hook that replaces its row copying loop and takes over the pending `record[0]`; it replaces the `copy_pending_row` parameter. `Window_func_runner::exec()` and `Window_funcs_sort::exec()` go back to their original form, as the conversion now happens where the failure is detected. The frame cursors keep the row they are on and take their layout, and whatever they cached from the sequence, again (`Frame_cursor::rowids_rewritten()`). Not re-running the computation has two visible consequences: - Compound expressions containing window functions (`items_to_copy`) are evaluated exactly once per row, as they are when the table does not overflow. The conversion no longer has to be refused, reporting that the table is full, when such an expression is non-deterministic (`RAND_TABLE_BIT`: a user variable assignment, a non-deterministic stored function); those statements now produce their result. - The statement sorts once, so sort related aggregate warnings, e.g. the `max_sort_length` truncation counts, are no longer reported for two passes. The rows are copied in the order of the sequence, so the converted table does not hold them in the order they were inserted in. This is safe even when that order is what the table was built for (`TABLE::keep_row_order`, set for `ROWNUM()` with GROUP BY or ORDER BY): before HEAP supported blobs, such a tmp table was created in the disk engine from the start and these statements simply ran, so refusing here would be a regression against that behavior. 1. `ROWNUM()` values are materialized into the tmp table rows during the fill, before the window computation begins; the copy moves them verbatim, so their pairing with the rows can not change. 2. The rowid sequence is translated in place, so the running window computation continues over exactly the same row order, and tie-sensitive window function values (`ROW_NUMBER`, frames over tied keys) keep the values the interrupted pass had already produced. 3. Consumers that scan the converted table afterwards see the rows in the copy order. Among rows with equal sort keys that order differs from the insertion order, but such tie order is unspecified and already differs between the memory and disk tmp engines. The converted table keeps `keep_row_order= true` so the copy order is also the order the disk engine preserves from then on. Tests, in `heap.blob_window_overflow` unless noted: the side-effecting window expression test verifies that the user variable is assigned exactly once per row, against the same query that does not overflow; a fourth test covers the sequence being an array in memory instead of a merge file; a fifth test covers the `keep_row_order` conversion, verifying that every `ROWNUM()` value keeps its insertion-order pairing with its row across the conversion; a sixth test covers blob values larger than a HEAP allocation block surviving the conversion of the pending row; `heap.blob_window_overflow_encrypt` runs the conversion with encrypted temporary files; `heap.blob_window_overflow_debug` covers the failure to re-create a slave cache of the sequence (`simulate_window_seq_slave_oom`), which previously hung the server in the cursor destructor.
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
4 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 (second commit; the first commit implemented this as a sort-step retry, which the second commit supersedes): the table is converted to Aria at the point of failure and the running computation is carried over the conversion; nothing is re-run.
The filesort result of a window sort is a total permutation of the table stored as a dense sequence of row positions, and it is the only place the positions are kept: the row scan and all frame cursors read the rows through it. The conversion copies the rows in the order of that sequence, so the new position of a row is known as soon as it is written and is stored back into the slot the old position was read from -- no position map, O(1) extra memory at the moment the server is out of memory. The sequence is rewritten in place and compacted to the disk engine's shorter
ref_length(everything that reads it derives its stride fromhandler::ref_length), and the merge file is truncated to match. Frame cursors re-derive their layout and their slaveIO_CACHEs through the newFrame_cursor::rowids_rewritten(). The row whose update did not fit is written fromrecord[0], which holds its new image, so the conversion applies the update that failed.create_internal_tmp_table_from_heap()gains aTmp_table_row_copierhook that replaces its row copying loop (supersedes the first commit'scopy_pending_rowparameter).RAND_TABLE_BIT: user variable assignments, non-deterministic stored functions) no longer need to be refused: nothing is re-evaluated, so those statements now produce their result with side effects applied exactly once per row.keep_row_ordertmp tables (ROWNUM()with GROUP BY or ORDER BY) are converted as well. The converted table holds the rows in the copy order rather than the insertion order;ROWNUM()values are materialized before the window step and keep their pairing with the rows, and the ordering difference is confined to unspecified tie order, which already differs between the memory and disk tmp engines. Before HEAP supported blobs these tmp tables were created on disk from the start, so refusing the conversion would have been a regression.max_sort_lengthtruncation) are no longer reported for two passes.Testing
heap.blob_window_overflowcovers six scenarios: single window over all rows, partitioned window, side-effecting compound expression (the user variable is verified to be assigned exactly once per row, identical to the same query without overflow), the position sequence held in memory instead of a merge file, thekeep_row_orderconversion (everyROWNUM()value verified to keep its insertion-order pairing with its row across the conversion; this test fails withER_RECORD_FILE_FULLif the conversion refuses), and blob values larger than a HEAP allocation block surviving the conversion of the row whose update overflowed (such values are reassembled into a shared buffer on read; pre-fix, the converted table received another row's bytes for that row).heap.blob_window_overflow_encryptruns the overflow with--encrypt-tmp-files=ON(the position sequence file is an encryptedIO_CACHE; pre-fix, raw file access read ciphertext as row positions).heap.blob_window_overflow_debuginjects a failure into the re-creation of a cursor's slave cache of the rewritten sequence and verifies a cleanER_OUT_OF_RESOURCES(pre-fix, the cursor destructor hung the server walking a stale cache ring).Reproduced the assertion pre-fix. Post-fix the heap suite, all
main.win*andmain.rownumtests, and fullmain+heapsuite runs pass. Conversion results were additionally validated against two baselines (HEAP without overflow, and disk-engine-from-the-start, i.e. the pre-blob behavior): byte-identical results including stream order for unique sort keys; identical row sets and identical tie-invariant values for heavily tied sort keys, with tie-order variation within the nondeterminism the two baselines already exhibit between each other.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