Console View
|
Categories: connectors experimental galera main |
|
| connectors | experimental | galera | main | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix after cherry-pick | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
fixup! Limit the memory used by GROUP_CONCAT() with ORDER BY `main.gconcat_warn` case 5 recorded `Row 88 was cut by group_concat()` and reads `Row 127` on a 32-bit build, so the test fails on `x86-debian-12-fulltest` and `x86-debian-12-fulltest-debug`. The case starves the sort tree until a repack drops rows, and the row the warning names is how many rows the budget held: the bytes the repack copies to, divided by `sizeof(TREE_ELEMENT)` plus the size of an element. `TREE_ELEMENT` is 24 bytes where a pointer is eight and 12 where it is four. The two recorded numbers agree with that. The case comment gives the repack 2720 bytes, and 2720 over 88 is about 31 bytes a row against 2720 over 127 for about 21, a difference of the order the smaller `TREE_ELEMENT` accounts for. The test already says this, and already masks the result it prints for the same reason: The exact result depends on sizeof(TREE_ELEMENT) and the reclength, so only check that the result came out short. The warning is printed beside that result and was not masked. Mask the row number with `--replace_regex`, as `main.gconcat_distinct_spill` masks the same warning for the same reason. `SHOW COUNT(*) WARNINGS` still checks that the group gave exactly one warning, which is what the case is there to show. Only case 5 needs it. The other cut warnings in this test come from `group_concat_max_len` on a handful of rows, and their row numbers do not depend on the width of a pointer. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40739 Server crashes in `spider_db_open_item_field` `spider_db_open_item_field()` looks a field's table up among the Spider tables of the query whenever the field does not belong to an internal temporary table: if (field->table->s->tmp_table != INTERNAL_TMP_TABLE) That is a hand-rolled copy of the server's own `TABLE_SHARE::is_optimizer_tmp_table()` predicate. Temporary tables created by `Create_tmp_table` are marked `RESULT_TMP_TABLE` rather than `INTERNAL_TMP_TABLE`, so a field of such a table passes the test, `spider_fields::find_table()` finds no holder for it, and the returned `NULL` is dereferenced. Only the second pass crashes. The first pass, which decides whether the group by handler can be created at all, does guard against a `NULL` holder. The two passes do not resolve to the same items, though: an `Item_direct_ref` is followed through `real_item()`, and between optimization and execution it is re-pointed at a field of the optimizer's result temporary table. Ask the server's accessor instead of restating it, so that the predicate keeps following the server's definition of an optimizer temporary table. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
chanztuying
ztuying.chan@ed.ac.uk |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MDEV-40827: make the MHNSW prefetch portable to MSVC | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40378 heap: roll back key changes when a blob write fails in `heap_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: widen the `err:` recovery to run for `HA_ERR_RECORD_FILE_FULL`, `HA_ERR_OUT_OF_MEM` and `ENOMEM` as well, so a failure raised after the key loop also moves every changed key back to its old value. One recovery path now serves every failure that leaves keys moved to their new values, including any future error source in the key loop itself. The `err:` block assumed the failure happened **inside** the key loop, so that `keydef` addresses the partially processed keydef. A blob-chain write fails after that loop has run to completion, and therefore arrives with `keydef == keydef_end`. Reading `info->errkey` and `keydef->algorithm` from there addresses `share->keydef[share->keys]`; as `sizeof(HP_KEYDEF)` (888) far exceeds the key segments and blob descriptors that follow the keydef array, that read runs past the end of the `HP_SHARE` allocation, and the rollback sweep then dereferences a garbage `keydef->seg` in `hp_rec_key_cmp()`. So the recovery distinguishes the two failure sites: with `keydef == keydef_end` there is no partly updated key to repair and none to name in `info->errkey`, and the sweep starts at the last keydef instead. The same branch also covers a table with no keys at all (`share->keys == 0`), where the sweep has nothing to do. `info->errkey` is initialized to `-1` on entry to `err:`, so a failure that is not a key error can never expose a stale key number from an earlier operation. The original errno is captured before the recovery 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 it. 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), a partial multi-row UPDATE, and a table with no indexes at all; each asserts the table stays consistent after the failure via `CHECK TABLE` and index lookups. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40920 Say whether a value cut in a group reached the answer A TEXT value longer than `group_concat_max_len` is cut on its way into `blob_storage`, in `Field_blob::handle_group_concat()`. That happens while the group is being built, not when the answer is put together, so whether the answer is any shorter for it depends on whether the row carrying the value reaches the answer at all. Every such cut was reported as `ER_CUT_VALUE_GROUP_CONCAT`, which says that the answer lost something the user asked for, and that is true of only some of them. `Blob_mem_storage` now writes one byte in front of every value it stores and returns the pointer past it, so `was_cut()` answers for any value a reader holds a pointer to. `Field_blob::store()` sends every blob of a table that has a `Blob_mem_storage` through `handle_group_concat()`, so no value in that storage is without the byte, and `Field_blob::get_ptr()` on the record of a row hands back exactly the pointer that was stored. `dump_leaf_key()` reads the mark off each row it appends and sets `value_cut_in_result`. `val_str()` then reports: 1. **A warning**, `ER_CUT_VALUE_GROUP_CONCAT`, when a row that reached the answer carried a cut value. The answer is short by what was cut. The result being cut at `gconcat_max_len()` already gives that same warning, and a group that hits both is told once, not twice. 2. **A note**, `ER_CUT_VALUES_WHILE_PROCESSING`, when a value was cut but no row carrying one reached the answer. The answer may well be what a larger limit would have given. One note per aggregate is enough for a statement however many groups had a value cut, and `cleanup()` clears the mark so a statement run again gets its own. Reporting the loss as a warning keeps a strict `sql_mode` aborting on it, which it does because `THD::raise_condition()` promotes a warning and never promotes a note. `ST_COLLECT` is not affected. It reports `ER_CUT_VALUE_GROUP_CONCAT` itself, against `group_collect_max_len`. `main.gconcat_cut_note` covers the split with one group holding a short value and a long one, where a `LIMIT` alone decides which of them the answer is built from, over both the sort tree and the duplicate filter. `main.func_gconcat` shows the granularity: of five groups at `group_concat_max_len=499999`, the one holding exactly 499999 bytes is the one that does not warn. Note that `blob_storage` only exists when the aggregate has an `ORDER BY` or a `DISTINCT` and a blob field, so this is the only shape in which a value is cut this way. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Tarun Wadhwa
tarunwadhwa85@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-37605: Extend mariadb-binlog to convert InnoDB-based binlogs to legacy format MariaDB 12.3 added support for storing binary logs inside InnoDB tablespace files (.ibb) instead of traditional flat sequential files. There was previously no way to convert these back to the legacy format, which is needed for downgrades, migrations, and third-party tooling that only understands classic binlogs. This adds a --convert-engine-binlog option to mariadb-binlog that reads one or more InnoDB-based binlogs and writes out equivalent legacy-format binlog files. Usage: mariadb-binlog --convert-engine-binlog -r output_legacy_binlog \ input1.ibb input2.ibb This produces output_legacy_binlog.0000001, .0000002, etc. Note there isn't a direct correlation between the number of input files and output files. The output is split whenever the current file hits --max-binlog-size (default 1GB, since legacy binlogs cannot exceed 4GB), and also on every server restart recorded in the InnoDB binlog, so a single input file can produce several output files and a restart boundary can force a split even before the size limit is reached. Additionally, --max-binlog-size is also supported as a standalone option to control this rotation threshold directly. Only --convert-engine-binlog, -r, and --max-binlog-size are supported alongside this mode; any other option will cause mariadb-binlog to throw an error. Caveats: - Encryption: InnoDB-based binlogs don't currently support encryption, so the converter doesn't handle it either. Output legacy binlogs are never encrypted by this mode. - Checksums: InnoDB-based binlogs rely on page-level checksums instead of the legacy per-event checksums, so the initial implementation generates legacy output with no event-level checksum. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Monty
monty@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Fixed internal temporary buffer sizes to use tmp_memory_table_size tmp_memory_table_size is limiting the size of internal temporary memory tables. max_heap_table_size is there to limiting the size of explictely created memory tables. max_heap_table_size can be much larger than tmp_memory_table_size as the memory used by temporary tables is in the control of the user. This commit changes the usage of max_heap_table_size for internal buffers to min(max_heap_table_size, tmp_memory_table_size), like we do for internal temporary tables. This changes the in memory buffer allocations for: - GROUP_CONCAT() - Calculating the cost for scanning memory tables (the original code was wrong here as it used the wrong size for memory tables). - ANALYZE TABLE buffer sizes for calculating distinct column values Other things: - Add THD::ram_limitation() to provide consistent memory limitations in all code that used variables.tmp_memory_table_size as buffers. If tmp_memory_table_size == 0, then 8192 is used. This replaces Item_sum::ram_limitation which used 1024 as min buffer, which is way to little for any practical case. - Added security guard in heap_prepare_hp_create_info to ensure that max_table_size is calculated same way as in MariaDB server. - Fixed initial memory allocations for Item_func_group::concat which allocated 'max allowed memory' at start. Now it allocates only 1/16 of that memory at start. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix compilation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39074 trans_rollback_stmt(THD *): Assertion `! thd->in_sub_stmt' failed. 1. Row-based events work for already locked tables. slave_close_thread_tables() unconditionally called trans_commit_stmt()/trans_rollback_stmt(), both of which cannot be done under sub-statment. In test case a BINLOG statement with malformed base64 payload executed from an AFTER INSERT trigger hit this: mysql_client_binlog_statement() sets thd->is_error() on decode failure and calls slave_close_thread_tables(), which then asserted since the trigger body runs as a sub-statement. The fix guards the commit/rollback with !thd->in_sub_stmt, matching the same idiom already used in mysql_execute_command() and open_and_lock_tables() for the same reason: sub-statements defer statement-transaction finalization to the enclosing top-level statement. Other callers of slave_close_thread_tables() run only from the top-level SQL slave applier thread, so this doesn't change their behavior. rows_event_stmt_cleanup() has its own trans_commit_stmt()/ trans_rollback_stmt() call after applying a row event; guard it the same way. Rows_log_event::do_apply_event() opens its target tables via a one-shot "if (!thd->lock)" check that only fires at the top of a fresh statement. Inside a trigger, thd->lock already belongs to the enclosing DML, so the table is never opened, leaving a NULL TABLE* used further down. Look it up among the enclosing statement's already open tables (find_locked_table()) instead, and raise ER_TABLE_NOT_LOCKED if not found -- the same error a normal trigger-body statement gets for referencing an unprelocked table. 2. Statement-based events are disabled. A BINLOG statement decoding to a Query_log_event, executed from within a trigger or stored routine, cannot be made to work easily. DML for statement event in trigger cannot be done for new tables because the locking must be done at once and it cannot be done for query tables because of ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG. - Query_log_event::do_apply_event() runs the embedded query via mysql_parse(), which assumes it starts a genuinely new top-level statement (THD::reset_for_next_command() asserts !spcont/!in_sub_stmt and documents itself as "not called by substatements of routines"; lex_start() reinitializes thd->main_lex, which the general BINLOG-statement code reuses as scratch space on the assumption that it is idle -- false while the enclosing statement is still running). This part is fixable: parse into a private LEX and a private Query_arena instead of going through mysql_parse(), the same way mysql_make_view() parses embedded SQL text mid-statement. - What isn't easily fixable is resolving the tables the embedded query references. Prelocking is computed statically from the trigger body's own SQL text; a table name decoded at runtime from a BINLOG payload can never be part of it. open_table(), in prelocked mode, only treats an already-open table as reusable when its query_id is 0 (free) -- a table still owned by the not-yet-finished enclosing statement is refused (ER_NO_SUCH_TABLE), and lock_tables() applies the same kind of check for a table the trigger writes back into (ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG). Bypassing that check (reusing the same TABLE object instead of a second, prelocked instance) removes the very guard that stops a trigger recursing into itself: a trigger on t1 whose BINLOG payload inserts into t1 recursed without limit, surfacing not as controlled recursion but as save_restore_context_apply_event()'s "!rli->mi" assert, since that scratch slot on the shared, fake rli is not reentrant. Refuse the combination outright instead: Query_log_event::do_apply_event() now raises ER_SP_BADSTATEMENT when thd->in_sub_stmt, before touching any THD state. 3. Query_log_event PS execute leak fix A BINLOG statement decoding to a Query_log_event, executed via PREPARE/EXECUTE, leaked its nested query's allocations onto the PS's own persistent arena: thd->stmt_arena pointed at it while mysql_parse() ran the decoded query. Second EXECUTE asserted on ROOT_FLAG_READ_ONLY, since PROTECT_STATEMENT_MEMROOT marks that arena read-only after a successful execution. The fix redirects thd->stmt_arena to thd itself for the duration of the nested mysql_parse() call, so stmt_arena->is_conventional() reads true and activate_stmt_arena_if_needed() (called e.g. from save_leaf_tables()) never redirects allocations to the PS's arena in the first place. Harmless for what it's protecting: leaf_tables_exec is normally cached on the persistent arena so a repeatedly-executed statement's SELECT_LEX doesn't rebuild it every time, but our SELECT_LEX is torn down and reparsed fresh (due to mysql_parse() semantics) on every EXECUTE, so there's nothing to cache here regardless of which arena is used. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix compilation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40591 Unexpected ER_NOT_KEYFILE or MSAN error in heap_check_heap `ha_heap::external_lock()` verifies the table with `heap_check_heap()` at `F_UNLCK`. That is safe on the ordinary unlock path, where `mysql_unlock_tables()` calls `unlock_external()` before `thr_multi_unlock()` and the lock is still held. It is not safe on either path that unlocks after a *failed* lock attempt, where the caller holds nothing at all while another connection is writing: 1. `mysql_lock_tables()` calls `unlock_external()` to balance the external locks it already took, because `thr_multi_lock()` timed out. 2. `lock_external()` unwinds the tables it has already locked, because a later table refused -- all before `thr_multi_lock()` runs at all. `ha_partition::external_lock()` unwinds its partitions the same way. MEMORY has no row-level concurrency control, so a scan taken outside the lock sees a writer's intermediate state by construction: `hp_alloc_from_tail()` publishes `total_records` at allocation time, before the slot is written, while the checker scans `[0, total_records + deleted)` and reads every slot's flags byte. Under MSAN that is a use of uninitialised `my_malloc()` memory; otherwise it is a spurious `total_records` mismatch. `heap_check_heap()` ends with `heap_mark_crashed()`, which sets `HEAP_STATE_CRASHED` in the **shared** `HP_SHARE`, so one bogus mid-write observation poisons a healthy table for every connection using it -- the reported `ER_NOT_KEYFILE`. MDEV-21373 disabled this check in 2021 for exactly this reason, by gating it on `EXTRA_DEBUG`. MDEV-38975 changed the gate to `EXTRA_HEAP_DEBUG` and defined that for every debug build, reviving the race. Rather than switch the check off wholesale again, only verify a table that this handle both holds a lock on and has changed under it: - `HP_INFO::lock_type` remembers the `ha_heap::external_lock()` argument, the way `MARIA_HA` and `MI_INFO` already do; - `HP_INFO::changed` is set by `heap_write()`, `heap_update()` and `heap_delete()`, and cleared by `ha_heap::external_lock()` on every grant, so it means "changed since this lock was taken"; - `table_is_locked_and_changed()` requires both. The change term is what separates the three unlock paths, because the lock type cannot: `ha_heap::external_lock()` records it before `thr_multi_lock()` runs, so it is armed on the two failing paths as well. Neither of them ever ran a row operation, so neither has changed anything. It has to be per handle rather than `HP_SHARE::changed`, which is true on exactly those paths, another connection being the one writing. Requiring a change also makes a debug build cheaper: the verification scans every record and every index, and now runs only after a statement that wrote to the table. Deriving this in the engine rather than repairing `lock_external()` also covers `ha_partition`, which reimplements the same unwind. A temporary table gets `F_EXTRA_LCK` and so counts as always locked: no other connection can reach its share. This covers the user's `CREATE TEMPORARY TABLE` and not only the optimizer's internal one -- an internal table frees its blob chains outright, whereas a user temporary table parks them, and `get_lock_data()` leaves it out of the lock set entirely, so it never reaches `external_lock()` at all. The `ALTER` copy target is temporary too, and additionally takes a direct `handler::ha_external_lock()` instead of going through the lock set. Redeeming a parked blob chain puts records back on the shared free list, so it needs the same protection, and both redemption points assert it. `hp_test_unlock_check-t` builds the lock states directly, in the order `ha_heap::external_lock()` builds them, so nothing here is raced. Four MTR tests cover the shapes it cannot reach: blob updates and deletes on a user `TEMPORARY` MEMORY table (`heap.blob_tmp_table`), `INSERT DELAYED` (`heap.blob_delayed_insert`), the `ALTER` copy target (`heap.blob_online_alter`), and one share locked twice in a lock set (`heap.blob_lock_twice`). No existing test exercised any of them. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
drrtuy
drrtuy@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| feat: MDEV-40672 implement basic support for the pluggable aggregate functions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40448 HEAP unique hash duplicate rejection re-hashes the full key 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. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Monty
monty@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Limit the memory used by GROUP_CONCAT() with ORDER BY GROUP_CONCAT() with ORDER BY collects all rows of the group in a TREE and only cuts it down in repack_tree(). The repack was triggered by (tree_len >> GCONCAT_REPACK_FACTOR) > thd->gconcat_max_len() with GCONCAT_REPACK_FACTOR 10, that is when the rows in the tree had produced 1024 * group_concat_max_len bytes, or 1G with the default settings. On top of that tree_len only counted the length of the strings, while the tree costs sizeof(TREE_ELEMENT) + reclength per row. For GROUP_CONCAT(int_col ORDER BY int_col) that is about 40 bytes per row against 6 bytes of result, so the tree had grown to several GB before the first repack. In practice the server ran out of memory first and the repack code was close to never used. The tree is now limited by the memory it has really allocated, tree->allocated, instead of by the length of the strings it holds. The limit is MY_MAX(thd->ram_limitation(), thd->gconcat_max_len()) and is never set so low that the tree can not hold a few rows. repack_tree() builds a new tree while the old one is still in memory, so the peak usage is the size we start the repack at plus the size we copy to. To keep the sum within the limit it is split into GCONCAT_TREE_PARTS parts; the repack starts when GCONCAT_TREE_REPACK_PARTS of them are used and copies to the remaining part. The part we do not copy to is also the room the tree has to grow before the next repack, which keeps the repacks amortized. Other changes: - tree_len is removed. It was only read by the old trigger. - repack_tree() decided that it had run out of memory by testing st.len <= st.maxlen after the walk. That test was only valid because the old trigger guaranteed that a complete copy had to overshoot st.maxlen. A repack triggered by memory can complete the walk with st.len far below st.maxlen, which would have failed the query with a wrong out of memory error. There is now an explicit flag for it. - The length that decides which rows to keep now also counts the separator that is put between two rows, so that it matches what val_str() will produce. - When the memory limit stops the copy, the result becomes shorter than group_concat_max_len. dump_leaf_key() can not detect this, as the result never reaches the maximum length. This is now remembered in result_cut and reported to the user. - All cut value reporting is moved to val_str(); dump_leaf_key() only marks that the result was cut. This removes the need to clear the truncated flag of table->blob_storage to avoid a duplicated warning, and gives one warning per group also when val_str() is called more than once for the same group, which repeated the warning before. - Added a function comment for repack_tree() that describes where the rows are cut away and why building a copy frees memory. Co-author: Arcadiy Ivanov <[email protected]> - Fixed a bug in copy_tree() |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Lena Voytek
lena@voytek.dev |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40964: Update default datadir for debian to be /var/lib/mariadb Match the existing datadir setting for Debian and Ubuntu, which was changed from /var/lib/mysql to /var/lib/mariadb in 1:11.8.6-5. This includes updating debian/ packaging to use /var/lib/mariadb for new installs, and maintaining /var/lib/mysql for legacy installs by providing 99-legacy-datadir.cnf. Likewise, update the cmake INSTALL_MYSQLDATADIR_DEB variable to reflect the change. The debian flag-based version check system has also been replaced as it is no longer needed. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix temporary table BLOBs results | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40634 Const MEMORY table's BLOB outlives the lock protecting it A single-row table is read once during optimization and its row kept in `record[0]` for the rest of the statement. `JOIN::optimize_stage2()` then releases the lock on every const table, on the premise stated in its own comment: *"It's safe to ignore result code as all tables where opened for read only."* That premise assumes a read leaves a **copy** of the row behind. MEMORY with a blob does not. `hp_read_blobs()` answers the read by pointing `record[0]` at the blob data inside `HP_SHARE` rather than copying it, so from the moment the lock is dropped another connection is free to overwrite, free or recycle those bytes -- and the statement goes on reading them. The result is a const table whose value changes in the middle of the statement using it, and a read of freed memory. Let a caller that keeps reading a row after the unlock ask for such tables to be left alone. `GET_LOCK_SKIP_ZERO_COPY_ROWS` drops them from the lock set `get_lock_data()` builds, exactly as `GET_LOCK_SKIP_SEQUENCES` already does, and the const-table unlock in `JOIN::optimize_stage2()` passes it. Which tables those are is for the engine to say rather than for the lock layer to infer. The 64-bit `table_flags()` space is full, so a second word `table_flags2()` carries the first such property, `HA2_CANNOT_ACCESS_ROWDATA_AFTER_UNLOCK`, and `ha_heap::open()` raises it for any table that has a blob. The skip has to be opt-in rather than a rule. `mysql_lock_remove()` also reaches `mysql_unlock_some_tables()`, and there the unlock is permanent and must not be skipped. A MEMORY blob const table now stays read-locked for the whole statement and blocks writers, which is the price every non-const MEMORY table already pays. The regression test parks the reader with `GET_LOCK()` rather than with a stored function. A stored function puts the statement into prelocked mode, and the const-table unlock is skipped entirely in that mode, so the code path under test would never run. The gate that parks it is taken with `--disable_ps2_protocol` in force. `--ps-protocol` executes every complete `SELECT` twice and compares the two result sets, and `GET_LOCK()` is recursive, so a doubled acquisition would outlive the single `RELEASE_LOCK()` that opens the gate again. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix tests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Monty
monty@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fixup! db1d6d824d96694acaa9e715836e43562a718e15 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40692 GROUP_CONCAT replays a group when an OFFSET skips every row Nothing says how many times a statement asks for the result of a group, and the answer must not depend on it. A `HAVING` clause on the alias is the shortest statement that asks twice, and it returns a different value than the same aggregate asked once: SELECT GROUP_CONCAT(a ORDER BY a LIMIT 2 OFFSET 4) v FROM t1; -> (empty) SELECT GROUP_CONCAT(a ORDER BY a LIMIT 2 OFFSET 4) v FROM t1 HAVING v LIKE '%'; -> a,b `val_str()` walks only while `result_finalized` is false, and `dump_leaf_key()` raises that flag for the first row it writes. A row that falls inside the offset is skipped by an earlier return, which decrements the offset counter and leaves the flag alone. The row-limit arm immediately above it does raise the flag before its own early return, so two adjacent early returns behave differently. A walk in which every row was skipped therefore writes nothing and records nothing. The next caller walks again with the offset already spent, and the rows skipped the first time are appended to a result buffer that was handed over once already. Once the duplicate filter has spilled to disk the second walk is worse than wrong. `Unique::reset()` documents the contract: Clear the tree and the file. You must call reset() if you want to reuse Unique after walk(). The first walk flushed the tree and emptied it, so the second flushes an empty tree, appending a chunk that holds no rows. `merge_walk()` reads nothing back from it and fails `DBUG_ASSERT(bytes_read)`. A build without assertions goes on to take keys from that chunk. Set `result_finalized` where the walk block ends, so that it records every path that has consumed the filter rather than only the paths that wrote a row. On the release branches only the form without `ORDER BY` reaches the duplicate filter. Since MDEV-21879 the `DISTINCT ... ORDER BY` combination builds its result the same way, so both forms can reach the assertion here. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40946 Two heap blob tests fail with the embedded server Both tests exercise server facilities that an embedded build does not have, so neither can run there. `INSERT DELAYED` has no delayed insert thread in an embedded build. The whole facility sits inside `#ifndef EMBEDDED_LIBRARY`, including the check that routes a delayed statement away from the ordinary insert path, so the statement is an ordinary insert and `DELAYED_WRITES` stays at 0. `ALTER TABLE ... LOCK=NONE` is never online in an embedded build. `online` is hard-wired to `false` when `HAVE_REPLICATION` is undefined, and `my_global.h` leaves it undefined for `EMBEDDED_LIBRARY`. The source lock is therefore not downgraded, the `alter_table_online_downgraded` sync point is never reached, and the test's `WAIT_FOR downgraded` runs out its `debug_sync` timeout. Skip both with `include/not_embedded.inc`, as `main.delayed` and `main.alter_table_online_debug` already do. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Fix the `HA_NO_KEY_READ` blob key guard `HA_NO_KEY_READ` marks a key whose blob segment `heap_prepare_hp_create_info()` converted from the VARTEXT2 form, so that `heap_rkey()` refuses an index read on it. It never worked, because the mark was written to the wrong structure member. `heap_rkey()` tests `HP_KEYDEF::flag`, which is where the flag belongs: `HA_NO_KEY_READ` is declared among the key flags, not the key-seg flags. The assignment instead targeted `HA_KEYSEG::flag`, which no reader consults for this flag, so the guard could never fire. That member is also `uint16`, so bit 20 was discarded on assignment as well; `-Wall -Wextra` does not warn, only `-Wconversion` does, and it is not enabled. Write the flag to `keydef[key].flag` instead. `HP_KEYDEF::flag` is `uint` and holds bit 20, and `heap_create()` copies it into the share that `heap_rkey()` reads. `hp_test_key_setup-t` covers the marking, the unmarked case, that `heap_create()` does not lose the flag while folding its own bits into `keydef->flag`, and that `heap_rkey()` refuses a marked key while accepting an unmarked one. The last pair clears `my_assert` so the guard reports instead of aborting, the same way the server's `--debug-assert=0` does, and skips on builds without `DBUG_ASSERT`. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Golubchik
serg@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
misc fixes 1. set PLUGIN_HEX_VERSION correctly for bundled plugins 2. don't add GenError dependency for external plugins 3. don't change the policy globally 4. only do EXTERNAL_PLUGIN_POST() if EXTERNAL_PLUGIN_PRE() was done |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41007 Warn when GROUP_CONCAT(DISTINCT) loses rows silently Give a warning when `GROUP_CONCAT(DISTINCT x)` or `JSON_ARRAYAGG(DISTINCT x)` returns only part of a group, or nothing at all, because the walk of the duplicate filter failed. The result is wrong rather than deliberately cut, and nothing was said about it. Both build their result in `val_str()` by walking `unique_filter`, and threw the walk's return value away. `Unique::walk()` reports its own failures through it, from allocating the merge buffer to reading back the chunks it merged, so a failure gave a short result, or an empty one, in silence. The return value cannot be used on its own. `dump_leaf_key()` also stops the walk, for two reasons that are not failures: it cuts the result at `group_concat_max_len`, which it already reports by setting `result_cut`, and it stops without losing anything once the `LIMIT` is used up. Reporting every non-zero return as a cut warns about `GROUP_CONCAT(DISTINCT a LIMIT 5)` returning exactly the five rows that were asked for. `dump_leaf_key()` now records that it was the one that stopped the walk, so `val_str()` warns only when the walk itself failed. The warning is a new one. `ER_CUT_VALUE_GROUP_CONCAT` reports the row the result was cut at, and there is no such row here: the walk failed before it delivered anything, and how much was lost is not known, so it would read `Row 0 was cut by group_concat()`. `ER_RESULT_CUT_BY_LIMIT` says that the result was cut and names the limit that cut it, which is the memory `Unique` was given to work in: the smaller of `tmp_memory_table_size` and `max_heap_table_size`. Not every failure is silent either. The merge buffer is allocated with `MY_WME` and the spill file is opened with `MY_WME`, so running out of memory or failing to read raises an error of its own. Only the guard at the top of `merge_walk()`, which refuses a merge buffer too small to hold one key per chunk, returns without saying anything. Warn only when no error was raised: where one was, the user has been told and the statement is failing, so describing the length of a result nobody will see adds nothing. The debug keyword `unique_walk_merge_fail` fails the merging walk quietly and `unique_walk_merge_error` fails it with an error raised. `main.gconcat_distinct_walk_fail` uses both. The `LIMIT` case needs no debug build and is checked in `main.gconcat_distinct_spill`. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Golubchik
serg@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
misc fixes 1. set PLUGIN_HEX_VERSION correctly for bundled plugins 2. don't add GenError dependency for external plugins 3. don't change the policy globally |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Golubchik
serg@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MDEV-40406 hide #mysql50# under old mode | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40669 HEAP MIN_ROWS pre-sizes blocks past max_heap_table_size `init_block()` captures `requested_min_records` from the caller's `min_records` before the `min_records= MY_MIN(min_records, max_records)` clamp, then restores that raw value when the block allocation cap added by MDEV-40447 fires. `max_records` is derived from `max_heap_table_size` / `tmp_memory_table_size` and is the most rows the table can ever hold, so that clamp is what has always bounded a HEAP table's allocations. Restoring the pre-clamp value undoes it, and an unreachable `MIN_ROWS` pre-sizes the record block and every hash key block past the ceiling. With `max_heap_table_size=64M` and `MIN_ROWS=20000000` a one-row table allocates 1GB where it used to allocate 96MB; `MIN_ROWS=4294967295` at the shipped default 16MB ceiling allocates 4GB, bounded only by the `INT_MAX32` clamp on `memory_needed`. A few such tables exhaust memory. Fix: clamp `requested_min_records` to `max_records` as well. `MY_MIN` keeps 0 at 0, so "no `min_records` requested" stays distinguishable from an explicit `MIN_ROWS`, and the cap keeps ignoring the defaulted 1000-row heuristic. The cap and the clamp together give four regimes, and only the last one changes: 1. No `MIN_ROWS`: capped, sizing comes from the ceiling alone. 2. `MIN_ROWS` below the cap: capped. 3. `MIN_ROWS` above the cap but within `max_records`: pre-sizes to `MIN_ROWS`, past the cap, as MDEV-40447 intends. 4. `MIN_ROWS` at or above `max_records`: unreachable, so it degrades to plain ceiling-derived sizing. In case 4 the cap branch becomes a no-op, because `records_in_block` already equals `max_records`, so sizing returns to exactly what it was before MDEV-40447. `init_block()` no longer defaults `max_records` itself. The block sizing ceiling is derived once in `heap_create()` and the parameter is `const`, so the caller's value reaches `share->max_records` unchanged: 0 there means "no row limit", and `hp_alloc_from_tail()` skips the limit check only while it is 0. Tests: - `storage/heap/hp_test_block_size-t.c`: a four-case boundary walk asserting the exact `alloc_size` of each regime above at one ceiling-derived `max_records`, and a keyed case asserting that an unreachable `MIN_ROWS` clamps the hash key block as well as the record block (`sizeof(HASH_INFO)` gives that block its own `recbuffer` and its own cap). That one expectation selects on `SIZEOF_CHARP`: `sizeof(HASH_INFO)` is 24 on LP64 and 12 on ILP32, which halves `memory_needed` and rounds a whole power of two lower. The record-block sizes round the same on both widths. A `max_records=0` case covers the derived ceiling: the block is sized from it while `share->max_records` stays 0, and the table accepts far more rows than that default. - `mysql-test/suite/heap/min_rows_alloc.test`: the same regimes end to end across three ceilings, plus `MIN_ROWS` at the .frm maximum under the shipped default ceiling. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Lena Voytek
lena@voytek.dev |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Update default rundatadir for debian to be /run/mariadbd Move mariadbd's runtime data directory in Debian from /run/mysqld to /run/mariadbd. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
chanztuying
ztuying.chan@ed.ac.uk |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40827: prefetch MHNSW neighbours during search Issue prefetches for unseen neighbour nodes before evaluating their distances. This overlaps later memory loads with distance calculations for earlier lanes without changing the search result. On a fixed 200k by 1024-dimensional cosine graph with M=6, a crossed AB/BA warm-cache run (30 paired observations) improved paired median QPS by 10.1% at ef_search=40 (95% CI 8.0%-14.4%) and 13.8% at ef_search=160 (95% CI 11.8%-20.0%), with identical exact-recall means. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41020 A `MEMORY` table refuses a row it just held `hp_alloc_from_tail()` tests the table memory ceiling before it decides whether the leaf it is about to use has to be allocated or is merely being reclaimed. Reclaiming adds no memory, so on that branch the test answers a question nobody asked. The state it misjudges is routine. `hp_find_free_hash()` allocates an index leaf without consulting `max_table_size`, and no leaf is smaller than `heap_min_allocation_block`, so a table with two hash indexes is already over a 32K ceiling once it holds its first row. That is legal: the ceiling is only tested when the record cursor lands on a leaf boundary, and the first row tests it while both counters are still zero. `hp_shrink_tail()` puts the cursor back on a leaf boundary whenever it empties the tail, and `data_length` goes on counting the leaf, which is still allocated. The next write re-reads that sum and reports `HA_ERR_RECORD_FILE_FULL` for a row the table held a moment earlier. Move the ceiling test to the arm that calls `hp_get_new_block()`. The `max_records` row-count test stays where it is, because it caps rows however the slot is obtained. **Why a master and a slave disagreed about the same statement.** `REPLACE INTO t SELECT * FROM t` feeds the row back out of the table, so the record buffer still points into the chain that the delete parked; the chain is adopted rather than freed and the cursor never moves. The slave builds the row from the replication event buffer, nothing points into the parked chain, it is freed, the tail empties and the write is refused. Replication is one way to reach the state, not the cause: a targeted `DELETE` and a re-`INSERT` on a single server reach it too. Tests: `heap.blob_delete_reinsert_ceiling` covers the single-server route and checks that a table that genuinely needs more memory is still refused; `heap.blob_replace_repl_ceiling` covers the reported master/slave divergence; `hp_test_freelist` test 22 pins the branch itself. Each fails without the change. **Storing the row limit as a ceiling.** `heap_create()` recorded its `max_records` argument in the share unchanged, so 0 arrived there still meaning "no limit" and every reader had to special-case it. The row-count test above carried that as a second condition, `&& info->max_records`. Write `NO_LIMIT_RECORDS` into the share instead. The share then always holds a real ceiling and the write path tests one value. That frees 0 to mean on the share what it says, a table that accepts no rows, which is the natural way to create one that is never written to; only `heap_create()`'s argument keeps using 0 for "no limit". `ha_heap::info()` multiplies `max_records` by the record length to report `MAX_DATA_LENGTH`. An unlimited table now reports `~(my_off_t) 0` rather than a product that would overflow. Nothing reaches that branch through SQL, because `ha_heap::create()` derives `max_records` from the memory ceiling and a `MAX_ROWS` clause only lowers it. Measured before and after, the reported `MAX_DATA_LENGTH` is unchanged for a plain `MEMORY` table and for one created with `MAX_ROWS`. The unit tests wrote the old convention themselves and move with it: `hp_test_freelist` assigned `share->max_records= 0` to lift the limit, which now reads as a limit of zero rows, and `hp_test_block_size` asserted the share kept the 0. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
fixup! MDEV-40802 COUNT(DISTINCT <blob>) fails when its tmp table converts `heap.count_distinct_blob_convert` asserts that four of its aggregates converted their in-memory temporary table to the on-disk engine. On a 32-bit build three of those assertions read `OFF`: the table never filled up, so nothing was converted and those cases covered nothing. The test filled a 64 KB in-memory table with 2000 rows, 1000 of them distinct. On a 64-bit build the table overflows somewhere between 1500 and 2000 rows, so 2000 clears the limit by only a few hundred. A record holding a blob carries a pointer to the value, and where a pointer is four bytes rather than eight the record is narrower, so the same 2000 rows no longer reach the limit. Use 8000 rows, 4000 distinct, several times what the wider build needs. Only the count is chosen that way. The value widths and the memory limit stay exactly where they were measured, because they are what makes the table run out of record slots rather than out of blob space. That is the case where the row left over at the overflow is a duplicate of one already copied, and handling that duplicate is what the fix is about; a limit or a width that overflows on a blob value converts the table while covering nothing. Rows added past the overflow cannot move it - it happens once the table is full, whatever follows - so the 64-bit build converts on the same row as before. Verified by backing the fix out. With `ignore_last_dupp_key_error` returned to 0 the test fails with `ER_DUP_UNIQUE` at 8000 rows exactly as it did at 2000. No recorded `CONVERTED` line changed; only the row and distinct counts did. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-21879 GROUP_CONCAT(DISTINCT ORDER BY) is wrong when Unique spills `Item_func_group_concat::add()` decided whether a row was a duplicate by checking whether `Unique::elements_in_tree()` had grown after `unique_add()`: uint count= unique_filter->elements_in_tree(); unique_filter->unique_add(get_record_pointer()); if (count == unique_filter->elements_in_tree()) row_eligible= FALSE; `Unique` flushes its whole in-memory tree to disk when it runs out of memory, and `elements_in_tree()` only counts what is still in memory. After the first flush the test says nothing about the rows that were already spilled. **MDEV-11563** made this harmless for `GROUP_CONCAT(DISTINCT x)` by building the result in `val_str()` from `unique_filter->walk()`, which merges the spilled parts back in. It left the `ORDER BY` case alone. There the result comes from the sort tree, which `add()` fills gated by `row_eligible`, so the defect is still fully live. Both directions of the failure are reachable, depending on how often the filter flushes relative to the insert: 1. Duplicates reach the result. 100 rows holding 50 distinct values give all 100 values back. 2. Rows are lost. 30 distinct rows of 2000 bytes give one value back. `JSON_ARRAYAGG(DISTINCT x ORDER BY y)` fails in the same way. Fixed by not filling the sort tree from `add()` when `DISTINCT` is used. `val_str()` now walks the merged `unique_filter` into the sort tree and then walks the sort tree, so the rows are sorted after the duplicate filtering is complete instead of during it. `Unique::walk()` merges everything it flushed, so the sort tree can be handed more rows than fit in memory. `insert_to_order_tree()` repacks it on the same memory budget `add()` used, and a walk that runs out of memory sets `result_cut`, so the user gets a cut value warning rather than a silently short result. **Behaviour change.** `ORDER BY` does not order rows that tie on the ordering expression, and which of them comes first changes here. It used to follow the order the rows were read in; it now follows the order the duplicate filter keeps them in. Unlike the old order, the new one depends on neither the memory available nor the physical row order. `main.gconcat_distinct_spill` checks that, and `main.func_gconcat` records one such tie. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Monty
monty@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Trivial optimziations for group_concat - Remove some if - Reorder code - More code comments (cherry picked from commit dc6a897961c311f981b150e4207ffc1390a219ef) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40431 heap: classify rb-tree insert failures, mark the table crashed when key recovery fails `hp_rb_write_key()` reported **every** rejected `tree_insert()` as `HA_ERR_FOUND_DUPP_KEY`, although `tree_insert()` also returns NULL when the allocation of the tree node fails. An out of memory during a BTREE-indexed UPDATE was therefore reported as a duplicate: the user got `ER_DUP_ENTRY` with a fabricated value, possibly naming a key that is not unique at all, `handler::is_fatal_error()` treated the allocation failure as not fatal for callers asking for `HA_CHECK_DUP_KEY`, and the `HA_ERR_OUT_OF_MEM` / `ENOMEM` arms of the `heap_update()` recovery list were unreachable on the rb-tree path. `tree_insert()` now records why it returned NULL in the new `TREE::error` (`TREE_ERROR_OOM` or `TREE_ERROR_DUP_KEY`), and `hp_rb_write_key()` maps that to `HA_ERR_OUT_OF_MEM` or `HA_ERR_FOUND_DUPP_KEY` (`HA_ERR_INTERNAL_ERROR` defensively, should a new NULL source appear). With the classification corrected, the allocation failure lands in the `HA_ERR_OUT_OF_MEM` arm of the `heap_update()` recovery, which moves the already changed keys back, so correcting the error report does not trade the wrong message for a silently corrupt index. The recovery at `err:` keeps its explicit list of error codes: those are the errors we know how to recover from, and recovering from errors whose meaning we do not know is worse than stopping. What changes around it: 1. `info->errkey` is only set for `HA_ERR_FOUND_DUPP_KEY`, the single error it describes; every other failure leaves the `-1` that `err:` starts with. 2. When the recovery itself cannot restore a key -- the re-insert of an old key value fails -- or the failure is outside the list, the index no longer describes the data and the table is marked **crashed** in the new `HP_SHARE::state_changed` (bits and macros modeled on the `state.changed` of Maria, see `storage/maria/maria_def.h`). A crashed table refuses every lock acquisition, read and write with `HA_ERR_CRASHED` ("Index for table is corrupt"). `heap_check_heap()` does not trust the mark: it clears it, re-validates the whole structure and marks the table crashed again when it finds damage, so a check of an intact table drops a mark that no longer protects anything, while a genuinely damaged table stays refused. `hp_clear()` -- reached through TRUNCATE or DELETE without WHERE -- clears the state along with the data, because it rebuilds the indexes from nothing. Refusing a table known to be corrupt beats continuing and delivering wrong results. 3. `delete_key()` is assumed to succeed: it allocates no memory, so it cannot fail unless the table is already inconsistent, and building recovery logic and injected failures for that case would complicate the code for a scenario that cannot happen on a healthy table. If it ever does fail, the error falls outside the recovery list and the table is marked crashed, preserving the damaged state for analysis. The debug-build consistency check in `ha_heap::external_lock(F_UNLCK)` skips tables already marked crashed: their inconsistency is known and deliberate, and re-detecting it would raise a second error into a diagnostics area that can already hold OK, firing the `Diagnostics_area` assertion the check exists to prevent. The allocation failure is injected inside `tree_insert()` itself: `simulate_tree_insert_oom` fails every node allocation until the caller disarms it, `once_simulate_tree_insert_oom` fails only the next one and disarms itself. The two names must not be a prefix of one another, because `DBUG_SET()` matches an existing keyword by prefix and would silently merge instead of adding. Callers arm the keywords themselves and keep an inert guard keyword in the list, because a keyword list that becomes empty while debugging is on matches every keyword. The unit test `hp_test_update` drives the failures through the engine API: the classification and its duplicate-key counterpart, an already moved key being moved back, and the crashed lifecycle -- marking, refusal of reads and writes, the check that clears a stale mark on an intact table and keeps a damaged one crashed, and the reset on emptying. With the defects reintroduced, 8 of its assertions fail. `heap.update_key_rollback` covers the same from SQL; without the fix its first UPDATE reports `ER_DUP_ENTRY 'Duplicate entry 101 for key k2'` on a key that is not unique. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40447 Cap HEAP block allocations derived from the memory ceiling `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. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40802 COUNT(DISTINCT <blob>) fails when its tmp table converts `COUNT(DISTINCT)` collects the distinct values in a temporary table with a unique constraint over the aggregate's arguments, and treats a duplicate key error from the write as "value already seen": ```c if (!table->file->is_fatal_error(error, HA_CHECK_DUP)) return FALSE; // duplicate, not an error ``` For a blob argument the record holds only a pointer to the value, so `Aggregator_distinct::setup()` cannot use the `Unique` tree, which compares raw record bytes, and every value goes through that write instead. When such a write overflows the in-memory table, `create_internal_tmp_table_from_heap()` copies the stored rows to an on-disk table and then writes the row that overflowed, which until then was held in `record[0]` alone. Whether a duplicate key error on that last write is fatal is decided by the caller's `ignore_last_dupp_key_error` argument, and `Aggregator_distinct::add()` passed **0** three lines below the code that ignores the very same condition. The statement failed with ERROR 1169 (23000): Can't write, because of unique constraint, to table '(temporary)' Pass **1** instead, so that a duplicate arriving through the conversion is discarded exactly like one arriving through the ordinary write. The result is `table->file->stats.records` of that table, so not storing the duplicate is what makes the count right. The argument is the same upstream, where it is unreachable: a temporary table with a blob column was created on the on-disk engine to begin with, so the conversion was never entered for the only tables whose pending row can be a duplicate. Supporting blob columns in the in-memory engine made the table start in memory and convert. New tests `heap.count_distinct_blob_convert` and `heap.count_distinct_blob_convert_debug`. A write rejected as a duplicate returns its record to the free list and never reaches the allocation of the blob value, so only the first copy of a value makes the in-memory table grow, and the write that finds it full is the second copy of the value stored last. That holds only while a record slot is what the table runs out of first. Blob values come out of the same space, and only a write that is not a duplicate ever allocates one, so when a blob allocation is the one that hits the limit, the pending row is not a duplicate at all. Which of the two runs out first follows from how records and blob values pack together, not from any threshold on the value width. Of 24 measured combinations of width and `max_heap_table_size`, 20 convert but only 8 reach a duplicate pending row, so asserting that the table was converted does not establish that the ignored duplicate was reached. The first test uses widths measured to overflow on a record slot. The second removes the dependency on that measurement, injecting the duplicate through a new debug point in `Tmp_table_default_copier::copy_rows()`, beside the one the row copy loop already carries. Every value is present twice, so whichever copy the injected duplicate discards, the other one is still written and the count does not depend on which write overflowed. The status counter is read with the in-memory limit restored. The status table is materialized into a temporary table of its own, and its VARIABLE_VALUE column is wide enough to be stored as a blob, so under the shrunken limit that table can overflow and be converted as well, and would then report its own conversion. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40781 Duplicate row despite DISTINCT when tmp table converts `create_internal_tmp_table_from_heap()` writes the pending `record[0]`, the row whose write filled the in-memory table, into the new table. Since **MDEV-40376** (`636f154bb49`) that write happens *before* `ha_end_bulk_insert()` rather than after it. `ha_maria::start_bulk_insert()` disables **all** indexes of an internal temporary table that is about to receive at least `MARIA_MIN_ROWS_TO_DISABLE_INDEXES` (100) rows: ```c if (file->open_flags & HA_OPEN_INTERNAL_TABLE) { /* Internal table; If we get a duplicate something is very wrong */ file->update|= HA_STATE_CHANGED; index_disabled= share->base.keys > 0; maria_clear_all_keys_active(file->s->state.key_map); } ``` `maria_write()` then skips `_ma_check_unique()` entirely, so the unique constraint that implements `DISTINCT` for a key too wide to be an index is not enforced. The rows copied out of the in-memory table are already distinct and need no checking against each other, but the pending row is exactly the row whose duplicate status is unknown, and it was written inside that window. A `SELECT DISTINCT` over wide columns could therefore return a duplicate row. Note that the justification given in `636f154bb49` is not the mechanism at work here. It refers to the bulk insert key *tree*, a different branch of `ha_maria::start_bulk_insert()`; setting `bulk_insert_buffer_size=0` does not avoid the problem. The fix splits the copy in two: 1. `Tmp_table_row_copier` gains a second virtual, `write_pending_row()`, defaulting to a no-op. 2. `copy_rows()` now only copies the rows the in-memory table holds. 3. `create_internal_tmp_table_from_heap()` calls `ha_end_bulk_insert()` and then `write_pending_row()`, so the pending row is written with the indexes of the new table back in place and a duplicate of an already copied row is detected. `Window_rowid_remapper` keeps writing its pending row within `copy_rows()` and inherits the no-op default. Its new position is only known once the rows before it have been written, and nothing is lost by writing it with the indexes still disabled: it replaces a row that is already in the table rather than adding one, and an update of a window function value cannot collide with another row, as a deduplicating key is not built on the columns it changes. The new test covers `SELECT DISTINCT`, `SELECT DISTINCT ... ORDER BY`, `GROUP BY`, `UNION` and `INSERT ... SELECT DISTINCT`, and asserts that the conversion actually happened so that a future sizing change cannot silently void the coverage. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Anway Durge
durgeanway@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-31342: Optimize INFORMATION_SCHEMA.TABLES queries to skip temp table writes For simple single-schema SELECT queries on information_schema.tables, bypass ha_write_tmp_row() and stream rows directly via the protocol. Add EXPLAIN is_fast_path visibility and MTR coverage, including Windows EXPLAIN casing normalization. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40585 Assertion `(data_len == 0) == (data_ptr == ((void *)0))' fails in hp_flush_unaliased_blob_free `hp_flush_unaliased_blob_free()` asserted that a zero-length blob in the record buffer carries a `NULL` data pointer. The SQL layer does not guarantee that direction of the invariant: - `Field_blob_compressed::store()` of a zero-length value allocates its scratch `String` first and then stores `(length = 0, ptr = value.ptr())`, leaving a stale non-`NULL` pointer. Plain `Field_blob::store()` zeroes the whole pack instead, which is why an uncompressed column does not reproduce this through SQL. - `Field_blob::unpack()` points a zero-length blob at the row-based replication event buffer, so the applier trips the same assertion on a slave-side `HEAP` table with a plain, uncompressed column. The assertion evaluates only for a column whose old chain was parked for deferred free, so the failing statement must both park a chain and write a zero-length blob: `REPLACE` over an existing row, `INSERT ... ON DUPLICATE KEY UPDATE`, or one replicated row-event group doing the same. A delete and an insert in separate statements redeem the parking through the record-less `hp_flush_pending_blob_free_impl()` and are unaffected. Debug builds only. Every decision in the engine -- here, in `hp_write_blobs()` and in `heap_update()` -- tests the stored length and never the pointer, so release builds store, free and adopt chains correctly and no wrong data is ever written. Keep the direction that is guaranteed, a non-empty blob must have a data pointer, and drop the reverse implication. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||