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. |
|
|
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) |
…ills HEAP tmp table `save_window_function_values()` did not take into account that one can get `HA_ERR_RECORD_FILE_FULL` on `ha_update_row()`. With blob support in heap, it can now happen more easily (expressions wider than 512 characters are promoted to TEXT in tmp tables, and each update of a blob result column allocates a continuation record). However, it could also happen with Aria tables, which is apparently not tested. The error was returned as a plain `true` 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). Errors of the computation are now exposed: `save_window_function_values()` calls `handler::print_error()` for any failure it does not recover from, the previously ignored `ha_rnd_pos()` return values are checked and reported, and `compute_window_func()` distinguishes a read error from EOF and stops the row scan as soon as the per-row loop fails. The overflow itself is recovered from by converting the tmp table to the disk-based tmp engine in place: the rows keep their identity and their contents, only their positions change, and those are translated in the rowid sequence that the computation is built on. 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 (`Window_rowid_remapper`) 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 very slot the old position was read from. For the new position to always fit into the slot, a window sort stores the row positions in slots of a fixed 8 bytes (`Filesort::min_ref_length`, set to the new `TMP_TABLE_MAX_REF_LENGTH`), which holds the position of any engine an internal tmp table can use: a pointer into memory or a data file offset. `make_sortkey()` zero-pads a shorter position to its slot, and `SORT_INFO::ref_length` carries the slot width to the readers, so `init_read_record()` no longer re-derives it from `handler::ref_length`, which changes when the table is converted. Sorts that do not set a minimum, and engines whose positions are wider than 8 bytes, are unchanged byte for byte. A sequence held in memory is thereby rewritten in place, keeping its layout, so the frame cursors' places in it stay valid. 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 (`Frame_cursor::rowids_rewritten()`). 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 for the copy from the HEAP table to the Aria or MyISAM table that replaces it. The plain copy loop and the append of the pending `record[0]` become `Tmp_table_default_copier`, used when the caller passes no copier, so both cases of the conversion run through the same code, defined together with `Window_rowid_remapper` just before the function. `ha_end_bulk_insert()` is called also when the copy fails, so the new handler does not keep bulk insert state when its table is dropped; the pending row is thereby written while the bulk insert is still active, which does not delay its duplicate detection, as bulk insert does not cache unique keys (`maria_init_bulk_insert()` skips `HA_NOSAME` keys). An ignored duplicate of the pending row is passed to the caller in `Tmp_table_row_copier::duplicate_row_error`, which sets `*is_duplicate`. Carrying the running computation over the conversion, instead of re-running it, 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, so the conversion does not have to be refused when such an expression is non-deterministic (`RAND_TABLE_BIT`: a user variable assignment, a non-deterministic stored function); those statements produce their result. - The statement sorts once, so sort related aggregate warnings, e.g. the `max_sort_length` truncation counts, are reported for one pass. 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: a single window over all rows and a partitioned window overflow mid-computation and must transparently convert; 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()and did not take into account that this can fail withHA_ERR_RECORD_FILE_FULL. With blob support in HEAP this now happens easily: 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, and once this crossesmax_heap_table_sizethe update fails -- but it could also happen with Aria tables, which was apparently never tested. The error was returned as a plaintruewithouthandler::print_error()and without any overflow 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: the table is converted to the disk-based tmp engine 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 (
Window_rowid_remapper) 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 very slot the old position was read from -- no position map, O(1) extra memory at the moment the server is out of memory. The row whose update did not fit is written fromrecord[0], which holds its new image, so the conversion applies the update that failed.Filesort::min_ref_length, set to the newTMP_TABLE_MAX_REF_LENGTH), enough for the position of any engine an internal tmp table can use -- a pointer into memory or a data file offset -- on 64-bit and 32-bit systems alike.make_sortkey()zero-pads shorter positions to the slot, andSORT_INFO::ref_lengthcarries the slot width to the readers, soinit_read_record()no longer re-derives it fromhandler::ref_length, which changes when the table is converted. Sorts that set no minimum, and engines with positions wider than 8 bytes, are unchanged byte for byte. An in-memory sequence is thereby rewritten in place, keeping its layout, and the frame cursors' places in it stay valid; a sequence held in a temporary file cannot be rewritten in place (itsIO_CACHEencrypts the file when tmp file encryption is enabled), so the translated sequence is streamed into a new cached temporary file that replaces the old one under the cache, with the cursors' slave caches re-created from the new master (Frame_cursor::rowids_rewritten()).create_internal_tmp_table_from_heap()gains aTmp_table_row_copierhook for the copy from the HEAP table to the Aria or MyISAM table that replaces it; its old copy loop and the append of the pendingrecord[0]becomeTmp_table_default_copier, so both cases of the conversion run through the same code.ha_end_bulk_insert()runs also when the copy fails, and an ignored duplicate of the pending row reaches the caller viaTmp_table_row_copier::duplicate_row_error.RAND_TABLE_BIT: user variable assignments, non-deterministic stored functions) do not need to be refused: nothing is re-evaluated, so those statements 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 reported for one pass.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. On 64-bit the fixed-width path is exercised end to end by the same tests (8-byte slots holding shorter Aria positions after the conversion); the 32-bit case, where the padding is nonzero for HEAP positions too, is covered by the 32-bit buildbot builders.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