Console View
|
Categories: connectors experimental galera main |
|
| connectors | experimental | galera | main | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40920 Say whether a value cut in a group reached the answer A TEXT value longer than `group_concat_max_len` is cut on its way into `blob_storage`, in `Field_blob::handle_group_concat()`. That happens while the group is being built, not when the answer is put together, so whether the answer is any shorter for it depends on whether the row carrying the value reaches the answer at all. Every such cut was reported as `ER_CUT_VALUE_GROUP_CONCAT`, which says that the answer lost something the user asked for, and that is true of only some of them. `Blob_mem_storage` now writes one byte in front of every value it stores and returns the pointer past it, so `was_cut()` answers for any value a reader holds a pointer to. `Field_blob::store()` sends every blob of a table that has a `Blob_mem_storage` through `handle_group_concat()`, so no value in that storage is without the byte, and `Field_blob::get_ptr()` on the record of a row hands back exactly the pointer that was stored. `dump_leaf_key()` reads the mark off each row it appends and sets `value_cut_in_result`. `val_str()` then reports: 1. **A warning**, `ER_CUT_VALUE_GROUP_CONCAT`, when a row that reached the answer carried a cut value. The answer is short by what was cut. The result being cut at `gconcat_max_len()` already gives that same warning, and a group that hits both is told once, not twice. 2. **A note**, `ER_CUT_VALUES_WHILE_PROCESSING`, when a value was cut but no row carrying one reached the answer. The answer may well be what a larger limit would have given. One note per aggregate is enough for a statement however many groups had a value cut, and `cleanup()` clears the mark so a statement run again gets its own. Reporting the loss as a warning keeps a strict `sql_mode` aborting on it, which it does because `THD::raise_condition()` promotes a warning and never promotes a note. `ST_COLLECT` is not affected. It reports `ER_CUT_VALUE_GROUP_CONCAT` itself, against `group_collect_max_len`. `main.gconcat_cut_note` covers the split with one group holding a short value and a long one, where a `LIMIT` alone decides which of them the answer is built from, over both the sort tree and the duplicate filter. `main.func_gconcat` shows the granularity: of five groups at `group_concat_max_len=499999`, the one holding exactly 499999 bytes is the one that does not warn. Note that `blob_storage` only exists when the aggregate has an `ORDER BY` or a `DISTINCT` and a blob field, so this is the only shape in which a value is cut this way. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41293: FederatedX: connection state after a net_read_timeout Problem: When reading the reply of the remote server times out, the client library compiled into the server returns from cli_safe_read() without closing the connection and without setting a client error. FederatedX took the raw ER_NET_READ_INTERRUPTED for an error of the remote server and went on using a connection that still had the reply of the failed statement to arrive. As a result: * the next statement on that connection failed in various ways, either with ER_QUERY_ON_FOREIGN_DATA_SOURCE ("Server has gone away") or with an error packet that mysqltest took for a malformed packet; * inside a transaction, the following statements continued on a new remote session in autocommit mode, so they were committed on the remote server one by one even though the local transaction was later rolled back. The savepoints of the lost session were also still remembered by FederatedX. Fix: * federatedx_io_mysql::close_on_net_timeout(): after such a timeout in actual_query() or store_result(), close the connection with mysql_close() (end_server() would leak, as the next query runs mysql_init() on the structure) and report CR_SERVER_LOST. Forget the autocommit mode and the savepoints of the lost session, and mark the local transaction to be rolled back, so that a new connection is switched to SET AUTOCOMMIT=0 again before its first statement. * ha_federatedx::info() reported the errors of the client library as server errors. Report them as ER_QUERY_ON_FOREIGN_DATA_SOURCE, like batch_update_delete() does, using the same is_client_library_errno(). Test: federatedx_pushdown_upd_del covers the timeout of a pushed down UPDATE, the statements after it, and the same inside a transaction. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-36356 Server crash in Item::save_int_in_field with Window functions A tail (ORDER BY / LIMIT / locking clause) that follows a parenthesized query expression is parsed while the select inside the parentheses is the current one, so everything the tail registers - its window functions, the window specifications they introduce and the units of its subqueries - ends up registered in that inner select. When the parenthesized query expression already has a tail of its own, LEX::add_tail_to_query_expression_body_ext_parens() wraps it into a derived table and attaches the new tail to the wrapping select. The registrations were left behind in the inner select, so the ORDER BY of the wrapping select contained window function items that no select had registered: they never got a Window_funcs_sort, were never computed, and their result field was read unset. The subqueries of the tail kept the inner select as their master and name resolution context. Move these registrations to the wrapping select in the new move_tail_registrations(): - The window functions of the tail are the ones its ORDER BY items contain, looked up with Item::walk() and walk_subquery == FALSE so that a window function belonging to a subquery of the tail is not found and stays registered where it was parsed. Each one takes along the window specification it introduced; "OVER win_name" has none of its own, it is looked up by name at fix_fields() time and never reaches window_specs. - Item::walk() does not descend into a window specification, so the items of a moved specification's PARTITION BY and ORDER BY lists are not re-targeted by Lex_order_limit_lock::set_to() and are pointed at the wrapping select's name resolution context here. - The subquery units of the tail are the ones registered in front of the first unit the inner select had when the tail started to be parsed; the grammar remembers it in a mid-rule action. There is no equivalent of the walk above for them: a unit records neither the clause it came from nor its position in the parsed text. They are excluded from the inner select, re-registered in the wrapping select and their Item_subselect::parent_select is updated. Per-select parse state that is not a registration (n_sum_items, with_sum_func, with_rownum, ftfunc_list, uncacheable, and the select_n_where_fields of the tail's own fields) is deliberately left behind, where it is merely over-counted. Also remove the unreachable !unit check - unit is dereferenced on entry - and fix the indentation of the surrounding block. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41243, MDEV-41273 Don't freeze mem_root after engine pushdown When a statement is pushed down to an engine, the once-per-statement part of the optimization (e.g. the first_cond_optimization part of JOIN::optimize(), or the whole of JOIN::optimize() for a pushed down UNION) is skipped. Re-executing the same prepared statement or stored routine without pushdown then allocated from a mem_root already marked ROOT_FLAG_READ_ONLY, hitting an assertion in alloc_root(). Add LEX::dont_freeze_mem_root, set by the pushdown_handler and derived_handler constructors so it covers every engine implementing them. Prepared_statement::execute_loop(), sp_head::execute() (through sp_head::dont_freeze_mem_root) and the SP instruction reparse path no longer freeze the mem_root of such statements. The flag is only declared and used in PROTECT_STATEMENT_MEMROOT builds. The flag is per execution for prepared statements: Prepared_statement::execute_loop() clears it after each execution, so the mem_root is frozen again after the first later execution that is not pushed down. For a stored routine the instruction's LEX keeps the flag, so once one instruction has been pushed down the routine's mem_root stays unfrozen, even if later executions are not pushed down, unless the routine is re-parsed into a new LEX. Add tests for prepared SELECT, UNION, UPDATE and DELETE, prepared EXPLAIN of a pushed UNION, derived tables, stored procedures, functions, triggers, cursors and metadata-invalidation reparse, with pushdown switched on and off. Most prepared statement and routine tests check the statements received by the remote server, or EXPLAIN, to verify that pushdown actually happened. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40692 GROUP_CONCAT replays a group when an OFFSET skips every row Nothing says how many times a statement asks for the result of a group, and the answer must not depend on it. A `HAVING` clause on the alias is the shortest statement that asks twice, and it returns a different value than the same aggregate asked once: SELECT GROUP_CONCAT(a ORDER BY a LIMIT 2 OFFSET 4) v FROM t1; -> (empty) SELECT GROUP_CONCAT(a ORDER BY a LIMIT 2 OFFSET 4) v FROM t1 HAVING v LIKE '%'; -> a,b `val_str()` walks only while `result_finalized` is false, and `dump_leaf_key()` raises that flag for the first row it writes. A row that falls inside the offset is skipped by an earlier return, which decrements the offset counter and leaves the flag alone. The row-limit arm immediately above it does raise the flag before its own early return, so two adjacent early returns behave differently. A walk in which every row was skipped therefore writes nothing and records nothing. The next caller walks again with the offset already spent, and the rows skipped the first time are appended to a result buffer that was handed over once already. Once the duplicate filter has spilled to disk the second walk is worse than wrong. `Unique::reset()` documents the contract: Clear the tree and the file. You must call reset() if you want to reuse Unique after walk(). The first walk flushed the tree and emptied it, so the second flushes an empty tree, appending a chunk that holds no rows. `merge_walk()` reads nothing back from it and fails `DBUG_ASSERT(bytes_read)`. A build without assertions goes on to take keys from that chunk. Set `result_finalized` where the walk block ends, so that it records every path that has consumed the filter rather than only the paths that wrote a row. On the release branches only the form without `ORDER BY` reaches the duplicate filter. Since MDEV-21879 the `DISTINCT ... ORDER BY` combination builds its result the same way, so both forms can reach the assertion here. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Monty
monty@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Fixed internal temporary buffer sizes to use tmp_memory_table_size tmp_memory_table_size is limiting the size of internal temporary memory tables. max_heap_table_size is there to limiting the size of explictely created memory tables. max_heap_table_size can be much larger than tmp_memory_table_size as the memory used by temporary tables is in the control of the user. This commit changes the usage of max_heap_table_size for internal buffers to min(max_heap_table_size, tmp_memory_table_size), like we do for internal temporary tables. This changes the in memory buffer allocations for: - GROUP_CONCAT() - Calculating the cost for scanning memory tables (the original code was wrong here as it used the wrong size for memory tables). - ANALYZE TABLE buffer sizes for calculating distinct column values Other things: - Add THD::ram_limitation() to provide consistent memory limitations in all code that used variables.tmp_memory_table_size as buffers. If tmp_memory_table_size == 0, then 8192 is used. This replaces Item_sum::ram_limitation which used 1024 as min buffer, which is way to little for any practical case. - Added security guard in heap_prepare_hp_create_info to ensure that max_table_size is calculated same way as in MariaDB server. - Fixed initial memory allocations for Item_func_group::concat which allocated 'max allowed memory' at start. Now it allocates only 1/16 of that memory at start. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39245: Added ASCII validation at two points using my_string_repertoire(): 1. Built-in plugins (plugin_init): A non-ASCII builtin name is a developer error. Added a check using my_string_repertoire() that prints an error via print_init_failed_error() and returns 1 to abort server startup. 2. Dynamically loaded plugins (plugin_add): Added a pre-validation loop before the registration loop. If any plugin name in the .so is non-ASCII, the entire .so is rejected before any plugin is registered, preventing partial state where valid plugins from the same .so get left INACTIVE in the hash. Added test plugins and MTR tests to verify both rejection of non-ASCII named plugins and successful loading of ASCII named plugins. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41228 Add Item::new_select_copy() for split materialization A query like SELECT * FROM t0 WHERE t0.b IN (SELECT t.n2 FROM t1, (SELECT n1, n2 FROM t1 GROUP BY n1, n2) t WHERE t.n1 = t1.n1 AND t1.c1 = 'a' UNION SELECT -1) returned only the row of t0 that was read first, when the derived table t was split-materialized. The IN->EXISTS transformation wraps t0.b in an Item_cache owned by the Item_in_optimizer, and injects t.n2 = <cache>(t0.b) into the subquery. The optimizer then pushes an equality on n2 into the derived table, so that the table only has to be filled with the matching rows. TABLE::add_splitting_info_for_key_field() built the right side of that equality as a deep copy of <cache>(t0.b). A copy of a cache is a second cache: it read t0.b for the first row of t0 and kept that value, because only the Item_in_optimizer stores new values, and it stores them into the original cache. Every later refill of the derived table was filtered on the first row's value. The right side is an expression of the outer select used inside the select that specifies the derived table. Add a copy hierarchy for that purpose. Item::new_select_copy_with_checks(thd, item_select) returns a copy for use in a select nested in item_select, the select the item belongs to. In the nested select the copy is an outer reference to item_select: - An Item_field is copied and its depended_from set to item_select. This is what add_splitting_info_for_key_field() did before with set_fields_as_dependent_processor, except that depended_from now names the select the column belongs to instead of the nested select. - An Item_direct_view_ref, a column of a merged view or derived table, is also copied like a column: the copy refers to a new_select_copy() of the item the view column stands for. Copying it this way, as the deep copy did, keeps a column of a constant NULL-complemented table constant in the pushed equality (the MDEV-30081 test in main.derived_cond_pushdown still shows ref "const" for its LATERAL DERIVED table). - Any other item that depends on tables of item_select is not copied. The copy is an Item_direct_ref to the original item, with depended_from set to item_select. So the nested select reads the cache, aggregate or subquery that its owner keeps up to date, instead of a copy of it that nobody refreshes. Item_default_value is treated this way too. - An item that depends on no table of item_select is shared. The copy is fixed, and only refers to tables outside the select it is used in, which the wrapper asserts. add_splitting_info_for_key_field() now uses it for the right side of the pushed equality, and no longer walks the copy with set_fields_as_dependent_processor or calls update_used_tables() on it. The debug-only copy of key_field->val used for the lookup still uses deep_copy_with_checks(). The check_deep_copy() call on the right side is removed, as it is no longer meant to be a deep copy. This commit was prepared with Claude Code (Opus 5), which designed and wrote the new_select_copy() hierarchy, switched the split equality to it, wrote the three-row test from the earlier rr analysis and confirmed that it fails without the change, and ran main and the SQL-layer suites in normal and --ps-protocol mode. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-28610 : Assertion marked_for_read() failed upon range select with virtual column in index. Problem: Executing a range select using an index that includes virtual/generated columns (e.g. SELECT ... WHERE c BETWEEN ... ORDER BY d) could trigger: Assertion 'marked_for_read()' failed in Field access during execution. Cause: The keyread initialization path was populating table->read_set using mark_index_columns(), which follows the non-recursive index marking path. This only marks direct index fields and does not resolve dependencies of virtual/generated columns. As a result, when an index contained virtual columns, their dependent base columns were not included in read_set, leading to invalid access during later evaluation of virtual columns. Fix: Replace mark_index_columns() with mark_index_columns_for_read() in the keyread initialization path. This ensures the recursive read-marking logic is used, so virtual/generated column dependencies are correctly propagated into read_set before execution. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41318 Incorrect recovery of FILE_CREATE after FILE_RENAME recv_recovery_from_checkpoint_start(): Before applying any page-level log records by invoking recv_scan_log(true, parser), invoke recv_rename_files() to replay FILE_RENAME records. In this way, recv_sys_t::recover_deferred() will not overwrite any old data files that had been renamed before a new file was created. recv_sys_t::validate_checkpoint(): Refer to recv_sys.scanned_lsn rather than recv_sys.lsn, because recv_scan_log(false, parser) may reset the latter to rewound_lsn in case we run out of memory for recv_sys.pages. In any case, the second call to recv_scan_log() will proceed to the end of the log and parse all records. Reviewed by: Thirunarayanan Balathandayuthapani |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41228 Remove Item::deep_copy() Every copy of an item tree that the server makes now goes through a copy hierarchy named after its purpose: same_select_copy(), new_select_copy() or pushdown_copy(). deep_copy() was left with three callers: - Virtual_column_info::clone(), which has had no callers since ef5adf52076 (MDEV-24274), - the "item_cache_clones" debug hook in Item::get_cache(), and - the "split_materialized_clones" debug hook in TABLE::add_splitting_info_for_key_field(), which used a deep copy of key_field->val for the lookup into the split-materialized table. The debug hooks were added only to exercise deep_copy() on Item_cache items, together with Item::check_deep_copy(), which compared a deep copy with its original. Remove all of them, with deep_copy_with_checks(), the pure virtual deep_copy() and its overrides, Item_cache::deep_copy_cache() and check_deep_copy(). Item::get_cache() is inline again, as it was before the debug hook. main.item_clone_debug ran its query under the split_materialized_clones flag and is removed; the same query, with three rows in t0 so that a stale value shows in the result, is in main.derived_split_innodb. Comments that pointed at deep_copy_with_checks() now point at the copy functions that replace it. This commit was prepared with Claude Code (Opus 5), which removed the deep_copy() hierarchy, its debug hooks, check_deep_copy() and Virtual_column_info::clone(), restored the inline Item::get_cache(), removed main.item_clone_debug, and ran main and the SQL-layer suites in normal and --ps-protocol mode. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-21879 GROUP_CONCAT(DISTINCT ORDER BY) is wrong when Unique spills `Item_func_group_concat::add()` decided whether a row was a duplicate by checking whether `Unique::elements_in_tree()` had grown after `unique_add()`: uint count= unique_filter->elements_in_tree(); unique_filter->unique_add(get_record_pointer()); if (count == unique_filter->elements_in_tree()) row_eligible= FALSE; `Unique` flushes its whole in-memory tree to disk when it runs out of memory, and `elements_in_tree()` only counts what is still in memory. After the first flush the test says nothing about the rows that were already spilled. **MDEV-11563** made this harmless for `GROUP_CONCAT(DISTINCT x)` by building the result in `val_str()` from `unique_filter->walk()`, which merges the spilled parts back in. It left the `ORDER BY` case alone. There the result comes from the sort tree, which `add()` fills gated by `row_eligible`, so the defect is still fully live. Both directions of the failure are reachable, depending on how often the filter flushes relative to the insert: 1. Duplicates reach the result. 100 rows holding 50 distinct values give all 100 values back. 2. Rows are lost. 30 distinct rows of 2000 bytes give one value back. `JSON_ARRAYAGG(DISTINCT x ORDER BY y)` fails in the same way. Fixed by not filling the sort tree from `add()` when `DISTINCT` is used. `val_str()` now walks the merged `unique_filter` into the sort tree and then walks the sort tree, so the rows are sorted after the duplicate filtering is complete instead of during it. `Unique::walk()` merges everything it flushed, so the sort tree can be handed more rows than fit in memory. `insert_to_order_tree()` repacks it on the same memory budget `add()` used, and a walk that runs out of memory sets `result_cut`, so the user gets a cut value warning rather than a silently short result. **Behaviour change.** `ORDER BY` does not order rows that tie on the ordering expression, and which of them comes first changes here. It used to follow the order the rows were read in; it now follows the order the duplicate filter keeps them in. Unlike the old order, the new one depends on neither the memory available nor the physical row order. `main.gconcat_distinct_spill` checks that, and `main.func_gconcat` records one such tie. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41318 Incorrect recovery of FILE_CREATE after FILE_RENAME recv_recovery_from_checkpoint_start(): Before applying any page-level log records by invoking recv_scan_log(true, parser), invoke recv_rename_files() to replay FILE_RENAME records. In this way, recv_sys_t::recover_deferred() will not overwrite any old data files that had been renamed before a new file was created. recv_sys_t::validate_checkpoint(): Refer to recv_sys.scanned_lsn rather than recv_sys.lsn, because recv_scan_log(false, parser) may reset the latter to rewound_lsn in case we run out of memory for recv_sys.pages. In any case, the second call to recv_scan_log() will proceed to the end of the log and parse all records. Reviewed by: Thirunarayanan Balathandayuthapani |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
ParadoxV5
paradox.ver5@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-32947 Async conflicts with semi-sync in multi-source `@@rpl_semi_sync_slave_enabled` can be changed without stopping replication, because it takes effect when a connection is established. But when it takes effect, it applies to *all* replication connections. This means that starting an async connection will switch any ongoing semi-sync connections to async as well, and vice versa, which will cause those connections to fail to replicate in the wrong mode. This fix resolves this oversight by changing the value application to specific to only the establishing connection (technically, moving the singleton’s field `Repl_semi_sync_slave::m_slave_enabled` to the instance variable `Master_info::semi_sync_enabled`). This designs build toward MDEV-40941, which proposes migrating `@@rpl_semi_sync_slave_enabled` to a proper per-connection config. Note, `Master_info::semi_sync_reply_enabled` must remain separate, because it only disables ACK replying in async slave fallback, whereas the semi-sync slave setting also controls whether the IO thread should expect additional semi-sync flags in event packets. The status variable `Rpl_semi_sync_slave_status` is left referencing ??? ??? Reviewed-by: Brandon Nesterenko <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40946 Two heap blob tests fail with the embedded server Both tests exercise server facilities that an embedded build does not have, so neither can run there. `INSERT DELAYED` has no delayed insert thread in an embedded build. The whole facility sits inside `#ifndef EMBEDDED_LIBRARY`, including the check that routes a delayed statement away from the ordinary insert path, so the statement is an ordinary insert and `DELAYED_WRITES` stays at 0. `ALTER TABLE ... LOCK=NONE` is never online in an embedded build. `online` is hard-wired to `false` when `HAVE_REPLICATION` is undefined, and `my_global.h` leaves it undefined for `EMBEDDED_LIBRARY`. The source lock is therefore not downgraded, the `alter_table_online_downgraded` sync point is never reached, and the test's `WAIT_FOR downgraded` runs out its `debug_sync` timeout. Skip both with `include/not_embedded.inc`, as `main.delayed` and `main.alter_table_online_debug` already do. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-28610 : Assertion marked_for_read() failed upon range select with virtual column in index. Problem: Executing a range select using an index that includes virtual/generated columns (e.g. SELECT ... WHERE c BETWEEN ... ORDER BY d) could trigger: Assertion 'marked_for_read()' failed in Field access during execution. Cause: The keyread initialization path was populating table->read_set using mark_index_columns(), which follows the non-recursive index marking path. This only marks direct index fields and does not resolve dependencies of virtual/generated columns. As a result, when an index contained virtual columns, their dependent base columns were not included in read_set, leading to invalid access during later evaluation of virtual columns. Fix: Replace mark_index_columns() with mark_index_columns_for_read() in the keyread initialization path. This ensures the recursive read-marking logic is used, so virtual/generated column dependencies are correctly propagated into read_set before execution. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41318 Incorrect recovery of FILE_CREATE after FILE_RENAME recv_recovery_from_checkpoint_start(): Before applying any page-level log records by invoking recv_scan_log(true, parser), invoke recv_rename_files() to replay FILE_RENAME records. In this way, recv_sys_t::recover_deferred() will not overwrite any old data files that had been renamed before a new file was created. recv_sys_t::validate_checkpoint(): Refer to recv_sys.scanned_lsn rather than recv_sys.lsn, because recv_scan_log(false, parser) may reset the latter to rewound_lsn in case we run out of memory for recv_sys.pages. In any case, the second call to recv_scan_log() will proceed to the end of the log and parse all records. Reviewed by: Thirunarayanan Balathandayuthapani |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Alexey Yurchenko
alexey.yurchenko@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MGL-301 Add MTR tests and fix error reporting | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41293: FederatedX: connection state after a net_read_timeout Problem: When reading the reply of the remote server times out, the client library compiled into the server returns from cli_safe_read() without closing the connection and without setting a client error. FederatedX took the raw ER_NET_READ_INTERRUPTED for an error of the remote server and went on using a connection that still had the reply of the failed statement to arrive. As a result: * the next statement on that connection failed in various ways, either with ER_QUERY_ON_FOREIGN_DATA_SOURCE ("Server has gone away") or with an error packet that mysqltest took for a malformed packet; * inside a transaction, the following statements continued on a new remote session in autocommit mode, so they were committed on the remote server one by one even though the local transaction was later rolled back. The savepoints of the lost session were also still remembered by FederatedX. Fix: * federatedx_io_mysql::close_on_net_timeout(): after such a timeout in actual_query() or store_result(), close the connection with mysql_close() (end_server() would leak, as the next query runs mysql_init() on the structure) and report CR_SERVER_LOST. Forget the autocommit mode and the savepoints of the lost session, and mark the local transaction to be rolled back, so that a new connection is switched to SET AUTOCOMMIT=0 again before its first statement. * ha_federatedx::info() reported the errors of the client library as server errors. Report them as ER_QUERY_ON_FOREIGN_DATA_SOURCE, like batch_update_delete() does, using the same is_client_library_errno(). Test: federatedx_pushdown_upd_del covers the timeout of a pushed down UPDATE, the statements after it, and the same inside a transaction. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41020 A `MEMORY` table refuses a row it just held `hp_alloc_from_tail()` tests the table memory ceiling before it decides whether the leaf it is about to use has to be allocated or is merely being reclaimed. Reclaiming adds no memory, so on that branch the test answers a question nobody asked. The state it misjudges is routine. `hp_find_free_hash()` allocates an index leaf without consulting `max_table_size`, and no leaf is smaller than `heap_min_allocation_block`, so a table with two hash indexes is already over a 32K ceiling once it holds its first row. That is legal: the ceiling is only tested when the record cursor lands on a leaf boundary, and the first row tests it while both counters are still zero. `hp_shrink_tail()` puts the cursor back on a leaf boundary whenever it empties the tail, and `data_length` goes on counting the leaf, which is still allocated. The next write re-reads that sum and reports `HA_ERR_RECORD_FILE_FULL` for a row the table held a moment earlier. Move the ceiling test to the arm that calls `hp_get_new_block()`. The `max_records` row-count test stays where it is, because it caps rows however the slot is obtained. **Why a master and a slave disagreed about the same statement.** `REPLACE INTO t SELECT * FROM t` feeds the row back out of the table, so the record buffer still points into the chain that the delete parked; the chain is adopted rather than freed and the cursor never moves. The slave builds the row from the replication event buffer, nothing points into the parked chain, it is freed, the tail empties and the write is refused. Replication is one way to reach the state, not the cause: a targeted `DELETE` and a re-`INSERT` on a single server reach it too. Tests: `heap.blob_delete_reinsert_ceiling` covers the single-server route and checks that a table that genuinely needs more memory is still refused; `heap.blob_replace_repl_ceiling` covers the reported master/slave divergence; `hp_test_freelist` test 22 pins the branch itself. Each fails without the change. **Storing the row limit as a ceiling.** `heap_create()` recorded its `max_records` argument in the share unchanged, so 0 arrived there still meaning "no limit" and every reader had to special-case it. The row-count test above carried that as a second condition, `&& info->max_records`. Write `NO_LIMIT_RECORDS` into the share instead. The share then always holds a real ceiling and the write path tests one value. That frees 0 to mean on the share what it says, a table that accepts no rows, which is the natural way to create one that is never written to; only `heap_create()`'s argument keeps using 0 for "no limit". `ha_heap::info()` multiplies `max_records` by the record length to report `MAX_DATA_LENGTH`. An unlimited table now reports `~(my_off_t) 0` rather than a product that would overflow. Nothing reaches that branch through SQL, because `ha_heap::create()` derives `max_records` from the memory ceiling and a `MAX_ROWS` clause only lowers it. Measured before and after, the reported `MAX_DATA_LENGTH` is unchanged for a plain `MEMORY` table and for one created with `MAX_ROWS`. The unit tests wrote the old convention themselves and move with it: `hp_test_freelist` assigned `share->max_records= 0` to lift the limit, which now reads as a limit of zero rows, and `hp_test_block_size` asserted the share kept the 0. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-30297 Server crash / assertion failure in Compare_identifiers::operator upon dropping period with empty name Problem: ALTER TABLE ... DROP PERIOD IF EXISTS FOR can trigger an assertion failure in debug builds and crash in non-debug builds. Cause: Lex_cstring_with_compare::streq() may invoke Compare_identifiers on default-constructed Lex_cstring objects (str == NULL, length == 0). Compare_identifiers assumes non-null string pointers and asserts or crashes when called with a null pointer. Fix: Add null-pointer handling to Lex_cstring_with_compare::streq() and avoid calling Compare_identifiers when either string pointer is null. This prevents invalid comparisons while preserving existing behavior for initialized strings. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-30297 Server crash / assertion failure in Compare_identifiers::operator upon dropping period with empty name Problem: ALTER TABLE ... DROP PERIOD IF EXISTS FOR can trigger an assertion failure in debug builds and crash in non-debug builds. Cause: Lex_cstring_with_compare::streq() may invoke Compare_identifiers on default-constructed Lex_cstring objects (str == NULL, length == 0). Compare_identifiers assumes non-null string pointers and asserts or crashes when called with a null pointer. Fix: Add null-pointer handling to Lex_cstring_with_compare::streq() and avoid calling Compare_identifiers when either string pointer is null. This prevents invalid comparisons while preserving existing behavior for initialized strings. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Alexey Yurchenko
alexey.yurchenko@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MGL-301 Add MTR tests and fix error reporting | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41228 Add Item::pushdown_copy() for condition pushdown Condition pushdown into materialized derived tables and views, into materialized IN subqueries, and from HAVING into WHERE copies parts of a condition with deep_copy_with_checks(). These copies are all made for the same purpose. The pushdown code transforms the copy, replacing the columns it refers to with what the target select computes them from, then walks it with cleanup_excluding_const_fields_processor and fixes it again in the target select. That walk cleans up every node it reaches, and Item_field::cleanup() drops the field. So none of these copies may share a node with the original, including the copies of select list items that the transformers insert: those would be cleaned up under the select list. This rules out same_select_copy() for the transformers, although their copies stay in the same select, and needs a copy hierarchy of its own. Item::pushdown_copy_with_checks() calls the protected virtual Item::pushdown_copy(). It copies every node, and does not use deep_copy() at any level. A failure to copy is a valid answer: the condition is then not pushed down. - The default, in Item, copies the node with surface_copy(). - Item_func_or_sum, Item_ref, Item_cond, Item_row, Item_default_value, Item_direct_ref_to_item and Item_assoc_array copy their child items too. Item_func_or_sum copies its arguments before creating its own copy, so that an argument that cannot be copied leaves no half-made copy behind, whose cleanup() would free what it still shares with the original. - Item_cache copies its example, unless that is or contains an aggregate or window function, which cannot be copied. - Item_bool_rowready_func2, Item_func_case_simple and Item_func_in give the copy its own comparators and value array. - Item_outer_ref and Item_cache_fbt are copied as a single node. - The classes that deep_copy() refuses to copy refuse here too: subqueries, parameters, SP variables that are not plain values, MATCH, the spatial relations, Item_cache_row, Item_cache_wrapper, Item_type_holder and the PROCEDURE items. The copies made by Item::build_pushable_cond(), Item_equal::create_pushable_equalities(), pushdown_cond_for_derived(), st_select_lex::build_cond_for_grouping_fields() and the derived_field, grouping_field and in_subq_field transformers for WHERE now use pushdown_copy_with_checks(). The behaviour is unchanged, so there is no test. This commit was prepared with Claude Code (Opus 5), which found that the pushdown cleanup walk rules out shared nodes in these copies, wrote the pushdown_copy() hierarchy from the deep_copy() overrides, switched the pushdown call sites to it, found and fixed the half-made copy crash that main.derived_cond_pushdown (MDEV-17177) showed in its first version, and ran main and the SQL-layer suites in normal and --ps-protocol mode. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41007 Warn when GROUP_CONCAT(DISTINCT) loses rows silently Give a warning when `GROUP_CONCAT(DISTINCT x)` or `JSON_ARRAYAGG(DISTINCT x)` returns only part of a group, or nothing at all, because the walk of the duplicate filter failed. The result is wrong rather than deliberately cut, and nothing was said about it. Both build their result in `val_str()` by walking `unique_filter`, and threw the walk's return value away. `Unique::walk()` reports its own failures through it, from allocating the merge buffer to reading back the chunks it merged, so a failure gave a short result, or an empty one, in silence. The return value cannot be used on its own. `dump_leaf_key()` also stops the walk, for two reasons that are not failures: it cuts the result at `group_concat_max_len`, which it already reports by setting `result_cut`, and it stops without losing anything once the `LIMIT` is used up. Reporting every non-zero return as a cut warns about `GROUP_CONCAT(DISTINCT a LIMIT 5)` returning exactly the five rows that were asked for. `dump_leaf_key()` now records that it was the one that stopped the walk, so `val_str()` warns only when the walk itself failed. The warning is a new one. `ER_CUT_VALUE_GROUP_CONCAT` reports the row the result was cut at, and there is no such row here: the walk failed before it delivered anything, and how much was lost is not known, so it would read `Row 0 was cut by group_concat()`. `ER_RESULT_CUT_BY_LIMIT` says that the result was cut and names the limit that cut it, which is the memory `Unique` was given to work in: the smaller of `tmp_memory_table_size` and `max_heap_table_size`. Not every failure is silent either. The merge buffer is allocated with `MY_WME` and the spill file is opened with `MY_WME`, so running out of memory or failing to read raises an error of its own. Only the guard at the top of `merge_walk()`, which refuses a merge buffer too small to hold one key per chunk, returns without saying anything. Warn only when no error was raised: where one was, the user has been told and the statement is failing, so describing the length of a result nobody will see adds nothing. The debug keyword `unique_walk_merge_fail` fails the merging walk quietly and `unique_walk_merge_error` fails it with an error raised. `main.gconcat_distinct_walk_fail` uses both. The `LIMIT` case needs no debug build and is checked in `main.gconcat_distinct_spill`. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-28610 : Assertion marked_for_read() failed upon range select with virtual column in index. Problem: Executing a range select using an index that includes virtual/generated columns (e.g. SELECT ... WHERE c BETWEEN ... ORDER BY d) could trigger: Assertion 'marked_for_read()' failed in Field access during execution. Cause: The keyread initialization path was populating table->read_set using mark_index_columns(), which follows the non-recursive index marking path. This only marks direct index fields and does not resolve dependencies of virtual/generated columns. As a result, when an index contained virtual columns, their dependent base columns were not included in read_set, leading to invalid access during later evaluation of virtual columns. Fix: Replace mark_index_columns() with mark_index_columns_for_read() in the keyread initialization path. This ensures the recursive read-marking logic is used, so virtual/generated column dependencies are correctly propagated into read_set before execution. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39245: Added ASCII validation at two points using my_string_repertoire(): 1. Built-in plugins (plugin_init): A non-ASCII builtin name is a developer error. Added a check using my_string_repertoire() that prints an error via print_init_failed_error() and returns 1 to abort server startup. 2. Dynamically loaded plugins (plugin_add): Added a pre-validation loop before the registration loop. If any plugin name in the .so is non-ASCII, the entire .so is rejected before any plugin is registered, preventing partial state where valid plugins from the same .so get left INACTIVE in the hash. Added test plugins and MTR tests to verify both rejection of non-ASCII named plugins and successful loading of ASCII named plugins. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Monty
monty@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Trivial optimziations for group_concat - Remove some if - Reorder code - More code comments (cherry picked from commit dc6a897961c311f981b150e4207ffc1390a219ef) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Monty
monty@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Limit the memory used by GROUP_CONCAT() with ORDER BY GROUP_CONCAT() with ORDER BY collects all rows of the group in a TREE and only cuts it down in repack_tree(). The repack was triggered by (tree_len >> GCONCAT_REPACK_FACTOR) > thd->gconcat_max_len() with GCONCAT_REPACK_FACTOR 10, that is when the rows in the tree had produced 1024 * group_concat_max_len bytes, or 1G with the default settings. On top of that tree_len only counted the length of the strings, while the tree costs sizeof(TREE_ELEMENT) + reclength per row. For GROUP_CONCAT(int_col ORDER BY int_col) that is about 40 bytes per row against 6 bytes of result, so the tree had grown to several GB before the first repack. In practice the server ran out of memory first and the repack code was close to never used. The tree is now limited by the memory it has really allocated, tree->allocated, instead of by the length of the strings it holds. The limit is MY_MAX(thd->ram_limitation(), thd->gconcat_max_len()) and is never set so low that the tree can not hold a few rows. repack_tree() builds a new tree while the old one is still in memory, so the peak usage is the size we start the repack at plus the size we copy to. To keep the sum within the limit it is split into GCONCAT_TREE_PARTS parts; the repack starts when GCONCAT_TREE_REPACK_PARTS of them are used and copies to the remaining part. The part we do not copy to is also the room the tree has to grow before the next repack, which keeps the repacks amortized. Other changes: - tree_len is removed. It was only read by the old trigger. - repack_tree() decided that it had run out of memory by testing st.len <= st.maxlen after the walk. That test was only valid because the old trigger guaranteed that a complete copy had to overshoot st.maxlen. A repack triggered by memory can complete the walk with st.len far below st.maxlen, which would have failed the query with a wrong out of memory error. There is now an explicit flag for it. - The length that decides which rows to keep now also counts the separator that is put between two rows, so that it matches what val_str() will produce. - When the memory limit stops the copy, the result becomes shorter than group_concat_max_len. dump_leaf_key() can not detect this, as the result never reaches the maximum length. This is now remembered in result_cut and reported to the user. - All cut value reporting is moved to val_str(); dump_leaf_key() only marks that the result was cut. This removes the need to clear the truncated flag of table->blob_storage to avoid a duplicated warning, and gives one warning per group also when val_str() is called more than once for the same group, which repeated the warning before. - Added a function comment for repack_tree() that describes where the rows are cut away and why building a copy frees memory. Co-author: Arcadiy Ivanov <[email protected]> - Fixed a bug in copy_tree() |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Mohammad Tafzeel Shams
tafzeel.shams@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40404 : Assertion '!lock.is_locked_or_waiting()' failed in buf_page_t::init() A locking read on a SPATIAL INDEX collects the matching records into a shadow block matched_rec_t::block. sel_set_rtr_rec_lock() holds page.lock on that block while it locks the records, because lock_rec_lock() asserts that the block of the record being locked is latched. If a lock request has to wait, the records are collected again after the wait, and rtr_init_match() re-initializes the block by buf_page_t::init(), which asserts that the block is not latched. The assertion is about a latch that would be a leftover from the previous identity of the block. It does not apply to rtr_init_match(), where the block is in buf_page_t::MEMORY state, private to the current thread, which is holding page.lock on it on purpose. Changes: - buf_page_t::init(): Tolerate a latch that the current thread is holding on a block that is being initialized in buf_page_t::MEMORY state. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Fix the `HA_NO_KEY_READ` blob key guard `HA_NO_KEY_READ` marks a key whose blob segment `heap_prepare_hp_create_info()` converted from the VARTEXT2 form, so that `heap_rkey()` refuses an index read on it. It never worked, because the mark was written to the wrong structure member. `heap_rkey()` tests `HP_KEYDEF::flag`, which is where the flag belongs: `HA_NO_KEY_READ` is declared among the key flags, not the key-seg flags. The assignment instead targeted `HA_KEYSEG::flag`, which no reader consults for this flag, so the guard could never fire. That member is also `uint16`, so bit 20 was discarded on assignment as well; `-Wall -Wextra` does not warn, only `-Wconversion` does, and it is not enabled. Write the flag to `keydef[key].flag` instead. `HP_KEYDEF::flag` is `uint` and holds bit 20, and `heap_create()` copies it into the share that `heap_rkey()` reads. `hp_test_key_setup-t` covers the marking, the unmarked case, that `heap_create()` does not lose the flag while folding its own bits into `keydef->flag`, and that `heap_rkey()` refuses a marked key while accepting an unmarked one. The last pair clears `my_assert` so the guard reports instead of aborting, the same way the server's `--debug-assert=0` does, and skips on builds without `DBUG_ASSERT`. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Added ASCII validation at two points using my_string_repertoire(): 1. Built-in plugins (plugin_init): A non-ASCII builtin name is a developer error. Added a check using my_string_repertoire() that prints an error via print_init_failed_error() and returns 1 to abort server startup. 2. Dynamically loaded plugins (plugin_add): Added a pre-validation loop before the registration loop. If any plugin name in the .so is non-ASCII, the entire .so is rejected before any plugin is registered, preventing partial state where valid plugins from the same .so get left INACTIVE in the hash. Added test plugins and MTR tests to verify both rejection of non-ASCII named plugins and successful loading of ASCII named plugins. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
ParadoxV5
paradox.ver5@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
To maintain backward compatibility until MDEV-40941, the status variable `Rpl_semi_sync_slave_status` is kept showing the applied `@@rpl_semi_sync_slave_enabled` for the most recent START SLAVE in any connection; that is, not yet made per-connection. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41228 Add Item::same_select_copy() for the casefold rewrite Item_func_in::varchar_upper_cmp_transformer() rewrites UPPER(key_col) IN (const, ...) into key_col IN (const, ...). It needs a copy of the IN predicate to put the new first argument into, it used deep_copy_with_checks(). A deep copy copies every node of the tree and refuses as a whole if any node cannot be copied, so an IN list holding an uncorrelated scalar subquery, which is constant but has no copy, silently lost the rewrite: UPPER(a) IN ((SELECT MAX(c) FROM t2),'A-2') read the whole table instead of using a range on the index on a. After copying, the rewrite walked the copy with cleanup_excluding_const_fields_processor and re-fixed it, which would also have cleaned up a subquery had one been copied. A copy for use in the same SELECT_LEX does not need to copy what can be shared there. We add a separate copy hierarchy for that purpose. Item::same_select_copy_with_checks() calls the protected virtual Item::same_select_copy(), and cleans up every node it actually copied, so that the caller only has to call fix_fields() on the result. fix_fields() re-fixes the copied nodes and skips the shared ones, which are still fixed. It does not use deep_copy() at any level; the only other copy function it calls is surface_copy() on the node being copied. Item default is to share the item. Item_func, Item_cond and Item_row copy their node and recurse into their arguments. Item_func copies an argument that occurs more than once in args[] only once. Item_bool_rowready_func2, Item_func_case_simple and Item_func_in detach the comparators and value array they would otherwise share with the original, as their deep_copy() overrides do. Fields, references, constants, parameters, caches, subqueries, aggregate and window functions are shared because they don't derive from Item_func. Some Item_func subclasses cannot safely be copied, so they override same_select_copy() to share the original instead: - Functions that own memory or handles, which the copy would free a second time in cleanup() or its destructor: the regex functions, UDFs, stored functions, the JSON functions that build hashes, VEC_FromText, and the geometry functions with Gcalc state. - Functions that their SELECT_LEX keeps a pointer to, so a copy would not be the item the SELECT_LEX uses: MATCH, ROWNUM and Item_equal. - Functions whose copy would behave differently: RAND would restart its random sequence, and SET @var would assign the variable twice. - TO_NUMBER, which refuses to be copied at all. - Functions whose arguments point at each other: NULLIF uses args[0] a second time as args[2], and Item_in_optimizer's cache belongs to its subquery. The rewrite now uses same_select_copy_with_checks() and no longer walks the copy with a cleanup processor. TABLE::add_splitting_info_for_key_field() uses it too, for the left side of the equality it pushes into a split-materialized derived table: the item of the derived table's select list, used in the WHERE condition of the same select. That item is always a column, so the copy is the column itself, where the deep copy made a second Item_field for it. The equality is fixed when it is injected. The split_materialized_clones check of the left side is removed, as it is no longer meant to be a deep copy. This commit was prepared with Claude Code (Opus 5), which found the subquery case that lost the rewrite and confirmed that the new test fails without the change, designed and wrote the same_select_copy() hierarchy and the list of shared Item_func classes, switched the casefold rewrite to it, and ran main and the SQL-layer suites in normal and --ps-protocol mode. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thirunarayanan Balathandayuthapani
thiru@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-32067 InnoDB linear read-ahead had better be logical The traditional linear read-ahead, enabled by innodb_read_ahead_threshold, only helps when consecutively accessed pages have adjacent page numbers. That is rarely true for B-tree leaf pages: after splits, merges and page reuse, the logical order of leaves has nothing to do with their physical page numbers. So for a scan of scattered leaves the old read-ahead either did nothing or read the wrong pages. This replaces linear read-ahead of index pages with a logical read-ahead that prefetches the actual leaf pages a scan is going to visit, discovered from the B-tree during traversal. After this change, the exact nonzero value of innodb_read_ahead_threshold matters only for the read-ahead of undo log pages. buf_read_ahead_undo(): Renamed from buf_read_ahead_linear(). Now invoked only for undo log pages, whose page numbers are sequential. It is no longer called on BLOB pages or index pages. buf_read_ahead_one(): Read ahead a single leaf page. buf_read_ahead_pages(): Read ahead a set of known B-tree leaf pages. Stops at the first page that is already resident or fails. buf_read_ahead_random(): Split into a decision part(which extent to read) and action part (issue the reads) The pending read throttle now compares the os_aio_pending_reads_approx() with buf_pool.curr_size() instead of current pool bytes, which had made the throttle a no-op. buf_read_ahead_pages() releases the tablespace reference on a failed/corrupted page. btr_ra_latch: A caller-owned buffer of leaf page numbers, which also records the level-1 page and the child page number of the last node pointer collected, so read-ahead can be resumed later. btr_ra_collect(): Scan the node pointers of PAGE_LEVEL=1 page from the given record in scan order, appending child page numbers btr_cur_t::search_leaf(), btr_cur_t::open_leaf(): At PAGE_LEVEL=1, after the descent has located the child to follow, harvest leaf page numbers starting at that child in the scan direction. These are exactly the leaves the cursor will visit, so prefetch is precise regardless of physical page numbers. btr_ra_resume_rec(): Locate the record from which read-ahead should resume, by re-finding the last harvested child by value. It walks only genuine records, so a concurrent page reorganization cannot cause an invalid read btr_pcur_move_to_next_page(): Prefetch the sibling after the one being entered, with buf_read_ahead_one(). btr_pcur_move_backward_from_page(): Add a no-wait fast path that latches the previous page if it is resident (validating it against the current page), and prefetch the one before it. btr_copy_blob_prefix(): Simplified; no longer relies on FIL_PAGE_PREV/NEXT, which are not initialized on BLOB pages. records_in_range() reports the first and last leaf page of a range, and the optimizer forwards that extent to the engine through handler::advise_page_range(). multi_range_read_info_const() takes a page_range parameter; the default MRR implementation aggregates the per-range extents, and opt_range.cc carries the result through the chosen QUICK_RANGE_SELECT, whose reset() calls advise_page_range() before multi_range_read_init(). The extent only sizes and bounds read-ahead; it never gates the pages the scan actually reads. The LIMIT is converted in multi_range_read_info_const() to a leaf count (m_ra_limit_pages). mrr_readahead_from_scan_range() derives the read-ahead ceiling from the advised extent, falling back to that LIMIT-based estimate. The ceiling is capped by init_readahead_window() at 1/256 of the buffer pool (but at least 4 pages), because every prefetched page occupies a buffer pool frame as soon as its read is issued. The window ramps up: a positioning read harvests at most 4 leaves and resets the window to 4, and readahead_refill() doubles it toward the ceiling. Single-row probes such as index_first() for MIN()/MAX(), and every range of a multi-range read, therefore prefetch only a few leaves while long scans reach full depth. search_with_readahead(): Position the cursor while collecting the leaf pages, then call start_readahead(). It is used by index_read() and by the first general_fetch() only when read-ahead is armed, so the other paths carry no read-ahead locals. start_readahead(): For a just-positioned scan, prefetch the collected batch and set up the rolling cursor. readahead_refill(): As the scan advances (general_fetch()), resume the level-1 harvest under an index S-latch, chain across level-1 siblings, and prefetch the next batch bounded by the advised last leaf, or to the end of the index when the extent is unknown. The reads are issued after the latches are released. Read-ahead is started from index_read() (range and full index scans), rnd_init()/general_fetch() (full table scans), and, at one-page depth, from row_merge_read_clustered_index() (OPTIMIZE/ALTER rebuild). general_fetch() sets active_handler_stats before issuing read-ahead so prefetched pages are attributed to the query. dict_stats_analyze_index_for_n_prefix(): Choose the records to dive below first, prefetch their child pages in one batch with buf_read_ahead_pages(), and then dive below the same records. Undo/system tablespace shrinking holds fil_space_t::latch exclusively (x_lock_space()), and buf_read_ahead_undo() holds it shared, via the new fil_space_t::s_lock_try() that never waits, while it checks the size and issues the reads. If the latch is not available, the read-ahead is skipped. trx_undo_get_prev_rec(), trx_undo_get_prev_rec_from_prev_page(): take the trx_undo_t object instead of a long parameter list and always latch shared. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
ParadoxV5
paradox.ver5@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
WIP tests for MDEV-39838 to repro the failure Co-authored-by: Roel’s AI. The final test should be nearly all my writing, with those AI parts barely distinguishable. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41228 Rename shallow_copy to surface_copy and check its type Item::shallow_copy() was meant to create a copy of the same class as the item it is called on, but nothing enforced that. A class that does not declare its own override inherits the one of its nearest concrete parent, and get_item_copy<Parent>() then silently creates an object of the parent class, losing the child's virtual behaviour (see MDEV-38801 for an earlier instance with Item_cache_year). Rename shallow_copy()/shallow_copy_with_checks() to surface_copy()/ surface_copy_with_checks(). "Shallow" said little about what such a copy is for; the rename prepares for copy methods named after their purpose. Add the ITEM_SURFACE_COPY(CLASS) macro, which declares the override, and Item::dbug_check_surface_copy_type(), a debug-only check called from Query_arena::free_items() for every item freed. Once per dynamic type it creates a surface copy, unlinks it from thd->free_list again (the copy shares pointers with the original and must not be destroyed) and asserts that the copy has the same typeid as the original. The check found these classes copied as their parent class, now fixed with ITEM_SURFACE_COPY(): Item_join_operator_plus, Item_null_result, Item_old_field, Item_datetime, Item_empty_string, Item_return_int, Item_trigger_type_of_statement, in_string::Item_string_for_in_vector, Item_is_not_null_test, Item_interval_DDhhmmssff_typecast, Item_string_xml_non_const, Item_func_trt_trx_sees_eq, Item_field_refcursor (type_cursor plugin), Item_field_packable_scalar (type_assoc_array plugin). Item_field_packable_row::surface_copy() asserted to mark the class as never copied, which the check trips over since it copies every type it meets. It now returns nullptr, the existing way for an item to refuse a copy. No test is added: the check runs under every existing test in a debug build. main and the SQL-layer suites (compat/*, json, funcs_*, vcol, gcol, versioning, period, sys_vars, type_*) pass with it in both normal and --ps-protocol mode. This commit was prepared with Claude Code (Opus 5), which wrote the runtime type check, ran the main and SQL-layer suites in normal and --ps-protocol mode to find the classes that inherited a parent's copy method, added the ITEM_SURFACE_COPY overrides for them, changed Item_field_packable_row to return nullptr, and performed the mechanical rename. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MDEV-39105: Shutdown hang after failed InnoDB startup | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||