mirror of
https://github.com/MariaDB/server.git
synced 2026-10-23 18:49:29 +03:00
copilot/research-proxy-protocol-v2
206209 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
forkfun
|
82964737c0 |
MDEV-31527 Flush buffered early messages on early exit
Buffered_logs::flush() prints and frees the buffered early-option messages. Call it on every early exit, so warnings and errors are not lost and the buffer is not leaked. Make the early-option error exit in mysqld_main() unconditional. Without the performance schema, a failed early option parse left "----file-marker----" in argv, and it was reported as an unknown option. Index.xml parse errors go through error_handler_hook, not vprint_msg_to_log(), so --validate-config reported "Configuration is valid." Install a hook that sets validate_config_has_warnings. Test cleanup. |
||
Abdelrahman Hedia
|
d74a938725 |
MDEV-31527: Add --validate-config option to check configuration without starting the server
Add a new --validate-config command-line option that validates the server configuration (from config files and command line) and exits with 0 on success or non-zero on failure, without actually starting the server. This is useful for DBAs to verify configuration changes before restarting: mariadbd --defaults-file=/etc/my.cnf --validate-config The validation checks the following: - Config files explicitly specified via --defaults-file or --defaults-extra-file must exist and be readable - There are no unknown options specified (in config files or on the command line) - Option values are type-checked (e.g. numeric options reject non-numeric input). Out-of-range values are adjusted to valid limits with a warning, not rejected. Output: [Note] messages are suppressed during validation (as with --silent-startup), so a clean configuration prints only Configuration is valid. If any warning was printed while checking the configuration, the verdict is instead "Configuration is valid, but warnings were reported; see above." The verdict is written directly to stdout rather than as a [Note], so neither the note suppression nor --silent-startup can hide it. Warnings and errors are still printed. Exit codes: 0 - Configuration is valid 2 - Unknown option encountered 7 - Unknown variable encountered Other non-zero codes are possible for other parse errors (e.g. invalid enum value, numeric parse error). The option description now correctly states "non-zero for failure" rather than "1 for failure". The implementation reuses the existing --help code path (opt_abort), which already loads plugins to validate their variables and runs a final parsing pass with skip_unknown=0 to detect unknown options. The only difference is that --validate-config suppresses help output. --help and --version take precedence: when either is present alongside --validate-config, the server prints help/version and exits cleanly without performing config validation, also with --verbose. Warning detection: validate_config_has_warnings is set in one place, vprint_msg_to_log() in log.cc, which every printed error-log message goes through (sql_print_warning(), option_error_reporter(), buffered early-option warnings, the charset reporter). Warnings that are not printed are not counted: option_error_reporter() drops --loose-* option warnings under --bootstrap before reaching vprint_msg_to_log(), so the --loose-* options in mtr's $MYSQLD_BOOTSTRAP_CMD do not turn the verdict into the "warnings were reported" variant. Warnings for options parsed in handle_early_options() (e.g. max_connections) used to be buffered and printed through the error log only in builds with the performance schema; otherwise mysys printed them straight to stderr, bypassing the error log. The buffering is now done in all builds, so these warnings are printed through the error log and counted like any other. Bug fixes included in this patch: - --validate-config --log-bin no longer creates a spurious <host>-bin.index file in the datadir. The binlog index open is now guarded by !opt_abort (was: !opt_help), skipping it for all early-exit paths including --help --verbose. - Simplified early-exit condition in init_common_variables() from a four-variable expression to (opt_help || opt_version) && !opt_verbose, which is equivalent and clearer (vuvova review). Note: this is intentionally named --validate-config (not --validate-defaults) for MySQL compatibility, although --validate-defaults would be more consistent with MariaDB's existing defaults-related options (--defaults-file, --no-defaults, my_print_defaults, etc.). Tests: the runs that expect the clean verdict pass options that keep the server from printing environment-dependent warnings (open files ulimit, case-insensitive file system), since any printed warning now correctly produces the "warnings were reported" verdict. |
||
Marko Mäkelä
|
45cb689c71 | Merge 13.1 into main | ||
|
Marko Mäkelä
|
81535439d3 | Merge 13.1 into main | ||
|
Marko Mäkelä
|
5ab51d6248 | Merge 12.3 into 13.1 | ||
|
Marko Mäkelä
|
d0661d9bb8 | Merge 12.3 into 13.1 | ||
|
Marko Mäkelä
|
4083fbef83 | Merge 11.8 into 12.3 | ||
|
Marko Mäkelä
|
5981efcecd | Merge 11.8 into 12.3 | ||
bsrikanth-mariadb
|
ba7112fcae |
MDEV-40553 Print GIS ranges in dbug_print_sel_arg()
dbug_print_sel_arg() printed the key values of a SEL_ARG over a SPATIAL index through Field::val_str(). The key of such a SEL_ARG holds the MBR (Minimum Bounding Rectangle) of the value, four doubles, not the geometry itself, and the SEL_ARG has no min/max bounds: the spatial relation is carried in min_flag as GEOM_FLAG | HA_READ_MBR_xxx. So the output was garbage bytes between "-inf"/"+inf" style bounds. Detect GEOM_FLAG and print the SEL_ARG the same way print_range() does for the optimizer trace: "col mbr_relation POLYGON(...)", using print_mbr_range_operator() and Field_geom::print_key_part_value() with image_type itMBR. Ranges over geometry columns in non-SPATIAL indexes (itRAW) still go through the old path. |
||
|
bsrikanth-mariadb
|
29f0cd882c |
MDEV-40553 Print GIS ranges in optimizer trace and context
Ranges built over GIS (geometry) columns could not be printed in the optimizer trace or the recorded optimizer context: Field_geom printed every key value as the placeholder "unprintable_geometry_value", regardless of whether the index stored the column's raw value (or a prefix of it) or, for a SPATIAL index, its MBR (Minimum Bounding Rectangle). Field::print_key_part_value() now takes an image_type argument (see Field::image_type()) that says which of the two the key holds. For a SPATIAL index (image_type itMBR), Field_geom::print_key_part_value() decodes the four doubles the key stores and prints them as a WKT POLYGON. For every other index (image_type itRAW), the key holds the value or a prefix of it, and it now prints in binary form, the same way Field_blob does, instead of the placeholder. print_mbr_range_operator() prints the spatial relation a GEOM range carries (MBRWITHIN, MBRCONTAINS, MBRINTERSECTS, MBRDISJOINT, MBREQUALS), inverted where needed so the indexed column reads on the left. print_range() and print_key_value() thread the new image_type argument through to Field_geom::print_key_part_value(). Added new tests in opt_trace, opt_context_store_stats, opt_context_replay_basic |
||
Sergei Petrunia
|
b2d2266674 |
MDEV-40553: include/opt_context_save_in_var.inc can't handle quotes
I_S.OPTIMIZER_CONTEXT has this statement: set @opt_context='CONTEXT'; -- denotes CONTEXT-STMT And opt_context_save_in_var.inc just took the CONTEXT and put it in a variable: SET @opt_context= (select REGEXP_SUBSTR(...) from i_s.optimizer_context); This ignored the fact that the "CONTEXT" is escaped for the SQL parser. Fix this by extracting the entire CONTEXT-STMT and running it. |
||
|
Marko Mäkelä
|
bff2c4e547 | Merge 13.1 to main | ||
|
Marko Mäkelä
|
0b24391c0b | Merge 12.3 to 13.1 | ||
|
bsrikanth-mariadb
|
b0cdf72a39 |
MDEV-40598: Capture sequences used only in column DEFAULT expressions
Problem: A sequence referenced only in a column's DEFAULT expression (e.g. "a INT DEFAULT NEXTVAL(s1)") is opened only when a statement evaluates DEFAULT values (INSERT, LOAD DATA, etc). A plain SELECT never opens it, so the sequence never appears in thd->lex->query_tables, and Optimizer_context_recorder:: dump_sql_script() had no way to see it. The dependent table's definition was then captured without the sequence it depends on, making the captured context unusable on replay. Fix: TABLE::internal_tables lists these sequences. dump_sql_script() walks it for each dumped table and calls the new dump_sequence_placeholder() for each entry, before the table's CREATE TABLE so that replay can recreate both in order. The sequence is not opened; only its database and name are used, so for now it is captured as a placeholder "CREATE SEQUENCE IF NOT EXISTS db.name" without its parameters or current value. The name is qualified with the sequence's own database, so a sequence in a different database than the table is recreated in that database. Such a placeholder is also remembered in a separate hash. If the query uses the same sequence directly as well, the main loop of dump_sql_script() reaches it later, finds the placeholder, emits "DROP SEQUENCE IF EXISTS db.name", and then dumps the sequence's real definition and current value (SETVAL) as for any sequence used in the query, regardless of the order the tables are listed in. To share code with the existing dump loop, the SETVAL logic of the loop is factored out into dump_sequence_current_value(). Even for the scenarios where sequence is dropped after table creation, a name is replaced by a view, or a user doesn't privilege on the sequence; The name is still captured as a sequence. Tests (opt_context_store_ddls.test): INSERT-opened sequence; plain SELECT; LOCK TABLES; a user without privilege on the sequence; two sequences in one table; dropped sequence; stored function in the query; name replaced by a view; re-execution as a prepared statement; sequence in a different database; sequence used both directly and as a DEFAULT dependency, in both orders. |
||
|
Marko Mäkelä
|
db94f391be |
MDEV-33966: buf_page_make_young() is a contention point
The buf_pool.LRU list needs to reasonably accurately reflect recently accessed blocks, so that they will not be evicted prematurely. Because the list is protected by buf_pool.mutex, it is not a good idea to maintain the position on every page access. Instead of maintaining the LRU position on each access, we will decide on each access whether the block qualifies for promotion, and record the decision in a "promote" flag in the block descriptor. The buf_flush_page_cleaner() thread as well as some traversal of the buf_pool.LRU list, all of which hold buf_pool.mutex anyway, will move the flagged blocks to the "recently used" end of buf_pool.LRU. A block qualifies only if it is accessed again at least innodb_old_blocks_time after the reference point, which is its first access and, after that, its most recent promotion. The age is measured in whole seconds and checked on the access, not when a sweep reaches the block, so that a block that a table scan accessed in one burst will not be promoted, however long it stays in buf_pool.LRU_old. After a promotion, the age is measured from the time the sweep moved the block, which can be later than the access that qualified it. The rule applies at any position in buf_pool.LRU, and the flag stays set until a sweep moves the block. Thus, a block that qualified outside buf_pool.LRU_old keeps the flag until a sweep reaches it, which is usually after it has moved into buf_pool.LRU_old; before, such a block was moved on any access when freed_page_clock showed that it was no longer close to the "recently used" end. buf_page_make_young_if_needed(), buf_page_make_young(), buf_page_peek_if_too_old(), buf_page_peek_if_young(), btr_cur_nonleaf_make_young(), buf_page_t::set_accessed(): Replaced by buf_page_t::touch(), buf_page_t::touch_no_stamp() and buf_page_t::make_young_if_needed(). buf_pool_t::freed_page_clock, buf_page_t::freed_page_clock: Remove. This is no longer meaningful in the revised design. INFORMATION_SCHEMA.INNODB_BUFFER_PAGE(_LRU).FREE_PAGE_CLOCK now always reports 0. Before the first eviction, buf_LRU_stat_update() now records statistics intervals, and buf_LRU_evict_from_unzip_LRU() uses its formula instead of assuming a disk-bound workload. page_zip_des_t::state: An atomic 16-bit field that will include the PROMOTE and OLD flags that would more logically belong to buf_page_t. We maintain them here (along with some ROW_FORMAT=COMPRESSED specific state that is protected by page latches) in order to avoid race conditions and unnecessary memory overhead. m_end moves out to its own field, shrinking the packed word to 16 bits; n_blobs narrows from 12 bits to 10 to make room for PROMOTE and OLD, still comfortably above the 744-column maximum. The helpers that set or clear one bit of page_zip_des_t::state always perform the atomic read-modify-write, which is a full barrier on x86. A caller that can find the bit already in the target state checks it first: touch_no_stamp() for PROMOTE, buf_LRU_add_block() for OLD, and page_zip_write_rec() for NONEMPTY. page_zip_des_t::copy_from() and set_n_blobs_and_empty() skip the fetch_add() when the delta is 0. page_zip_decompress_clust() counts the BLOB pointers locally and invokes add_n_blobs() once. page_zip_des_t::copy_from(): Replaces the copy constructor that page_zip_copy_recs() invoked. It copies m_end, NONEMPTY and n_blobs from the source page, but preserves the PROMOTE and OLD flags of the destination block, because they describe the position of that block in buf_pool.LRU. Copying OLD from the source would make buf_pool.LRU_old_len inconsistent with the list. Like set_n_blobs_and_empty(), it updates the state with a single fetch_add(), which preserves any concurrent change of PROMOTE or OLD. buf_page_t::init(): Takes the ROW_FORMAT=COMPRESSED shift size (ssize) directly and clears the zip descriptor state itself, so callers (buf_block_t::initialise(), buf_page_init_for_read()) no longer need a separate page_zip_des_init()/page_zip_set_size() call. buf_page_t::invalidate(): Replaces buf_block_modify_clock_inc(). Instead of maintaining a 64-bit counter, we will maintain one comprising 32+16=48 bits, in modify_clock_low,modify_clock_high. Worst case there will be exactly n<<48 calls to buf_page_t::invalidate() before some operation such as btr_pcur_t::restore_position() is executed. Such a count should be extremely unlikely but not completely impossible. It is worth noting that the DB_TRX_ID is only 48 bits, and each transaction start and commit/rollback will consume an identifier. buf_page_t::modify_clock(): Replaces the read access of buf_block_t::modify_clock. Assert that the caller is holding a page latch. Note: because invalidate() and modify_clock() are protected with buf_pool.mutex or the buf_page_t::lock, there can be no issue with regard to the atomicity of accessing the 48-bit field. PageBulk::m_modify_clock: Remove. It was assigned but never read. buf_page_t::relocate_from(): Copies a descriptor that is relocated to this block, in buf_relocate() and buf_pool_t::shrink(), but keeps the frame and the modify_clock() of this block. The copy constructor does not copy the clock: it belongs to the block descriptor and must only grow during its lifetime, as buf_block_t::modify_clock did. Otherwise, the clock of this block could return to a value that a btr_pcur_t saved before the block was freed, and btr_pcur_t::restore_position() could reuse a record pointer although the page was modified in another block in the meantime. buf_page_t::access_time: Store the 16-bit buf_pool.access_clock rather than a 32-bit millisecond ut_time_ms(). It wraps around every 18.2 hours; ages are computed as uint16_t(now - access_time), which stays correct across that wrap. This avoids any alignment loss: the adjacent fields modify_clock_low, modify_clock_high, access_time of 32+16+16 bits nicely add up to 64 bits. access_time is stamped on the first access after the block was initialized, and on each promotion by make_young_if_needed(), so that the age of a frequently promoted block stays exact across the wrap. buf_pool_t::access_clock: uint16_t(my_interval_timer() / 1000000000), never 0 (see buf_pool_t::now()), refreshed about once per second by buf_pool_t::refresh_clock() so that page accesses need not read the system clock. srv_master_callback() refreshes it. From buf_pool_t::create() until srv_master_timer is started, and for good when srv_master_timer is not started (innodb_read_only, innodb_force_recovery>=2, mariadb-backup), a separate buf_pool_clock_timer refreshes it. buf_pool_t::access_clock, buf_pool_t::LRU_old_threshold: Located in a cache line of their own, because they are read on page accesses and the adjacent buf_pool fields are frequently written. buf_page_t::touch(): Stamp access_time on the first access, then invoke touch_no_stamp(). Return whether this was not the first access, as the result of buf_page_make_young_if_needed() used to be. A buffer-fix is sufficient, as in MVCC undo page lookups. buf_page_t::touch_no_stamp(): Set the PROMOTE flag if innodb_old_blocks_time is 0, or if accessed_at() is at least that old, at any position of the block in buf_pool.LRU. Because buf_pool.access_clock advances once per refresh_clock(), about once per second, a difference of threshold ticks can span less than innodb_old_blocks_time; the difference must exceed the threshold, so that a burst of accesses that crosses a refresh is not promoted. Like btr_cur_nonleaf_make_young(), do not stamp access_time. Once PROMOTE is set, later accesses only load the state and return. buf_page_t::make_young_if_needed(): If PROMOTE is set, invoke buf_page_t::make_young(). This part is inline, so that a sweep pays no function call for a block that stays in place. It reads the state of the block once: buf_pool.mutex, which the caller holds, protects OLD, and only make_young() clears PROMOTE. The block need not be in buf_pool.LRU_old: while buf_pool.LRU is shorter than BUF_LRU_OLD_MIN_LEN, buf_pool.LRU_old does not exist, and blocks are evicted without ever becoming old. The template parameter only_old leaves a block that is not old in place; buf_do_flush_list_batch() uses it, because it visits blocks in the order of oldest_modification, and moving a block that is not old would not save it from eviction. The template parameter count_not_young selects whether an old block that has an accessed_at() stamp but is not flagged is counted in buf_pool.stat.n_pages_not_made_young. This includes a block that had no access since its last promotion, so the count is of sweep visits, not of rejected accesses; only the eviction sweeps buf_LRU_free_from_common_LRU_list() and buf_flush_LRU_list_batch() do this, and a block that a sweep leaves in the list can be counted again by a later sweep. As before, buf_pool.stat.n_pages_made_young counts only the moves of old blocks. Because a sweep moves the block, not the access, and counts the blocks that it does not move, Innodb_buffer_pool_pages_made_young and Innodb_buffer_pool_pages_made_not_young can differ from before for the same workload. buf_page_t::make_young(): Clear PROMOTE, stamp access_time, and move the block to the "recently used" end of buf_pool.LRU. The block can be read-fixed, because buf_pool_t::unzip() copies PROMOTE and OLD from the compressed-only descriptor and releases buf_pool.mutex during buf_zip_decompress(). Unlike buf_page_make_young(), we do not skip such a block: a page access no longer acquires buf_pool.mutex, and the sweeps hold buf_pool.mutex but no buffer-fix on the block. buf_LRU_remove_block(): When buf_pool.LRU becomes too short for buf_pool.LRU_old, clear the OLD flags from buf_pool.LRU_old to the end of the list, which holds all the old blocks, not the flags of every block. While buf_pool.LRU_old does not exist, no block in buf_pool.LRU is old, so on later removals the loop does nothing. buf_LRU_add_block(): Write the OLD flag only if it changes. In a list that is too short for buf_pool.LRU_old, assert that buf_pool.LRU_old is not defined and that the block is not old, instead of writing the flag. innodb_old_blocks_time_update(): New sysvar update callback, replacing a NULL one, that calls buf_pool_t::set_old_threshold_ms(), so that SET GLOBAL innodb_old_blocks_time also updates the LRU_old_threshold in seconds that page accesses read. The threshold is clamped to 65534 seconds (UINT16_MAX - 1), because the age uint16_t(now - access_time) cannot exceed 65535 and a larger threshold would prevent any promotion. The sysvar itself still accepts up to UINT_MAX32 milliseconds. buf_pool_t::set_old_threshold_ms(): Round the millisecond threshold up, not down, to the nearest second. innodb_old_blocks_time is documented and accepted in milliseconds; flooring instead of ceiling would make any configured value from 1 to 999 silently behave as 0 (disabled). buf_page_t::is_accessed(): Renamed accessed_at(), to stop reading as a boolean. It returns the access_time stamp of the first access or of the last promotion, in seconds; INFORMATION_SCHEMA.INNODB_BUFFER_PAGE(_LRU).ACCESS_TIME now reflects that. buf_read_ahead_random(): A page now qualifies once accessed_at() holds, together with either zip.is_promote() or !zip.old(). buf_read_ahead_linear(): Compare access_time stamps as a signed 16-bit difference, not raw unsigned, so the monotonic-access check stays correct across the access_time wrap. The resolution of the stamps is 1 second instead of 1 millisecond. buf_LRU_scan_and_free_block(): Declare static. buf_flush_LRU_list_batch(): In a run of blocks that make_young_if_needed() moves, release and reacquire buf_pool.mutex after every 512 scanned blocks, except on the first scanned block. A move costs much less than an eviction, so the stride is longer than the one of the eviction path. buf_pool_invalidate(): Define in the same compilation unit with buf_LRU_scan_and_free_block(). buf_pool.LRU_old_time_threshold: Replaces buf_LRU_old_threshold_ms. PageConverter::run(): Renamed from fil_iterate(). In debug builds, initialize and acquire a dummy exclusive latch on the block, so the assertion in buf_page_t::invalidate() is satisfied; free the latch on every path, not only on success. AbstractCallback::m_zip_ssize: Replaces m_zip_size. page_zip_des_t: Add calc_ssize()/zip_size() helpers, replacing the zip_size<->ssize conversion duplicated across buf0buf.cc, buf0rea.cc and row0import.cc. innodb.buf_lru_scan_resistance: A new big test that checks that pages of a table that is accessed in one burst are evicted, even if a sweep reaches them only after innodb_old_blocks_time, that pages accessed again after that time are promoted, and that pages read while the buffer pool is being filled are not promoted later. innodb_zip.n_blobs_700: A new test that stores 700 BLOB pointers on one ROW_FORMAT=COMPRESSED page, within the 10-bit n_blobs field. The index_page_splits counter confirms that the clustered index consists of its root page alone, without depending on which pages remain in the buffer pool. Co-Authored-By: Alessandro Vetere <iminelink@gmail.com> |
||
|
Marko Mäkelä
|
6e14c96d15 | Merge 13.1 into main | ||
|
Marko Mäkelä
|
f01bb8c7ed |
MDEV-41359 Potential hang with innodb_log_archive=ON
log_t::checkpoint_margin(): In the target LSN computation, account
for the special case resize_log.m_file==log.m_file which signals
that a checkpoint write is in progress. Assert that the target
checkpoint LSN is sensible.
Fixes up commit
|
||
Arcadiy Ivanov
|
cf542109f3 |
MDEV-41297 A stored empty blob never equals an all-space value
Blob values for empty strings should return a pointer to an empty string and not NULL. `hp_materialize_one_blob()` returned NULL, which its callers `hp_rec_key_cmp()` and `hp_key_cmp()` read as an allocation failure. `hp_test_write_dup-t.c` was extended to test key reads on `TEXT` columns. This could not be done in MTR, as a `MEMORY` table cannot be created with a key on a `TEXT` column. |
||
|
Arcadiy Ivanov
|
2650897e3d |
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`. |
||
|
Arcadiy Ivanov
|
6ba1f128f7 |
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. |
||
Monty
|
36a2729d5b |
Trivial optimziations for group_concat
- Remove some if
- Reorder code
- More code comments
(cherry picked from commit
|
||
|
Arcadiy Ivanov
|
c82597af99 |
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
|
ffc817f040 |
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. |
||
|
Arcadiy Ivanov
|
4346702fdb |
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`. |
||
|
Arcadiy Ivanov
|
a4bb5c7b2e |
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.
|
||
|
Arcadiy Ivanov
|
92e3185719 |
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. |
||
|
Monty
|
2c6f9f11b0 |
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 <arcadiy@ivanov.biz> - Fixed a bug in copy_tree() |
||
|
Monty
|
c37016b0e5 |
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. |
||
|
Arcadiy Ivanov
|
578469d10c |
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
|
03c04bbd6c |
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.
|
||
|
Arcadiy Ivanov
|
4e7a9fe98c |
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.
|
||
|
Arcadiy Ivanov
|
e080205915 |
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. |
||
|
Arcadiy Ivanov
|
dd896f05d9 |
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. |
||
|
Arcadiy Ivanov
|
6dd5856f5f |
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. |
||
|
Arcadiy Ivanov
|
d503d486d2 |
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. |
||
|
Arcadiy Ivanov
|
a9bc620abb |
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
|
b22309eaf1 |
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. |
||
|
Arcadiy Ivanov
|
beaadd34c9 |
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
|
45fec44293 |
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
|
f1a72e2cca |
MDEV-40376 Protocol::end_statement assertion when window function fills HEAP tmp table
`save_window_function_values()` did not take into account that one can get `HA_ERR_RECORD_FILE_FULL` on `ha_update_row()`. With blob support in heap, it can now happen more easily (expressions wider than 512 characters are promoted to TEXT in tmp tables, and each update of a blob result column allocates a continuation record). However, it could also happen with Aria tables, which is apparently not tested. The error was returned as a plain `true` with an empty diagnostics area: debug builds hit `Assertion '0'` in `Protocol::end_statement()`, release builds send a bare OK packet after the result set metadata, which the client mis-parses (appears as a hang / lost connection). Errors of the computation are now exposed: `save_window_function_values()` calls `handler::print_error()` for any failure it does not recover from, the previously ignored `ha_rnd_pos()` return values are checked and reported, and `compute_window_func()` distinguishes a read error from EOF and stops the row scan as soon as the per-row loop fails. The overflow itself is recovered from by converting the tmp table to the disk-based tmp engine in place: the rows keep their identity and their contents, only their positions change, and those are translated in the rowid sequence that the computation is built on. The filesort result of a window sort is a sequence of row positions that covers every row of the table exactly once (the sort is set up without a limit) and is the only place where the positions are kept: the row scan and all frame cursors read the rows through it. The conversion (`Window_rowid_remapper`) therefore copies the rows in the order of that sequence, which makes the new position of a row known as soon as the row has been written, and stores it back into the very slot the old position was read from. For the new position to always fit into the slot, a window sort stores the row positions in slots of a fixed 8 bytes (`Filesort::min_ref_length`, set to the new `TMP_TABLE_MAX_REF_LENGTH`), which holds the position of any engine an internal tmp table can use: a pointer into memory or a data file offset. `make_sortkey()` zero-pads a shorter position to its slot, and `SORT_INFO::ref_length` carries the slot width to the readers, so `init_read_record()` no longer re-derives it from `handler::ref_length`, which changes when the table is converted. Sorts that do not set a minimum, and engines whose positions are wider than 8 bytes, are unchanged byte for byte. A sequence held in memory is thereby rewritten in place, keeping its layout, so the frame cursors' places in it stay valid. A sequence held in a temporary file is written and read through an `IO_CACHE`, which encrypts the file when tmp file encryption is enabled, so the file can not be rewritten in place: the old sequence is streamed out of its cache and the translated sequence into a new cached temporary file, which then replaces the old one under the cache. The frame cursors' slave caches stay linked to the master through `next_file_user` across the replacement, and are re-created from the new master afterwards (`Frame_cursor::rowids_rewritten()`). Should re-creating one fail, the cursor forgets its already-released cache, so that it is not released a second time when the cursor is destroyed, and reports the failure. The row whose update did not fit is written from `record[0]`, which holds its new image, instead of being read from the table, so the conversion applies the update that failed. Blob values of `record[0]` that were read from the HEAP table can point into the handler's shared blob reassembly buffer (`hp_read_blobs()`, blob values that span multiple allocation blocks), which reading the other rows overwrites, so they are first given memory of their own. When the window sort was set up with a deferred filter (a HAVING clause deferred to the final ORDER BY sort), the sequence omits the rows the filter rejected and the conversion drops them: every later reader of the table applies the same filter, either directly or by reading the table through a sort that does, so such rows can never reach the result. `create_internal_tmp_table_from_heap()` gains a `Tmp_table_row_copier` hook for the copy from the HEAP table to the Aria or MyISAM table that replaces it. The plain copy loop and the append of the pending `record[0]` become `Tmp_table_default_copier`, used when the caller passes no copier, so both cases of the conversion run through the same code, defined together with `Window_rowid_remapper` just before the function. `ha_end_bulk_insert()` is called also when the copy fails, so the new handler does not keep bulk insert state when its table is dropped; the pending row is thereby written while the bulk insert is still active, which does not delay its duplicate detection, as bulk insert does not cache unique keys (`maria_init_bulk_insert()` skips `HA_NOSAME` keys). An ignored duplicate of the pending row is passed to the caller in `Tmp_table_row_copier::duplicate_row_error`, which sets `*is_duplicate`. Carrying the running computation over the conversion, instead of re-running it, has two visible consequences: - Compound expressions containing window functions (`items_to_copy`) are evaluated exactly once per row, as they are when the table does not overflow, so the conversion does not have to be refused when such an expression is non-deterministic (`RAND_TABLE_BIT`: a user variable assignment, a non-deterministic stored function); those statements produce their result. - The statement sorts once, so sort related aggregate warnings, e.g. the `max_sort_length` truncation counts, are reported for one pass. The rows are copied in the order of the sequence, so the converted table does not hold them in the order they were inserted in. This is safe even when that order is what the table was built for (`TABLE::keep_row_order`, set for `ROWNUM()` with GROUP BY or ORDER BY): before HEAP supported blobs, such a tmp table was created in the disk engine from the start and these statements simply ran, so refusing here would be a regression against that behavior. 1. `ROWNUM()` values are materialized into the tmp table rows during the fill, before the window computation begins; the copy moves them verbatim, so their pairing with the rows can not change. 2. The rowid sequence is translated in place, so the running window computation continues over exactly the same row order, and tie-sensitive window function values (`ROW_NUMBER`, frames over tied keys) keep the values the interrupted pass had already produced. 3. Consumers that scan the converted table afterwards see the rows in the copy order. Among rows with equal sort keys that order differs from the insertion order, but such tie order is unspecified and already differs between the memory and disk tmp engines. The converted table keeps `keep_row_order= true` so the copy order is also the order the disk engine preserves from then on. Tests, in `heap.blob_window_overflow` unless noted: a single window over all rows and a partitioned window overflow mid-computation and must transparently convert; the side-effecting window expression test verifies that the user variable is assigned exactly once per row, against the same query that does not overflow; a fourth test covers the sequence being an array in memory instead of a merge file; a fifth test covers the `keep_row_order` conversion, verifying that every `ROWNUM()` value keeps its insertion-order pairing with its row across the conversion; a sixth test covers blob values larger than a HEAP allocation block surviving the conversion of the pending row; `heap.blob_window_overflow_encrypt` runs the conversion with encrypted temporary files; `heap.blob_window_overflow_debug` covers the failure to re-create a slave cache of the sequence (`simulate_window_seq_slave_oom`), which previously hung the server in the cursor destructor. |
||
|
Arcadiy Ivanov
|
4bfa20722a |
MDEV-40523 Versioned UPDATE on a HEAP table with blobs corrupts the history row
A system-versioned `UPDATE` of a blob column on a `HEAP` table stored garbage in the history row, and an `AFTER UPDATE` trigger reading `OLD.<blob>` saw the same garbage. Both values are durable: the history row is what `SELECT ... FOR SYSTEM_TIME ALL` returns, and both reach replicas through the row-based binlog image. No ASAN build is needed to reproduce either. ## How a versioned UPDATE reaches the engine The row is first updated in place with `ha_update_row(old_data, new_data)`. The SQL layer then calls `vers_insert_history_row()`, which restores the pre-update row from `record[1]` into `record[0]`, stamps it with the delete-time and calls `ha_write_row()`. So the record handed to `ha_write_row()` is a verbatim copy of the row the engine was just told to overwrite -- blob data pointer included. A versioned `DELETE` is not affected: `TABLE::delete_row()` stamps the end field with `vers_update_end()` and issues a single `ha_update_row()`. It writes no history row. ## Cause `heap_update()` and `heap_delete()` do not free the old blob chain outright. They park it, because the SQL layer keeps reading the pre-update row out of `record[1]` after `ha_update_row()` returns -- `binlog_log_row()` builds the before-image from it, and an `AFTER UPDATE` trigger reads `OLD.<blob>` from it. Those are zero-copy pointers straight into `HP_BLOCK`, so freeing the chain would make them dangle. `heap_write()` redeemed that parking unconditionally, before allocating. For the history row that is exactly the wrong moment, since it sources its blob from the chain the update just parked. The free put those records on the delete list, where the allocation immediately below handed them straight back as the history row's own chain -- with `hp_push_free_block()`'s free-list links already scribbled through the payload. Source and destination of the blob copy overlapped, and `record[1]` was left pointing at reused memory for the rest of the statement. ## What triggers the bug, and what reads the corrupted data Triggering statements are every `vers_insert_history_row()` caller reaching a `HEAP` table whose record buffer was filled by a read: single-table `UPDATE`, multi-table `UPDATE` (both the on-the-fly and the deferred `do_updates()` path), `INSERT ... ON DUPLICATE KEY UPDATE`, and the row-based replication applier. A versioned `UPDATE` that does not change the blob column is unaffected -- `heap_update()` keeps the chain and parks nothing. Three consumers then read corrupted data, all of them out of `record[1]` after the history-row write has recycled the chain: - the history row itself, as returned by `SELECT ... FOR SYSTEM_TIME ALL`; - the row-based binlog before-image, so the corruption reaches replicas; - `AFTER UPDATE` triggers reading `OLD.<blob>`, so whatever the trigger does with that value -- typically writing it to an audit table -- stores wrong data, and that write is itself replicated. The trigger case is the easiest to miss, because `LENGTH(OLD.<blob>)` is still correct: the length lives in the record buffer and survives, and only the payload has been recycled. A `BEFORE UPDATE` trigger on the same table reports the correct value, which localises the damage to the history-row write that happens between the two. ## Fix The parked chain already holds exactly the bytes the history row needs -- it is a verbatim copy of the same record. So instead of freeing it and allocating a duplicate, the new row adopts it: - `hp_flush_unaliased_blob_free()` redeems every parked chain **except** ones the record being written still sources blob data from. - `hp_write_blobs()` takes those over: the stored row points at the parked chain and no new chain is written. The pending slot is cleared only once every column has succeeded, so the rollback path can tell an adopted chain from an allocated one and leaves it parked rather than freeing it. Both record buffers stay valid, because the chain's contents are never disturbed. Adoption also needs no space at all, which matters at `max_heap_table_size`: the history row previously had to find room for a second copy of a blob that was already resident, and the parked chain was often the only reclaimable space. Aliasing is detected by exact pointer equality against the parked chain head. `hp_read_blobs()` hands out zero-copy pointers of exactly two forms -- the chain head, or `chain + recbuffer` -- and reassembles a multi-run chain into `info->blob_buff`, which can never alias, so the two comparisons in `hp_blob_sources_chain()` are complete and need no walk of the `HP_BLOCK` tree. Matching is per blob column, since `pending_blob_chains[i]` is the chain parked for column `i`, and that slot is `NULL` for a column the update did not change. An adopted chain may be longer than the adopting row's blob length: `UPDATE t SET f = LEFT(f,6)` leaves the shortened value pointing at the original data. That is harmless. `hp_read_blobs()` picks the chain layout from the chain's own flag byte rather than from the length, so a short read off a long chain still starts at the right offset, and `hp_free_run_chain()` walks `run_rec_count`/`next_cont` rather than the length, so the whole chain is still reclaimed. `REPLACE` was never affected: `HA_EXTRA_WRITE_CAN_REPLACE` makes `hp_read_blobs()` copy rather than hand out zero-copy pointers. Internal temporary tables never park -- they free their chains outright and do not allocate the array -- so `hp_write_blobs()` guards on it. A blob cannot itself be indexed in a `HEAP` table: `ha_heap` does not set `HA_CAN_INDEX_BLOBS`, so both `KEY (f(10))` and `UNIQUE (f)` are rejected at `CREATE` with `ER_BLOB_USED_AS_KEY`. There is therefore no versioned blob-as-key combination for adoption to get wrong. Blob key segments exist only for internal temporary tables, which never park a chain and are never versioned. ## Tests `storage/heap/hp_test_blob_alias-t.c` drives the sequence at the `heap_write()` API for a single-record chain, a zero-copy run and a multi-run chain, and checks both the stored row and the caller's buffer. It also pins adoption itself, by the stored row's chain pointer and by the growth of `block.last_allocated` across the write: exactly one record slot and no chain. The multi-run case is reassembled into `info->blob_buff` and so cannot alias; the test asserts which layout it got, so the coverage cannot silently degrade. `blob_vers_trigger` covers `OLD.<blob>` in triggers across single-table `UPDATE`, multiple blob columns, `ON DUPLICATE KEY UPDATE` and multi-table `UPDATE`, with `BEFORE UPDATE` alongside `AFTER UPDATE` on one table, and non-versioned `HEAP` and versioned `MyISAM` controls. `blob_versioning`, `blob_vers_odku`, `blob_vers_multi` and `blob_vers_repl` cover the history row itself for every `vers_insert_history_row()` caller and replication of the before-image. `blob_versioning` additionally covers the cases where only one of two blob columns changed, so the history-row write sees a mix of parked and empty slots; a blob shrunk in place with `LEFT()`, so the row and its history carry the same pointer with different lengths; an indexed table, so the key loop runs while `record[1]` still holds a zero-copy pointer and a `UNIQUE` key materializes stored blobs into `key_blob_buff`; and the two `ER_BLOB_USED_AS_KEY` rejections. |
||
|
Arcadiy Ivanov
|
6d9f06c01b |
MDEV-40033 Free-list contiguous block tracking for HEAP engine
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. |
||
|
Monty
|
ce0bc07763 |
MDEV-40029 Add support for bit fields to HEAP
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. |
||
|
Monty
|
1286c59ddf |
MDEV-40030 Add support for CHECK TABLE for memory tables
CHECK TABLE is now supported, but will only return ok or fail. Still good enough for testing heap table consistenty. |
||
|
Arcadiy Ivanov
|
11f859d9d3 |
Fix CI regressions from MDEV-38975 forward-port to main
Seven code fixes, a new test, and test re-recordings for issues found by CI on PR #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) - 'funcs_1.is_tables_is' ; re-recorded test as INFORMATION_SCHEMA.SLAVE_STATUS is now back as MEMORY table - `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 |
||
|
Arcadiy Ivanov
|
3331b76240 |
MDEV-38975: HEAP engine BLOB/TEXT/JSON/GEOMETRY support with indexable 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. |
||
PranavKTiwari
|
5e1d303aa6 |
MDEV-38839: Assertion (thd->state_flags & Open_tables_state::BACKUPS_AVAIL) || !thd->has_pending_row_events() failed in close_thread_tables on CREATE TABLE
Problem: CREATE TABLE ... SELECT ... FOR UPDATE involving a MyISAM temporary table can trigger an assertion in close_thread_tables() when binary logging is enabled in MIXED mode. Cause: MyISAM temporary tables do not support row-level locking, so SELECT ... FOR UPDATE acquires a write lock even when the access is read-only. This causes the table to be incorrectly classified as a write operation, preventing binlog_truncate_trx_cache() from running and eventually triggering the assertion. Fix: Update decide_logging_format() to use tbl->updating to distinguish actual table modifications from read-only accesses. For read-only access to temporary non-transactional tables, mark the statement as STMT_READS_TEMP_NON_TRANS_TABLE instead of treating it as a write operation. This prevents incorrect write classification and avoids the assertion failure. |
||
Aleksey Midenkov
|
4ea31db683 | MDEV-38839 table_list->updating flag comment | ||
|
Monty
|
23d8294273 | MDEV-31138 Enable the test spider/bugfix.mdev_29676 | ||
|
Monty
|
d0b279f185 |
MDEV-25292 Atomic CREATE OR REPLACE TABLE
Atomic CREATE OR REPLACE allows to keep an old table intact if the
command fails or during the crash. That is done by renaming the
original table to temporary name, as a backup and restoring it if the
CREATE fails. When the command is complete and logged the backup
table is deleted.
Atomic replace algorithm
Two DDL chains are used for CREATE OR REPLACE:
ddl_log_state_create (C) and ddl_log_state_rm (D).
1. (C) Log rename of ORIG to TMP table (Rename TMP to original).
2. Rename orignal to TMP.
3. (C) Log CREATE_TABLE_ACTION of ORIG (drops ORIG);
4. Do everything with ORIG (like insert data)
5. (D) Log drop of TMP
6. Write query to binlog (this marks (C) to be closed in
case of failure)
7. Execute drop of TMP through (D)
8. Close (C) and (D)
If there is a failure before 6) we revert the changes in (C)
Chain (D) is only executed if 6) succeded (C is closed on
crash recovery).
Foreign key errors will be found at the 1) stage.
Additional notes
- CREATE TABLE without REPLACE and temporary tables is not affected
by this commit.
set @@drop_before_create_or_replace=1 can be used to
get old behaviour where existing tables are dropped
in CREATE OR REPLACE.
- CREATE TABLE is reverted if binlogging the query fails.
- Engines having HTON_EXPENSIVE_RENAME flag set are not affected by
this commit. Conflicting tables marked with this flag will be
deleted with CREATE OR REPLACE.
- Replication execution is not affected by this commit.
- Replication will first drop the conflicting table and then
creating the new one.
- CREATE TABLE .. SELECT XID usage is fixed and now there is no need
to log DROP TABLE via DDL_CREATE_TABLE_PHASE_LOG (see comments in
do_postlock()). XID is now correctly updated so it disables
DDL_LOG_DROP_TABLE_ACTION. Note that binary log is flushed at the
final stage when the table is ready. So if we have XID in the
binary log we don't need to drop the table.
- Three variations of CREATE OR REPLACE handled:
1. CREATE OR REPLACE TABLE t1 (..);
2. CREATE OR REPLACE TABLE t1 LIKE t2;
3. CREATE OR REPLACE TABLE t1 SELECT ..;
- Test case uses 6 combinations for engines (aria, aria_notrans,
myisam, ib, lock_tables, expensive_rename) and 2 combinations for
binlog types (row, stmt). Combinations help to check differences
between the results. Error failures are tested for the above three
variations.
- expensive_rename tests CREATE OR REPLACE without atomic
replace. The effect should be the same as with the old behaviour
before this commit.
- Triggers mechanism is unaffected by this change. This is tested in
create_replace.test.
- LOCK TABLES is affected. Lock restoration must be done after new
table is created or TMP is renamed back to ORIG
- Moved ddl_log_complete() from send_eof() to finalize_ddl(). This
checkpoint was not executed before for normal CREATE TABLE but is
executed now.
- CREATE TABLE will now rollback also if writing to the binary
logging failed. See rpl_gtid_strict.test
backup ddl log changes
- In case of a successfull CREATE OR REPLACE we only log
the CREATE event, not the DROP TABLE event of the old table.
ddl_log.cc changes
ddl_log_execute_action() now properly return error conditions.
ddl_log_disable_entry() added to allow one to disable one entry.
The entry on disk is still reserved until ddl_log_complete() is
executed.
On XID usage
Like with all other atomic DDL operations XID is used to avoid
inconsistency between master and slave in the case of a crash after
binary log is written and before ddl_log_state_create is closed. On
recovery XIDs are taken from binary log and corresponding DDL log
events get disabled. That is done by
ddl_log_close_binlogged_events().
On linking two chains together
Chains are executed in the ascending order of entry_pos of execute
entries. But entry_pos assignment order is undefined: it may assign
bigger number for the first chain and then smaller number for the
second chain. So the execution order in that case will be reverse:
second chain will be executed first.
To avoid that we link one chain to another. While the base chain
(ddl_log_state_create) is active the secondary chain
(ddl_log_state_rm) is not executed. That is: only one chain can be
executed in two linked chains.
The interface ddl_log_link_chains() was defined in "MDEV-22166
ddl_log_write_execute_entry() extension".
Atomic info parameters in HA_CREATE_INFO
Many functions in CREATE TABLE pass the same parameters. These
parameters are part of table creation info and should be in
HA_CREATE_INFO (or whatever). Passing parameters via single
structure is much easier for adding new data and
refactoring.
Aria changes:
- Fixed issue in Aria engine with CREATE + locked tables
that data was not properly commited in some cases in
case of crashes.
InnoDB related changes (by Marko Mäkelä):
- table_name_t::is_create_or_replace(): A new predicate to check for
CREATE OR REPLACE TABLE will rename an old table to
and eventually drop after creating the replacement.
- dict_table_t::parse_name(): Do acquire MDL on #sql-create- names
for partitioned tables.
- dict_table_rename_in_cache(): On CREATE OR REPLACE TABLE ... SELECT,
forget the original dict_table_t::mdl_name so that purge will
acquire MDL on the #sql-create- name instead. In this way, the
MDL_EXCLUSIVE that the CREATE OR REPLACE TABLE holds on the
user-visible name will not unnecessarily block any purge of old history
until the very end when the #sql-create- table will be dropped.
- ha_innobase::delete_table(): Do not check FOREIGN KEY consistency
when dropping an #sql-create- table.
- row_rename_table_for_mysql(): Update SYS_FOREIGN.ID also
when renaming to #sql-create- in order to avoid any
duplicate key error when CREATE OR REPLACE TABLE is
creating some FOREIGN KEY constraints by names
that existed in the old table.
Other changes:
- Removed some auto variables in log.cc for better code readability.
- Fixed old bug that CREATE ... SELECT would not be able to auto repair
a table that is part of the SELECT.
- Marked MyISAM that it does not support ROLLBACK (not required but
done for better consistency with other engines).
- maria_create_trn_for_mysql() does not register a new transaction
handler for commits. This was needed to ensure create or replace
will not end with an active transaction.
- We do not get anymore warnings about "Engine not supporting atomic
create" when doing a legal CREATE OR REPLACE on a table with
foreign key constraints.
- Updated VIDEX engine flags to disable CREATE SEQUENCE.
- Removed mysql_mutex_unlock(&LOCK_gdl) / mysql_mutex_lock(&LOCK_gdl)
around calls to binlog as these are unsafe. The binlog code uses
global variables that needs protection from other caller.
- EITS data is preserved if create or replace fails if
drop_before_create_or_replace=OFF. If ON, then create or replace
will drop EITS before the drop of the original table (as before).
- Using CREATE OR REPLACE on a encrypted table that the user cannot
decrypt will fail instead of replacing the encrypted table.
The encrypted table will unchanged.
Known issues:
- One cannot use create or replace on an InnoDB tables that has foreign
key references point to it
- CREATE OR REPLACE TEMPORARY table is not full atomic. Any conflicting
table will always be dropped before creating a new one. (Old behaviour).
Bug fixes related to this MDEV:
MDEV-36435 Assertion failure in finalize_locked_tables()
MDEV-36439 Assertion `thd_arg->lex->sql_command != SQLCOM_CREATE_SEQUENCE...
MDEV-36498 Failed CoR in non-atomic mode no longer generates DROP in RBR...
MDEV-36508 Temporary files #sql-create-....frm occasionally stay after
crash recovery
MDEV-38479 Crash in CREATE OR REPLACE SEQUENCE when new sequence cannot
be created
MDEV-36497 Assertion failure after atomic CoR with Aria under lock in
transactional context
MDEV-36501 EITS data is lost after failed attempt to CREATE OR REPLACE
table
MDEV-36493 Atomic CREATE OR REPLACE ... SELECT blocks InnoDB purge
MDEV-39367 MSAN/valgrind errors in temp_file_size_cb_func,
main.tmp_space_usage fails
MDEV-39446 Atomic CREATE OR REPLACE fails if a table cannot be decrypted
MDEV-40776 Atomic CREATE OR REPLACE silently breaks the foreign key
MDEV-40765 Assertion `"unexpected references" == 0' failed upon failing
CREATE OR REPLACE
MDEV-40994 Atomic CoR: Transaction not registered for MariaDB 2PC, but
transaction is active
Reverted commits:
- MDEV-36685 "CREATE-SELECT may lose in binlog side-effects of
stored-routine" as it did not take into account that it safe to
clear binlogs if the created table is non transactional and there
are no other non transactional tables used.
This was done because it caused extra logging when it is not
needed (not using any non transactional tables) and it also did
not solve side effects when using statement based loggging.
|