MDEV-40032: Promote wide VARCHAR to BLOB for HEAP internal temp tables#5230
Open
arcivanov wants to merge 10 commits into
Open
MDEV-40032: Promote wide VARCHAR to BLOB for HEAP internal temp tables#5230arcivanov wants to merge 10 commits into
arcivanov wants to merge 10 commits into
Conversation
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements support for BLOB, TEXT, JSON, and GEOMETRY columns in the HEAP (MEMORY) storage engine (MDEV-38975), allowing operations like GROUP BY, DISTINCT, and joins on blob columns to remain in memory without immediately forcing a conversion to Aria on disk. The code review identified several critical and high-severity issues, including a potential server crash due to a missing NULL check on ftfunc_list in sql_derived.cc, a compilation error from the missing definition of Item_type_holder::create_tmp_field_ex, and a potential infinite loop in heap_check_heap() under corrupted data. Additionally, the reviewer provided actionable suggestions to clean up redundant assignments, use typed pointer arithmetic instead of raw byte manipulation, and parenthesize macro operands to prevent precedence issues.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Closed
9 tasks
gkodinov
added
the
External Contribution
gkodinov
approved these changes
Jun 15, 2026
gkodinov
left a comment
gkodinov
left a comment
Member
There was a problem hiding this comment.
LGTM after reverting the space only changes. Please stand by for the final review.
arcivanov
force-pushed
the
MDEV-40032-main
branch
from
July 17, 2026 19:26
5afac2a to
d7a0dc7
Compare
…e blob columns
Remove the HA_NO_BLOBS restriction from the HEAP engine, allowing
the optimizer to keep temporary tables with BLOB/TEXT columns in
memory when they fit within max_heap_table_size / tmp_memory_table_size
limits. Additionally, advertise HA_CAN_GEOMETRY so explicit
CREATE TABLE ... ENGINE=MEMORY with GEOMETRY columns works.
Unlike other HEAP blob implementations (e.g. Percona), this patch
provides full HASH index support on blob columns, enabling efficient
lookups, GROUP BY, and DISTINCT operations directly in HEAP without
falling back to disk.
Architecture
------------
BLOB data is stored using continuation records -- additional fixed-size
records allocated from the same HP_BLOCK that holds regular rows. This
reuses existing allocation, free list, and size accounting with minimal
structural change, and avoids per-blob my_malloc() calls.
The existing single-byte visibility flag is extended into a flags byte
with bits for HP_ROW_HAS_CONT, HP_ROW_IS_CONT, HP_ROW_CONT_ZEROCOPY,
HP_ROW_SINGLE_REC, and HP_ROW_MULTIPLE_REC. Continuation records are
grouped into variable-length runs -- contiguous sequences within a leaf
block. Only the first record of each run carries a 10-byte header
(next_cont pointer + run_rec_count); inner records are pure payload.
Three storage formats, detected by flag bits via inline predicates:
Case A (HP_ROW_SINGLE_REC): single record, no header, data at
offset 0. Zero-copy read.
Case B (HP_ROW_CONT_ZEROCOPY): single run, multiple records.
Header in rec 0, data contiguous in rec 1..N-1. Zero-copy read
via chain + recbuffer.
Case C (HP_ROW_MULTIPLE_REC): one or more runs linked via
next_cont. Reassembly into blob_buff required.
Run allocation uses a two-phase strategy: (1) peek-then-unlink walk
of the free list detecting contiguous groups, (2) tail allocation
from HP_BLOCK for remaining data. A Step 3 scavenge fallback
walks the entire free list when tail allocation fails.
HP_SHARE::total_records tracks all physical records (primary +
continuation), while HP_SHARE::records remains the logical count
used by hash bucket mapping.
Reassembly buffer (HP_INFO::blob_buff) follows the same pattern as
InnoDB's blob_heap -- allocated once, grown via my_realloc, freed
on heap_reset()/close. Zero-copy cases (A/B) return pointers
directly into HP_BLOCK with no copy.
Full HASH index key handling for BLOB columns: hp_rec_hashnr(),
hp_rec_key_cmp(), hp_key_cmp(), hp_make_key(), hp_hashnr() are
extended for HA_BLOB_PART segments. Hash pre-check optimization
skips expensive blob materialization when hashes differ. PAD SPACE
collation semantics are preserved for blob key comparisons.
Field_blob_key (Monty) produces HEAP-native key format (4-byte length
+ 8-byte data pointer) directly, eliminating key buffer translation
between the SQL layer and HEAP engine.
SQL layer changes
-----------------
pick_engine() (new, extracted from choose_engine()): replaced the
blob_fields check with a reclength > HA_MAX_REC_LENGTH guard.
choose_engine() calls pick_engine() with the real reclength.
pick_engine() is also called early in finalize() with reclength=0
to predict whether the engine will be HEAP, enabling blob-aware
GROUP BY key setup that avoids unnecessary m_using_unique_constraint.
finalize(): HEAP+blob uses fixed-width rows; GROUP BY key setup sets
key_part_flag from field, uses item max_length for blob key sizing.
store_length initialized for all GROUP BY key parts. key_type uses
field->binary() to determine FIELDFLAG_BINARY vs text collation.
DISTINCT key setup skips null-bits helper for HEAP.
remove_duplicates(): blob check moved before HEAP check to fall
through to remove_dup_with_compare().
Aggregator_distinct::add(): overflow-to-disk conversion via
create_internal_tmp_table_from_heap() for non-dup write errors.
Expression cache disabled for HEAP+blob (key format incompatibility).
FULLTEXT early detection in mysql_derived_prepare(): forces disk
engine via TMP_TABLE_FORCE_MYISAM when outer query uses MATCH.
Deferred blob chain free (MDEV-39732): heap_delete() saves chain
pointers to pending_blob_chains, flushed on next mutation or
heap_reset()/close. Prevents dangling zero-copy pointers during
binlog_log_row().
REPLACE safety (MDEV-39825): HP_SHARE::write_can_replace flag
forces copy mode in hp_read_blobs(), preventing blob data corruption
from freed-then-reused continuation records during REPLACE.
Geometry GROUP_CONCAT fix (MDEV-39761): downgrade Field_geom to
Field_blob for GROUP_CONCAT temp tables in both expression creation
paths. Type_handler_geometry::type_handler_for_tmp_table() added.
Geometry GROUP BY key fix (MDEV-39871): detect when new_key_field()
produced non-blob Field_varstring for a blob column, replace with
Field_blob_key.
Performance
-----------
Non-blob tables: zero regression. Every blob-specific code path is
guarded by if (share->blob_count). No new allocations, no format
changes, no hash function changes for non-blob keys.
Blob tables: eliminates file creation/deletion overhead and page cache
management. For single-run blobs (common case), the read path is
entirely zero-copy.
Limitations
-----------
1. No BTREE indexes on blob columns (HASH only)
2. No partial-key prefix indexing for blobs
3. 2x memory for Case C reads only (A/B are 1x)
4. No blob compression
5. 65,535 records per run (uint16 cap, auto-split)
6. max_heap_table_size applies to continuation records
7. Expression cache disabled for HEAP+blob
8. FULLTEXT forces disk engine
Linked bugs fixed:
- MDEV-39703: mroonga fulltext test ordering
- MDEV-39723: ER_DUP_ENTRY on GROUP BY with blob column
- MDEV-39724: crash in hp_is_single_rec with GROUP BY
- MDEV-39732: slave crash in hp_free_run_chain on blob replication
- MDEV-39761: Field_geom::store() assertion in GROUP_CONCAT
- MDEV-39782: RBR ER_KEY_NOT_FOUND on HEAP blob UPDATE
- MDEV-39825: blob data corruption on REPLACE into HEAP table
- MDEV-39871: crash in my_hash_sort_bin on GROUP BY with geometry
Reviewed by: Michael Widenius <monty@mariadb.org>
Monty reviewed the entire patch. Areas where he suggested changes
or contributed code:
- Field_blob_key class (HEAP-native blob key format, 4-byte length +
data pointer)
- Duplicate key fix on HEAP-to-Aria conversion
- hp_blob_key_length() uint32 fix
- hp_rec_hashnr_stored removal
- type_handler_for_tmp_table() param cleanup
- Type_handler_geometry::type_handler_for_tmp_table() virtual
- blob pointer bzero()
- find_unique_row() double-materialization fix
- Tail reclaim review
- Batch tail allocation review
- hp_update.c cleanup
- Field_blob_compressed temp table fix
- row_pack_length() dedup
- pack_length_no_ptr() removal
- Race condition fix in HEAP
- MDEV-39703 mroonga test fix
- MDEV-39825 write_can_replace optimization
- is_text_key_segment removal (field->binary() simplification)
- Documentation (Docs/internal-temporary-tables.txt)
Contribution by: Alexander Barkov <bar@mariadb.com>
Type_handler::make_and_init_table_field_ex() -- refactored temp table
field creation from inline code in sql_select.cc into type handler
virtual methods (sql_type.cc, sql_type_geom.cc), enabling clean
per-type-handler field creation for HEAP blob promotion.
Seven code fixes, a new test, and test re-recordings for issues found by CI on PR MariaDB#5222. **NULL dereference in `create_tmp_field()`**: `SYS_REFCURSOR` plugin returns NULL from `make_new_field()` (cursor values cannot be materialized). The feature added `result->flags |= FIELD_PART_OF_TMP_UNIQUE` without a NULL check. Added `if (result)` guard. **xmltype identity loss and recursive CTE reclength mismatch in `Item_type_holder::create_tmp_field_ex()`**: the blob_key dispatch now requires both: (1) `type_handler_for_tmp_table()` returns `blob_key_type_handler()`, AND (2) `dynamic_cast<Type_handler_blob_common*>` confirms the original type is a native blob. Condition 1 excludes xmltype (its override returns itself). Condition 2 excludes VARCHAR types promoted via `varstring_type_handler()` -> `too_big_for_varchar()` -> `blob_type_handler()`. Without condition 2, wide VARCHAR in recursive CTEs (e.g. `cast('...' as varchar(1000))`) was promoted to `Field_blob_key` in the main UNION DISTINCT table (`part_of_unique_key=true`) but stayed as `Field_varchar` in the incremental table (`part_of_unique_key=false`), causing a `reclength` mismatch assertion in `select_union_recursive::send_data()` (`main.json_equals` crash). **Spurious `reclength > HA_MAX_REC_LENGTH` in `pick_engine()`**: the original `choose_engine()` (both 10.11 and upstream/main) never had a reclength check. MDEV-38975 introduced it when replacing the `blob_fields` condition. HEAP has no internal reclength limit -- `hp_create.c` stores `uint reclength` and allocates blocks of that size; `max_supported_record_length()` is only checked in `unireg.cc` during user-facing CREATE TABLE. I_S tables like SLAVE_STATUS routinely have reclength ~880KB (13 bare `Varchar()` columns). The check forced them to Aria where `fill_slave_status()` returned 0 rows. Removed the check and the unused `reclength` parameter from `pick_engine()`. **Multi-update `tmp_memory_table_size` override**: the 10.11 feature overrode `big_tables=FALSE` for multi-update dedup tables. The forward-port translated this as `tmp_memory_table_size=SIZE_T_MAX` when the variable was 0. But `big_tables=FALSE` was a soft "don't force disk" hint, while `tmp_memory_table_size=SIZE_T_MAX` overrides the user's explicit `tmp_memory_table_size=0` directive. Since main removed `big_tables` entirely (MDEV-19713), the override is not needed. Removed. **Zero-length key rejection in `check_tmp_key()`**: reject `key_len == 0` to prevent useless zero-length keys from being created by `add_tmp_key()`. Reachable when all key parts are CHAR(0) NOT NULL: `key_length()` returns 0, the field is not nullable (no HA_KEY_NULL_LENGTH) and not VARCHAR/BLOB/GEOMETRY (no HA_KEY_BLOB_LENGTH), so `fld_store_len` is 0 for every part. Without this guard, `check_tmp_key()` would accept the key (0 <= max_key_length), and the optimizer would create a ref key that cannot distinguish any rows. Added `heap.char0_key` test exercising this via a materialized derived table with CHAR(0) NOT NULL join columns. **Non-deterministic `column_compression` test**: HEAP blob support allows compressed VARCHAR/TEXT temp tables to stay in HEAP instead of falling to Aria, changing row iteration order. Added `--sorted_result` to the two MDEV-24726 subqueries that lack `ORDER BY`. Test changes: - `spatial_utility_function_collect`: added ORDER BY to window function that lacked it (results were engine-row-order-dependent) - `tmp_space_usage`: removed multi-update override; forced disk for MDEV-34016/34060 Aria-specific test sections (blob I_S tables now stay in MEMORY) - `blob_update_overflow`: replaced `SHOW STATUS LIKE 'Created_tmp_%'` with targeted I_S query (Created_tmp_files varies on sanitizer builds) - `column_compression`: added `--sorted_result` for MDEV-24726 queries - `char0_key` (new): CHAR(0) NOT NULL derived table ref key rejection - Re-recorded 8 tests for expected "temp table stays in MEMORY" changes
CHECK TABLE is now supported, but will only return ok or fail. Still good enough for testing heap table consistenty.
This adds support for BIT_FIELD in record and keys for HEAP tables. The HEAP engine now has HA_CAN_BIT_FIELD set in table_flags() Multiple bugs in BIT field handling was found fixed. Some in HEAP table code, other bugs was affecting usage of BIT fields as keys.
Replace the HEAP engine's O(records) per-record free-list traversal with O(blocks) by grouping contiguous deleted records into logical blocks. Each block uses two sentinel records (block-start at the lowest address, block-end at the highest) to represent an arbitrarily large contiguous group. Interior "dark" records are implicitly covered by the block's address range and have their metadata bytes cleared by `hp_clear_dark_records()`. **Block metadata layout** (stored inline in deleted records): - Byte 8 (`HP_DEL_FLAG_OFFSET`): `del_flag` -- `HP_DEL_BLOCK_START` (2) on the first record, `HP_DEL_BLOCK_END` (1) on the last, 0 on dark records and singles - Bytes 9-10 (`HP_DEL_COUNT_OFFSET`): `uint16` block record count, stored only on the block-start record **`visible` minimum** bumped from `sizeof(char*)` (8) to `HP_DEL_METADATA_SIZE` (11) for all tables. This has zero memory impact: `recbuffer = ALIGN(visible + 1, 8)` absorbs the shift entirely (16 bytes for all small records). **Delete neighbor coalescing**: `hp_push_free_block_coalesce()` (in `hp_delete.c`) normalizes the free-list head by treating a single record as a block of count 1, then checks adjacency in two directions (above/below). Handles all merge cases uniformly: single-to-single, single-to-block, block-to-single, and block-to-block. Combined count capped at `UINT_MAX16`; falls back to `hp_push_free_record` / `hp_push_free_block` when no adjacency. `hp_push_free_record_coalesce()` is a thin inline wrapper. **Dark record clearing** is centralized in `hp_clear_dark_records`, a strided loop that zeroes only the essential metadata bytes per record (`del_link`, `del_flag`, `visible`) rather than the entire `recbuffer`. Used by both `hp_push_free_block` and `hp_push_free_block_coalesce`. **New API in `heapdef.h`:** - Push: `hp_push_free_record()`, `hp_push_free_block()`, `hp_push_free_record_coalesce()` - Consume: `hp_pop_free_record()`, `hp_take_free_block()` - Traverse: `hp_is_free_block_end()`, `hp_free_block_first()`, `hp_is_free_block_start()`, `hp_free_block_start_count()` Non-inline functions: `hp_push_free_block()` and `hp_push_free_block_coalesce()` in `hp_delete.c`; `hp_take_free_block()` in `hp_write.c`. **Caller updates:** - `hp_delete.c`: `heap_delete()` calls `hp_push_free_record_coalesce()` for single-record deletes - `hp_write.c`: error-path push and `next_free_record_pos()` pop use `hp_push_free_record()` / `hp_pop_free_record()` - `hp_blob.c`: `hp_free_run_chain()` uses coalescing versions; `hp_take_free_list_runs()` extracted from Steps 1 and 3 of `hp_write_one_blob()`, parameterized by `min_avail`; blob allocation uses `hp_take_free_block()` / `hp_pop_free_record()` - `hp_scan.c`: batch-skip entire deleted blocks - `_check.c`: block-aware free-list count and scan - `hp_shrink_tail()`: reclaims entire blocks at the tail **Unit tests** (`hp_test_freelist-t.c`): 13 new tests (252 assertions): block formation, pop/shrink, collapse to single, partial block take, scan batch-skip, shrink-tail with blocks, interleaved singles and blocks, ascending/descending delete coalescing, non-adjacent gap, coalesced block reuse by blob insert, block-to-block via adjacent blob chains.
…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.
…ap_update()` `heap_update()` moves all changed key entries to the new key values **before** writing the new blob chains. When a blob chain write then failed (e.g. with `HA_ERR_RECORD_FILE_FULL`), the rollback restored the record bytes and blob chain pointers, but the `err:` label only undid key changes for `HA_ERR_FOUND_DUPP_KEY` -- historically the only possible failure once the key loop had run. The hash/btree entries were left keyed on the new values while pointing at a record holding the old values, corrupting the index: 1. index lookups by the old key value missed the row 2. `CHECK TABLE` reported the table corrupt 3. on debug builds the heap consistency check in `ha_heap::external_lock()` raised a second error into an already-set diagnostics area, firing a `Diagnostics_area` assertion on the next statement Fix: give the blob-write failure its own recovery label (`err_blob:`) that moves every changed key back to the old value (`delete_key(new)` + `write_key(old)`), mirroring the existing `HA_ERR_FOUND_DUPP_KEY` recovery. All keys were fully moved before the blob section runs, so a plain sweep over all changed keys suffices, and the record at `pos` has already been restored to the old image -- exactly the state the `DUPP_KEY` recovery path operates in. The blob failure's errno is captured before the rollback loop and restored after it, so that a rollback `write_key` failure (which `hp_rb_write_key()` reports as `HA_ERR_FOUND_DUPP_KEY` with a stale `errkey`) cannot mask the original error. `err_blob:` then falls through into `err:` to share the record-count/blength epilogue, which is safe because the restored errno is never `HA_ERR_FOUND_DUPP_KEY`. The new test `heap.blob_update_key_rollback` exercises hash, BTREE, two changed indexes, an index on an unchanged column (which the rollback must leave untouched), and a partial multi-row UPDATE; each asserts the index stays consistent after the failure via `CHECK TABLE` and index lookups.
arcivanov
force-pushed
the
MDEV-40032-main
branch
from
July 17, 2026 22:12
d7a0dc7 to
62b84f6
Compare
`init_block()` sizes every `HP_BLOCK` allocation as `max_records / heap_allocation_parts` records. `max_records` is normally computed from the table memory ceiling (`max_heap_table_size` / `tmp_memory_table_size`), not from any estimate of the expected number of rows, so a tuned-up ceiling turns directly into giant allocations: with `tmp_table_size=16G` the **first row written** to an internal temporary table allocates a 256MB..2GB block, and the per-key `HASH_INFO` blocks are inflated the same way. For create-and-drop-per-statement tables (`SHOW`/`INFORMATION_SCHEMA` materializations, `DISTINCT`/`GROUP BY` temporary tables) such allocations are served by `mmap()` and unmapped again on drop by common malloc implementations, so every statement pays page fault-in and zeroing, page-table teardown with TLB-shootdown IPIs, and process-wide `mmap_lock` serialization. On a `SHOW FULL COLUMNS` loop workload with `tmp_table_size=16G` this loses 36% QPS at 16 threads, 50% at 64 and 60% at 128; blob-bearing I_S temporary tables newly qualify for HEAP since MDEV-38975, which exposed the pre-existing sizing heuristic to this workload. Fix: cap ceiling-derived block allocations at `heap_max_allocation_block` (4MB): 1. 4MB stays within the range that mainstream allocators (glibc, jemalloc, tcmalloc, mimalloc) recycle from their free lists instead of returning to the kernel on free, so per-statement blocks are reused with no syscalls at steady state. The nearest boundary is jemalloc's `oversize_threshold` (8MB); glibc's dynamic mmap threshold adapts up to 32MB. 2. 4MB is large enough to keep typical blob values (up to ~1MB) in a single continuation run, preserving zero-copy blob reads. 3. At the default `tmp_table_size` (16MB) blocks come out at 1-2MB, so default-configuration sizing is unchanged; the cap only binds for ceilings above ~64MB. An explicit `min_records` (`CREATE TABLE ... MIN_ROWS=N`) is a real row count expectation and still pre-sizes beyond the cap. The cap compares against the **caller-supplied** `min_records`, not the defaulted "optimize for 1000 rows" value: for rows wider than ~4KB (`heap_max_allocation_block / 1000`) the 1000-row default exceeds `cap_records` and would otherwise silently override the cap (8KB rows -> 8MB blocks, 64KB rows -> 64MB blocks, and a row wider than the cap itself -> an `INT_MAX32`-clamped 1GB block). Wide internal temporary table rows are reachable without `MIN_ROWS`: fields wider than the VARCHAR limit become out-of-row blobs, but many inline columns (multi-table joins with `DISTINCT`/`GROUP BY`, wide `CHAR` columns) can sum past 4KB. Rows wider than the cap itself cannot honor it and degrade to the existing 10-records-per-block floor (e.g. 5MB rows -> 64MB blocks), which is as close to the cap as a block that must hold at least a few whole rows can get. Tables larger than 4MB simply allocate more blocks; the `HP_PTRS` block tree (128-ary) accommodates this with no depth issues. Tests: - `storage/heap/hp_test_block_size-t.c`: unit tests asserting the record and hash key block `alloc_size` cap with a ceiling-derived `max_records` (keyed and keyless), `MIN_ROWS` override, unchanged small-table sizing, and full functionality across the first-block boundary at the capped geometry (45K rows, key reads, `heap_check_heap`); wide-row scenarios asserting the cap holds for 8KB and 64KB rows despite the defaulted `min_records`, that rows wider than the cap degrade to the 10-record floor instead of the 1000-record geometry, and that an explicit `min_records` still overrides the cap for wide rows. - `mysql-test/main/tmp_table_heap_alloc.test`: end-to-end check that `Max_memory_used` stays bounded when a small `SELECT DISTINCT` on a TEXT column and a `SHOW FULL COLUMNS` materialize under a 4GB ceiling.
…y value A rejected duplicate insert into a HEAP unique hash index paid the full-value key hash twice: `hp_write_key()` hashed the key to insert it (the documented contract was "the record was still added and the caller must call hp_delete_key for it"), and `hp_delete_key()` then hashed the same record **again** just to locate the bucket to unlink from. For long key values (BLOB keys, MDEV-38975) the second scan dominates duplicate-heavy workloads: `SELECT DISTINCT` over 20-50KB blob values (~80% duplicates) regressed 25-32% vs Aria tmp tables at low concurrency, with `my_uca_hash_sort_utf8mb4` doing 1.83x the hashing work for the identical query. The hash caching added by `de1765fb64d` (`HASH_INFO::hash_of_key`) already made all chain surgery hash-free; the two full-value hashes per rejected row were the entire cost. Fix: 1. **Probe before insert** (`hp_write_key()`): compute the hash once at the top; for `HA_NOSAME` keys without NULL key parts walk the key's chain under the pre-insert mask comparing cached `HASH_INFO::hash_of_key` values, comparing full key values only on a hash match. On a duplicate return `HA_ERR_FOUND_DUPP_KEY` with nothing modified; otherwise proceed with the linear-hash split and insertion reusing the already-computed hash. Records with equal full hashes share a bucket under any mask, so probing the pre-insert chain finds any duplicate. A successful insert costs exactly one full-value hash, as before; a rejected duplicate drops from two full hashes + insert + undo delete to one hash + compare, the same as a lookup. 2. **Error-path contract change**: `hp_write_key()` no longer inserts the key on a duplicate, so `heap_write()` rolls back only the preceding keys (unconditional `keydef--`, as for BTREE/ENOMEM), and `heap_update()`'s duplicate path now re-inserts the old key for the failing keydef for both algorithms (previously BTREE-only) before rolling back earlier keys. 3. **Delete-side hash reuse** (`hp_delete_key()`): when the row being deleted was positioned via the same hash index (`flag` set and `info->current_hash_ptr->ptr_to_rec == recpos`), take the hash from the index entry instead of re-scanning the key value; a `DBUG_ASSERT` cross-checks the cached hash in debug builds. This removes the remaining full-value hash from `DELETE`/`UPDATE` of rows located through the index (`heap_rkey()` already hashed the key). 4. **`heap_rfirst()`/`heap_rlast()`**: clear `info->current_hash_ptr` when rejecting a hash index with `HA_ERR_WRONG_COMMAND`. Both functions retarget `info->lastinx` before the algorithm check, so a stale `current_hash_ptr` from a previous search on a different key could otherwise satisfy the new cached-hash guard in `hp_delete_key()` and send the bucket lookup to the wrong chain (reachable through the heap API only; the SQL layer never issues ordered reads on hash indexes). New unit test `hp_test_write_dup-t` (105 assertions) wraps the key charset's `hash_sort` collation handler in a counting shim, asserting the exact number of full-value hashes for every operation: 1 per insert attempt (successful or rejected), 1 total for index-read + delete, 1 for an index-positioned re-key. Behavioral coverage: single- and multi-key rollback for INSERT and UPDATE duplicates, NULL key parts, non-unique keys, delete after rejected `heap_rfirst()`/`heap_rlast()`, and a 500-row duplicate-heavy stress across linear-hash splits (exactly 500 hashes; was 827 before the fix). Benchmarked (16 threads, 120s runs, 8c/16t AMD 7840HS; `base` = preview-13.1 without MDEV-38975 = Aria tmp tables, `new` = unfixed, `fix` = this patch): | test (QPS) | base | new | fix | |---------------|------|--------------|---------------------| | `blob_case_c` | 4.65 | 3.36 (-28%) | 5.61 (+21% vs base) | | `blob_mixed` | 5.87 | 4.13 (-30%) | 6.92 (+18% vs base) | `hp_delete_key()` (38% inclusive before) is absent from the fixed profile; the low-concurrency regression becomes a win on top of the existing 2x+ high-concurrency wins.
Use `Field_blob_key` as the single unified mechanism for ALL blob columns in HEAP temp tables, replacing Phase 1's dual approach of `Field_blob_key` for GROUP BY/DISTINCT + `rebuild_blob_key_from_segments` for derived table ref access. Key changes: - `Tmp_field_param::is_heap_engine()` gates `Field_blob_key` creation for all blob fields in HEAP temp tables (not just `part_of_unique_key`) - `varstring_type_handler()` promotes VARCHAR > `HEAP_CONVERT_IF_BIGGER_TO_BLOB` to blob via `blob_type_handler()` -> `type_handler_blob_key` - `Type_handler_blob_common::type_handler_for_tmp_table()` returns `blob_key_type_handler()` when `is_heap_engine()` - `Item_field::create_tmp_field_from_item_field()` redirects HEAP blob fields through the type handler system - `Item_type_holder::create_tmp_field_ex()` extended for UNION/CTE blobs Fix three latent bugs in `Field_blob_key` exposed by ref access: 1. `Field_blob_key::key_cmp()` treated key bytes at offset 4 as inline data, but the key format is `[4B length][8B pointer_to_data]` -- comparison was against raw pointer bytes instead of actual data. 2. `cmp_buffer_with_ref()` eq_ref cache compared raw key buffer bytes. When `Field_blob_key::value` buffer is reused across lookups, the `[4B length][8B pointer]` bytes don't change even when the pointed-to data differs, causing stale result reuse. Disable the cache for all HEAP blob key parts (remove the `length == 0` guard). 3. `Field_blob_key::new_key_field()` returns `Field_blob_key` (unlike `Field_blob::new_key_field()` which returns `Field_varstring`). The `store_key` mechanism stores into `to_field->value` String, which leaks because `store_key` is `Sql_alloc` with no destructor. Add `store_key::cleanup()` called from `JOIN_TAB::cleanup()` and `subselect_uniquesubquery_engine::cleanup()`. `Field_blob_key::key_part_length_bytes()` changed from 4 to 0 so `store_length = key_length (12) + null_byte + 0`, matching HEAP's `seg->length (12) + null` for correct multi-part key alignment. `hp_key_cmp()` blob packlength changed from hardcoded 4 to `seg->bit_start` (actual field packlength) for TEXT (packlength=2). Re-record GROUP_CONCAT-related results: the `Tmp_field_param` threading through `tmp_table_field_from_field_type()` (base branch) closed a plumbing gap where `Item_sum` and literal items dropped the param, so they now reach the HEAP promotion gates like all other expression items. `GROUP_CONCAT` results in HEAP temp tables become `Field_blob_key` (longtext metadata, 12-byte blob key parts), consistent with the `Item_func` path that was already recorded (e.g. `substring()` sj-materialization keys).
arcivanov
force-pushed
the
MDEV-40032-main
branch
from
July 19, 2026 23:55
62b84f6 to
45a781a
Compare
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.
Summary
octet_length > HEAP_CONVERT_IF_BIGGER_TO_BLOB(32 bytes) to BLOB when the temporary table uses the HEAP engine, storing only actual data in continuation chains (from MDEV-38975) instead of reserving the full declared width in every rowField_blob_keyas the single unified mechanism for ALL blob columns in HEAP temp tables -- both GROUP BY/DISTINCT keys and derived table ref accessField_blob_keybugs exposed by ref access:key_cmp()pointer-vs-data comparison,cmp_buffer_with_ref()stale cache, andnew_key_field()memory leakheap_store_key_blob_refstore_key subclass bypasses SQL-layer key buffer for BLOB key parts, enabling ref access for all BLOB sizes including LONGBLOB/JSON on HEAP derived tablesDepends on MDEV-38975 (#5222), MDEV-40030, MDEV-40029.
JIRA: https://jira.mariadb.org/browse/MDEV-40032