Console View
|
Categories: connectors experimental galera main |
|
| connectors | experimental | galera | main | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PQ: tidy up some of the mess, remove pointless code. reworked the code in sql_select.cc to make it clearer and shift more out. Huge tidy up. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PQ: the workers pre-aggregate a query that has no GROUP BY SELECT COUNT(*), SUM(b) FROM t ran serially. The workers could pre-aggregate a GROUP BY, but aggregation over the whole table was refused, because the shape the grouped path merges through is not there: with no group the server materialises nothing, and aggregates in end_send_group() straight off the last join tab -- no aggregation table, no group key, nothing for a partial to be merged into. It turns out neither end needs new machinery, and this commit is mostly the finding of that. The manager already merges these. end_send_group() reaches Item_sum::add(), and add() honours a value left by direct_add() for every aggregate the gate accepts -- COUNT adds a count instead of incrementing, AVG takes a partial sum together with the number of rows behind it. So the drain primes the aggregates and hands the row to the plan's own terminal here exactly as it does through end_update() for a GROUP BY, and the drain needed no case of its own. The workers accumulate into the grouped machinery with a single group in it. The layout gives this shape a group of one constant column, and from there it is the grouped path: a worker keeps one partial in a keyed container, merges into it through end_update(), and ships one row. Beyond costing a column per shipped row -- of which there is one per worker -- that is also what keeps Item_sum_avg::create_tmp_field() packing the count beside the sum, which it does only for a container that has a group. An average built any other way would have arrived without its count. So what is actually new is the gate. The aggregate tests -- the five that can be merged, no DISTINCT, one argument, an argument a worker can evaluate -- are the same for both shapes and move out to pwt_aggregates_can_be_merged(). What is left is the shape question, and which shape to ask about is the query's own grouping rather than the plan's: a query that names a GROUP BY stays the grouped question whatever the optimizer then did with it, so a plan with no group key to merge through is a refusal of that query and not an invitation to read it as aggregating over everything, which would answer something else. Only a query that never grouped reaches the second question. Refused there: a select-list item reading a column outside an aggregate. Its value is whichever row the terminal happened to hold, and a worker holds a different one. An empty result needed nothing. No rows means no groups means no partials shipped, and end_send_group() still sends its one row at end of records: COUNT(*) is 0, SUM is NULL, one row and not one per worker. The new test checks each of those rather than assuming them. main.parallel_query_aggregate_whole is that test: serial against parallel for COUNT, COUNT(col), SUM, MIN, MAX and AVG over integer, decimal and double columns; the empty case; that exactly one row is sent; and that a DISTINCT aggregate, STD, GROUP_CONCAT and a bare column outside an aggregate are still refused and run serially. main.parallel_query_aggregate gains the fall-back case, where the container turns out to have no group index: the constant group reaches the same fall-back, ships plain rows, and the manager aggregates them a row at a time. Known and accepted: adding doubles is not associative, so SUM and AVG over DOUBLE or FLOAT can differ from the serial answer, and between runs, in the last significant bit -- which worker summed which chunk is scheduling. Decimal and integer aggregates are exact and unaffected. The test rounds well clear of the last bit rather than leaving floats uncovered. Under a forced-worker sweep of the main suite (--parallel-worker-threads=4) three more tests differ than before, all of them EXPLAIN text where a query of this shape now says ALL_parallel or range_parallel: main.costs, main.func_group_innodb and main.innodb_full_scan. No value changes in any of them. This commit was prepared with Claude Code (Opus 5), which established that Item_sum::add() already honours a directly added partial -- the fact the whole change rests on -- and found that a container without a group would have dropped an average's count. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-42009: Set GLOBAL innodb_log_file_size=ON is not crash safe log_t::header_rewrite(): Before setting log_sys.archive=true, ensure that any garbage entries at the end of the file will be zeroed out so that they cannot be mistaken for newer records. On Linux and Microsoft Windows we will attempt a shortcut for writing zeroes, before falling back to log_file_t::write(). Reviewed by: Alessandro Vetere |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41251 Wrong result with split materialized query 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. A constant view column keeps its constant underlying item, which 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). If the copy still depends on a table of item_select, which happens for a constant column of a view on the NULL-complemented side of an outer join, since used_tables() then names that table, the copy gets its depended_from set as well. - 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, Item_insert_value and Item_trigger_field are treated this way too, as they are not plain columns. - An item that depends on no table of item_select is shared. This includes items whose deep_copy() returns nullptr, such as stored routine variables, which therefore no longer prevent splitting. 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. A second test, taken from Dave Gosselin's MDEV-41251 commit, pushes a comparison, dt.a <=> (t1.b = 1), and its row form. The deep copy of the comparison was never fixed again. Its Arg_comparator still pointed at the original, which sets null_value of the original, and a copied row comparison had comparators set to NULL. The row of t1 with a NULL b was missing from the result, and the row form crashed in Arg_comparator::compare_row(). Now the comparison is not copied: the right side refers to the original. A third test pushes dt.a <=> v.one where v is a merged view with the constant column one, on the NULL-complemented side of an outer join. It covers the view column copy that depends on a table only through its NULL-complemented table. 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, added the comparison test, confirmed in a build of the parent commit that both tests fail without the change, and ran main and the SQL-layer suites in normal and --ps-protocol mode. Claude Code (Fable 5.1) then reviewed the commit, found and reproduced the NULL-complemented view column case, wrote its fix and test, and reran the derived and SQL-layer tests. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleg Smirnov
olernov@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| InnoDB: extract pscan_chunk_clamp_t as a separate struct from row_prebuilt_t | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleg Smirnov
olernov@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Rename Parallel_coordinator, some other refactorings and clean-up | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Petrunia
sergey@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Coding style fixes, remove out-of-date comments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 PQ: leave a condition the workers cannot evaluate to the manager A conjunct of a table's condition that no worker can evaluate kept the whole query serial. The common one is a correlated subquery: its JOIN belongs to the manager's thread, so there is no clone of it to hand a worker, and TPC-H Q21's NOT EXISTS over LINEITEM was enough to refuse a plan whose driving table the optimizer had already costed as a parallel scan. The gate now divides each table's condition instead of refusing it (pwt_split_table_cond). The conjuncts a worker can evaluate stay with the table, as the worker's select_cond, and the rest are ANDed into JOIN::parallel_residual_cond for the manager to apply to every row it drains, after copy_back_row() has put the shipped columns back into the manager's own records, where the residual's items already look. A correlated subquery among them runs on the manager's thread through the manager's own handlers exactly as it would have run serially. A conjunct of an inner join may be applied anywhere after its tables have been read, so the answer is the same; what changes is the number of rows it is asked of, which is the finished join's rather than this table's. For the residual to be answerable from a drained row, every field it reads from a join table has to be a shipped column, which pwt_row_layout::build() takes from the read_set. The split checks that, walking into subqueries for their outer references, and refuses the plan if a conjunct can be evaluated on neither side. The split is taken by the gate and taken again by init_parallel_workers() from the plan as it stands when the workers are built (pwt_split_plan_conditions), because it names the plan's own items. A plan with a residual ships joined rows rather than pre-aggregating: the filter has to run before any grouping, so pwt_row_layout::build() takes a ship_rows argument that keeps 'grouped' off while 'plan_aggregates' stays on, which is the fall-back shape the drain already handles, with the plan's own terminal grouping what the manager lets through. The non-aggregating drain applies the residual before sending a row, and before collecting it when the sort is the manager's. The optimizer trace records the manager's share under parallel_scan_manager_evaluates. On TPC-H SF1 Q21 the serial plan evaluates the NOT EXISTS once per late LINEITEM row of a finished order, 1.8M times, and the manager once per row of the finished join, 77k times, since the NATION filter has run by then. With a FirstMatch semijoin admitted to the workers in the next commit, Q21 goes from 2.9 s serially to 0.85 s with four workers and 0.68 s with eight, with the same result. main.parallel_query_residual pins the four shapes: whole-table aggregation, GROUP BY, a plain select, and a select whose sort the manager performs, each with a correlated subquery in the WHERE clause, each run in the workers and each giving the serial answer. This commit was prepared with Claude Code (Fable 5.1), which diagnosed the refusal from the optimizer trace, measured that applying the subqueries after the join was cheaper even serially, designed and wrote the split, the layout change and the drain-side filter, and wrote the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PQ: re-split a chunk into a few pieces, not one per page A benchmark of "select grp, sum(i) from perf group by grp" over a million narrow rows showed the pages the engine touched jumping fivefold as the worker count crossed a threshold, while the time barely moved: one more worker turned 19 chunks into 10127. Exec_ctx::split() was the cause. It is reached when a worker runs dry and the chunk it picked up was flagged to be divided, and it divides by asking partition() for split_level 1 -- every child of the sub-tree's root. One level down from a chunk that spans a whole sub-tree is one range per page below it, so a chunk became hundreds or thousands of chunks where a handful would have done. Each of those then costs a descent from the root when a worker takes it, which is what the page counter was showing. partition() is left alone; only the number of boundaries used changes. The ranges it returns are contiguous intervals that share endpoints, so a run of them merges into one by taking the first's start and the last's end, and every m'th boundary is kept for a target of about one piece per worker. The same benchmark, before and after, at the worker counts around the step: workers chunks resplit pages chunks resplit pages 13 1403 2 20988 41 2 15540 14 282 1 16508 29 1 15496 15 15415 15 76984 240 15 16284 The re-split counts are unchanged, which is the point: the same chunks are flagged and the same decisions are taken, only each division is now the size it was meant to be. What was a fivefold rise in pages accessed is five per cent. Those counts are the other half of this commit, because none of it was visible before. ANALYZE FORMAT=JSON now reports, beside r_rows_per_worker on the table that was divided: "r_rows_per_worker": [...], "r_chunks": 41, "r_chunks_resplit": 2, r_rows_per_worker on its own cannot answer this. It is the cut and the order the pull queue handed chunks out, together, so an uneven row split says nothing about whether the index was cut badly or the chunks merely landed unevenly. r_chunks is the cut by itself: every chunk the engine produced, the ones a re-split made included, and how many were divided rather than scanned. The counts come from the coordinator through a new handler method, parallel_get_chunk_stats(), which defaults to reporting nothing so that an engine that does not divide scans adds no output. They are read once the workers have been joined, from the handler parallel_init_coordinator() ran on -- the manager's, not a worker's copy -- and only for the driving table, which is the only one divided. Both are masked by include/analyze-format.inc: a chunk is only re-split when a worker runs dry and asks, so how often that happens, and therefore how many chunks exist at the end, depends on which worker finished first. What this does not address: create_contexts() still flips from flagging the tail of the chunk list to flagging all of it the moment the chunk count stops exceeding the worker count, which is the step visible above between 14 workers and 15. It is a discontinuity where a slope belongs, and it is now cheap rather than expensive, but it is still there. This commit was prepared with Claude Code: the re-split was identified from the chunk counts once they existed, and the before-and-after table above is its measurement of the same test with the coalescing removed and restored. (Slightly) tidied up by Rex. Then... Three fixes from review of the coalescing commit, none a wrong answer, each a number coming out other than intended. Round the merge stride down, not up. m was ceil(size/target), which makes ceil(size/m) groups, and for a size just over the target that collapses to half of it: 16 boundaries at target 15 merged into 8 pieces. Floor overshoots instead, bounded by twice the target -- and too many small pieces costs less than too few large ones, since the whole reason a chunk is being split is that a worker had nothing to do. Aim for twice the worker count, not once. The pieces a re-split produces are never re-split again -- create_context() flags none of them -- so if the rows inside the chunk are themselves skewed, having more pieces than workers is the only slack left for evening that out. Twice keeps the piece count, and with it the per-piece descent from the root, small, while leaving a second helping for whoever finishes early. The benchmark from the coalescing commit, re-measured across the same worker counts: workers chunks resplit pages 13 69 2 15652 14 45 1 15560 15 484 15 17260 16 511 15 17368 The 15-worker case reconciles exactly -- about 1027 leaf ranges per flagged chunk, target 30, stride 34, so ~31 pieces each; 15 x 31 plus the 15 initial chunks is the 484 -- and the headroom costs six per cent more page accesses than the single-worker-count target did, against the fivefold the shattering cost before coalescing existed. The step between 14 and 15 workers, where create_contexts() goes from flagging the tail of the chunk list to flagging all of it, is now a slope a reader can see the shape of rather than a cliff. Make every statistic the parallel scan reports per-execution. quiesce_workers() accumulated r_rows and its neighbours across executions of the same plan -- a correlated subquery, a routine loop -- the way the serial path does, while r_rows_per_worker and the chunk counts were overwritten each time, so ANALYZE for a re-executed plan mixed totals with last-execution figures in one block. The split and the chunk counts only describe one execution, and totals beside them answer a different question about a different number of rows, so the tracker now starts from zero each time the workers are reaped: everything ANALYZE prints for a parallel scan describes the same, latest, run. This is a deliberate departure from the serial convention, and r_scans is reset with the rest so the figures stay consistent with each other. This commit was prepared with Claude Code, from findings of its own review of the coalescing commit: the half-target collapse and the mixed accumulate-and- overwrite semantics were found by re-deriving what the arithmetic does at the boundaries rather than re-running it. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40012 PQ: end a worker's parallel scan before close() frees m_prebuilt A worker's handler was destroyed with its parallel scan still open. exec() and scan_only() jump to their exit labels when parallel_init_worker() fails, and the commonest failure is HA_ERR_END_OF_FILE: no chunk was left for this worker, which happens all the time on small tables. InnoDB has allocated m_pscan_worker by then, so the scan was left for ~ha_innobase() -> parallel_scan_free(), which runs after close() has freed m_prebuilt. parallel_scan_free() says it drops the worker without end() for exactly that reason, but ~Parallel_scan_worker() called end() anyway, and end() writes m_prebuilt->pscan_chunk_clamp: a heap-use-after-free that ASAN reports on every such worker. In a build without ASAN the write lands in freed memory, and it is the likely cause of the intermittent SIGSEGV in free() inside pthread_create() when a later worker reused a cached thread stack whose DTV had been overwritten. Three changes close it. Both workers now end the scan at their exit label instead of after the loop, so every way out ends it while the handler is still open; ending a scan that was never begun is harmless. ha_innobase::close() ends any worker or coordinator scan still open before row_prebuilt_free(), so a path that misses it is ended properly rather than left for the destructor; with the SQL layer put back as it was, this alone keeps ASAN clean. And ~Parallel_scan_worker() no longer calls end(), so parallel_scan_free(), now reached only for a handler destroyed without close(), cannot write through m_prebuilt either. The close() hook and the single call site are taken from a patch by a co-worker. Two debug assertions state the rule the defect broke, so it no longer takes ASAN to see it. ha_innobase::close() asserts that the SQL layer has already ended a worker's scan, and parallel_scan_free() asserts that no worker survives close(). The injection pwt_leave_scan_for_close makes the SQL layer leave the scan of a worker that holds no cursor -- one handed no chunk -- for close() to end, so the backstop is exercised on purpose and the first assertion is told to allow it. pwt_end_worker_scan() holds the injection and ends the scan at both exit labels. parallel_query_no_chunk (debug only) runs a full scan and a range scan of a one-page table with four workers, so three of them get no chunk, twice each, through the normal and the scan-only path, with and without the injection, checking the rows and that the workers ran every query. In a plain debug build, removing the exit-label calls fails the first assertion and removing the close() backstop fails the second. ~Parallel_scan_worker() no longer calling end() is defence for release builds and is not reachable while the backstop is there. This commit was prepared with Claude Code (Opus 5.5), which reproduced the pthread_create() crash under ASAN and traced it, wrote the fix, folded in the co-worker's close() hook and single call site, added the debug assertions, the injection and the test, confirmed in a plain debug build that the test fails with each of the two fixes removed, and ran the parallel_query tests in normal and --ps-protocol modes and the main suite with --parallel-worker-threads=4 under ASAN. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 PQ: run a FirstMatch semijoin in the workers The gate refused every semijoin duplicate-elimination strategy, on the grounds that a worker's nested loop implements none of them. For FirstMatch that was not true. FirstMatch is the nested loop backing out early: when the last inner table of the semijoin nest matches, the executor sets join->return_tab to the outer table the nest follows (JOIN_TAB::do_firstmatch) and sub_select() unwinds to it rather than looking for further matches that would only duplicate the row. A worker runs sub_select() over its own JOIN and its own JOIN_TABs, so it does the same -- provided its copy of do_firstmatch names a tab of its own array, since return_tab is only ever compared with the tabs of the array being walked. setup_worker_jointabs() now remaps do_firstmatch, first_sj_inner_tab and last_sj_inner_tab from the manager's array onto the worker's, and the gate admits SJ_OPT_FIRST_MATCH, refusing a nest whose return tab is a const table, which no worker holds. LooseScan and DuplicateWeedout keep state that no copy carries -- a match flag, a weed-out table -- and stay refused, as does materialization. This is the second half of what TPC-H Q21 needed: its EXISTS over LINEITEM is flattened to a FirstMatch nest after NATION, and with its NOT EXISTS left to the manager by the previous commit, the plan the optimizer had costed as a parallel scan of ORDERS now runs in the workers. main.parallel_query_firstmatch runs an EXISTS flattened to FirstMatch under whole-table aggregation, GROUP BY and a manager-side sort, with two matching inner rows per outer row so that a nested loop without the early return would double every count, and shows DuplicateWeedout still refused. This commit was prepared with Claude Code (Fable 5.1), which read what sub_select() and evaluate_join_record() consult for FirstMatch, wrote the remap and the gate change, and wrote the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleg Smirnov
olernov@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Extract parallel logic from ha_innobase to separate files | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40012 PQ: a worker must not end its container's scan after handing it over A pre-aggregating worker accumulates into its own container, a temporary table, and hands that container to the manager with sink->flush(). Just before the hand-off, execute_and_handoff() closes the container's index, because the manager starts its own scan of the container immediately after the hand-off. The tidy-up at exec_exit then closed the container a second time. Its comment said this was only for the paths that reach exec_exit without a hand-off, but the code ran on every path, including the path with a successful hand-off. By that point the container may already belong to the manager: claim_next_result() has seen it marked as done, and pwt_tmp_table_source::next_row() has rebound it to the manager's thread and called ha_rnd_init() on it. The worker's ha_index_or_rnd_end() therefore ends the manager's scan. The manager's next ha_rnd_next() then fails "inited == RND" in a debug build, and in a release build it reads from a table with a closed scan. The window between the flush and the second close is only a few instructions wide, so the failure is rare: main.ps_3innodb hit it once in a main-suite run with --parallel-worker-threads=4, on SELECT ... UNION DISTINCT SELECT sum(a) ... GROUP BY b. The second close now runs only on the paths that skipped the first close, as recorded by a flag set next to the first one. parallel_query_handoff makes the race certain. With pwt_handoff_sync set, a worker that has handed over its container waits until the manager reaches the new sync point pwt_manager_scanning_container, and the manager keeps its scan open until the worker has finished its tidy-up. The test uses one worker, so there is a single container and no risk of crossed signals. It checks the rows and that the query ran in the worker. Without the fix it fails the inited == RND assertion on every run. This commit was prepared with Claude Code (Opus 5.5), which found the crash in a main-suite sweep with workers enabled, traced it from the core dump to the second close, wrote the fix, the sync points and the test, and confirmed that the test fails without the fix in five runs out of five and passes with it in normal and --ps-protocol modes. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40012 PQ: refuse a plan that reads a table through a join buffer A worker has no join buffer. setup_worker_jointabs() stripped the cache off every worker JOIN_TAB and joined a row at a time, and pwt_table_conds() handed the worker the buffer's scan filter (cache_select->cond) as well as select_cond so that it still rejected what the buffer would have. The result was correct, but it was not the plan the optimizer costed: BNL, BNLH, BKA and BKAH were chosen for scanning the inner table once per buffer-full of outer rows, and a worker scanned it once per driving row instead. For a full-scan inner table that can be orders of magnitude more work than the serial plan the parallel one is meant to beat. So the gate now refuses the plan, as the prototype did. The check sits in can_run_query_in_workers() beside the other per-table refusals, on tab->cache or tab->use_join_cache, both of which make_join_readinfo() has settled by the time parallel_join_check() runs in optimize_stage2(). The refusal names the table in the optimizer trace. The worker-side handling of a buffered table goes with it: - pwt_table_conds() becomes pwt_table_cond() and no longer reports cache_select->cond. remove_redundant_bnl_scan_conds() only moves conjuncts out of select_cond when the tab has a BNL/BNLH cache, so for every tab the gate now admits select_cond is already the whole condition. A tab can still carry a cache_select without a buffer, because make_join_select() builds the scan filter before check_join_cache_usage() decides, but its cond is then a copy of conjuncts select_cond still holds. The gate no longer asks whether that copy is worker-safe, since no worker evaluates it. - pwt_clone_table_conds(), which cloned both halves and ANDed them, is gone; setup_worker_jointabs() clones the one condition with pwt_clone_rebind(). - setup_worker_jointabs() no longer clears cache, use_join_cache and jbuf_tracker on the worker's copy; pwt_assert_tab_inert() asserts that the manager's tab has no cache instead. The copy's cache_select is still cleared, as it can be set on a tab without a buffer. parallel_query_join: the BNL case now expects the query to run serially, with the same rows. The full-scan inner table case (d3) was getting a BNL buffer under the default join_cache_level, so it runs with join_cache_level=0 to keep covering a worker-side full scan of an inner table. parallel_query_why pins the new refusal reason. This commit was prepared with Claude Code (Opus 5.5), which wrote the gate check, removed the worker-side join buffer handling it made unreachable, updated the tests, reconfigured and rebuilt the worktree, and ran the parallel_query tests in normal and --ps-protocol modes. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PQ: report per worker what the scan cost it, not just how much it read ANALYZE FORMAT=JSON says how many rows each worker read, which answers how the chunks divided the work but not why a worker finished when it did. Two figures the engine already keeps answer that, once they are kept per worker rather than summed: how long each worker was busy inside the engine, and how much of that was waiting for pages to come off disk. "r_rows_per_worker": [181967, 93080, 159138, 165815], "r_engine_time_per_worker_ms": [69.577, 74.895, 72.507, 69.902], "r_pages_read_time_per_worker_ms": [0.276, 0.229, 0.285, 0.283], "r_chunks": 18, "r_chunks_resplit": 1, The pair is what makes a straggler readable. Rows without time is a worker that was handed more to do; time without rows is one that was blocked. Above, four workers within seven per cent of each other on time despite a two-to-one spread in rows says the spread is the pull queue's doing, not the disk's. The read times sum to the pages_read_time_ms already reported in r_engine_stats, so the array is a decomposition of that total rather than a second measurement of it. Reading is the wait worth attributing this way: a parallel scan takes no row locks, because the gate declines the scan when the read is a locking one, so there is no lock wait to divide. The figures are undercounts wherever the pre-fetcher did its job -- a page somebody else fetched costs its reader nothing here -- so a low read time is not proof that nobody waited. Two things had to be fixed before the numbers existed rather than merely being hidden. The page read time was being collected per worker all along and thrown away at the merge: each worker resets its handler stats when it opens its tables and snapshots them into tab_hstats before closing them, and quiesce_workers() added those into the manager's handler without keeping the split. It now keeps it. Engine time was always zero for a worker, for two reasons. It is folded into the handler stats from the handler's Exec_time_tracker by close_thread_table(), which a worker's tables never go through, so snapshot_table_stats() now does that fold itself. Under that, the worker's handlers had no tracker at all to fold: set_time_tracker() is called by the optimizer on the plan's tables, and a worker's copies are opened after the plan exists, so nothing was timing them. Each worker now owns one Exec_time_tracker per table, installed as the table is opened. Both arrays are omitted when the engine recorded no time, so a scan that never touched the disk does not carry an array of zeroes; this is why the read-time array appears in some plans and not others. Timings and a per-worker split are both non-deterministic, so analyze-format.inc masks them, and the three .result files that show a parallel scan are re-recorded. This commit was prepared with Claude Code, which found that the read time was already being collected and discarded, and traced the always-zero engine time to the missing tracker on the worker's handlers. Added, r_peak_to_average_ratio for worker engine time. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-42009: Set GLOBAL innodb_log_file_size=ON is not crash safe log_t::header_rewrite(): Before setting log_sys.archive=true, ensure that any garbage entries at the end of the file will be zeroed out so that they cannot be mistaken for newer records. On Linux and Microsoft Windows we will attempt a shortcut for writing zeroes, before falling back to log_file_t::write(). Reviewed by: Alessandro Vetere |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-42009: Set GLOBAL innodb_log_file_size=ON is not crash safe log_t::header_rewrite(): Before setting log_sys.archive=true, ensure that any garbage entries at the end of the file will be zeroed out so that they cannot be mistaken for newer records. On Linux and Microsoft Windows we will attempt a shortcut for writing zeroes, before falling back to log_file_t::write(). Reviewed by: Alessandro Vetere |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleg Smirnov
olernov@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| InnoDB: fix unlatched right-move in the parallel scan partitioner | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Petrunia
sergey@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Remove Parallel_coordinator::Exec_ctx::m_id, it is not used anywhere. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PQ: a pre-aggregating worker hands over its grouping table A worker that pre-aggregates kept two temporary tables of the same shape: the grouping table its aggregates accumulated into, and the shipping container the manager reads. At end of records flush_groups() walked the first and wrote every row into the second -- a memcpy and a keyed insert per group -- so that the manager could read rows it already had, in a table it could already have been given. The tables hold the same rows because a worker produces one row per group and ships one row per group; there was never anything for the second table to do but hold a copy. So there is one table now. setup_worker_preagg() builds the shipping container itself, from the worker's own item list, and the aggregates accumulate straight into it; flush_groups() is gone, and so is the per-group copy and the second table's memory. What the manager reads is the table end_update() wrote. The batch transport is disabled, which is what makes that possible to say plainly. It carries record images through a buffer rather than handing a table over, so everything that goes through it is copied twice, and a worker with a finished table still has to stream it. pwt_create_transport() now builds the temporary-table transport unconditionally and the debug flag that selected the other one is gone. The classes are kept, compiled out, in case the streaming shape is wanted again. Two things the copy had been hiding had to be fixed before the table could be handed over. A rebuild on disk has to be the cross-thread kind. A container is instantiated cross-thread, because the manager reads and frees what the worker wrote, and the transport's own spill path passes that through. But a pre-aggregating worker's table fills while end_update() is writing it, and end_update() rebuilt it with cross_thread=false: Aria then ties the new handle's allocations to the worker's thread, and the manager frees them after that thread is gone. The decision now travels on TMP_TABLE_PARAM, set where the container is created and read where the rebuild happens. Without it the new test fails with the allocation crossing threads, named: 16416 bytes freed by T@3, allocated by T@71 at maria/ma_bitmap.c:242, ... sql/sql_select.cc:23926, sql/sql_select.cc:26598 And the memory that rebuild allocates has to change hands. The transport already moves the charge from the worker's THD to the manager's for a spill it performs itself, but a spill inside end_update() never passes through the transport. pwt_end_send() now brackets put_record() and hands anything the call left outstanding to the sink: working memory a call allocates and frees nets to nothing there, and the rebuild is what does not. main.parallel_query_spill covers the new path: twenty thousand groups against a 64K heap, asserting the query ran in the workers, that a row per group arrived, that the container really was rebuilt on disk, and that the answer matches the serial one group for group. main.parallel_query_transport loses the halves that exercised the batch transport, and with them its dependency on a debug build. Measured against this tree with the same forced-worker sweep of the main suite (--parallel-worker-threads=4): the same fifty tests fail before and after, the same fifty by name. This commit was prepared with Claude Code (Opus 5), which found that the two tables held the same rows, and identified both the non-cross-thread rebuild and the unaccounted memory before either could reach a test. The we found a bug, we need to close the container's index before handing it to the manager A pre-aggregating worker leaves its container's index open: AGGR_OP::put_record() opens it on its first call and nothing since has closed it. When the copy into a separate shipping container went away, closing it moved to the worker's tidy-up at the end of execute_and_handoff() -- which is after sink->flush(), and flush() is what tells the transport this producer is finished. The manager may claim the container the moment it is marked done. So a manager that got there before the worker reached its tidy-up called ha_rnd_init() on a table whose index was still open, and that asserts: mariadbd: sql/handler.h:3770: int handler::ha_rnd_init(bool): Assertion `inited==NONE || (inited==RND && scan)' failed. handler::ha_rnd_init pwt_tmp_table_source::next_row pwt_manager::drain_and_send Query: SELECT col1, COUNT(col2) FROM t1 GROUP BY col1 ORDER BY NULL Which thread wins is scheduling, so this showed up as main.parallel_query_aggregate failing about one run in eight with the whole parallel set running at --parallel=18, and passing every time on its own. Closing the index is moved to before the flush, where the hand-off is. The tidy-up still closes it, for the paths that reach the end without handing anything over; ha_index_or_rnd_end() on a handler that is already closed does nothing. pwt_tmp_table_sink::flush() now asserts what the ordering is for: that nothing this producer had open on the table is still open when the container becomes the consumer's to read. That turns a race the manager notices only when it wins into a condition the producer checks every time -- with the close put back after the flush, main.parallel_query_aggregate fails on the assertion on every run rather than on one in eight. This commit was prepared with Claude Code (Opus 5). The crash was found by running the parallel test set repeatedly under load rather than once; the producer-side assertion was added so the next mistake of this shape does not need luck to be seen. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 PQ: cost the parallel scan in the join-order search Until now the join order was chosen by the serial optimizer and the driving table it settled on was then handed to the workers whenever the gate admitted the query. On TPC-H that split REGION in Q5 and NATION in Q11: five- and twenty-five-row tables of which one row survives the WHERE, so the whole join under them ran on one worker and the query paid the setup of the workers for nothing. The plan that would actually divide -- ORDERS first in Q5, split four ways with the lookups below it spread evenly -- costs 1917 against the chosen plan's 873 when read serially, and the serial optimizer had no way to know that read by four workers it costs about 490. best_access_path() now prices the driving table's scan both ways and keeps the cheaper (parallel_scan_cost): serially, or as one worker's share of the scan plus optimizer_parallel_setup_cost per table in the join, with the rows surviving the table's own WHERE spread over min(N, rows_out) workers. The join-order search divides every later position's cost by that divisor (position_cost_in_plan), since each worker joins only its share of the driving rows and the plan's cost is one worker's. A large table can therefore win the driving position it loses serially, while a tiny driver gains nothing from the division and is charged the setup, so it stays serial. parallel_join_check() runs the workers only for a plan chosen that way. Three things are needed for the search to find such a plan honestly. The pruning heuristic drops a prefix that is worse on both rows and cost than the best at its depth, which is always true of a big-table prefix next to a five-row one; prefixes under a parallel driver are now compared only with each other. The tables under a parallel driver are costed as a worker runs them, a plain nested loop with no join buffer, hash join or rowid filter, and no covering-index or rowid-filtered scan is offered as a driver, because the gate refuses all of these and a plan chosen on their cost would run serially. And the refusals the gate can make without a plan -- limits, DISTINCT, HAVING, outer joins, window functions, aggregates whose partials do not merge, a GROUP BY on an expression, a select-list column GROUP BY does not name, an item a worker cannot evaluate -- are asked before the search as well (pwt_query_shape_allows_workers), so that a query the workers will refuse is costed as the serial plan it will run. Q7's GROUP BY on extract(year ...) is the case that showed why: chosen for LINEITEM first on the discount, refused after the plan, and run serially in that shape at five times its serial time. The setup cost is a session variable, optimizer_parallel_setup_cost, in microseconds, default 2000 per table. It was measured on this branch at about 2 ms for a single-table scan and about 8 ms for a three-table join, dominated by creating the worker threads with a THD each, opening every table once per worker, and building and draining the result containers. The parallel-query tests set it to 0, since their tables are far too small to repay it, and main.parallel_query_costing pins the three behaviours on a five-row dimension and a 20000-row fact table. Measured on TPC-H SF1 with histograms, four workers against serial: Q5 490 -> 164 ms, Q3 623 -> 188 ms, Q10 341 -> 139 ms, Q12 615 -> 191 ms, Q15 1225 -> 396 ms, Q1 3966 -> 1160 ms; Q11 and Q7 unchanged. This commit was prepared with Claude Code (Fable 5.1), which diagnosed the REGION/NATION splits and the Q7 regression on a scratch copy of the TPC-H data, implemented the costing, the two-family pruning, the worker-shaped costing of the parallel chain and the pre-plan gate, wrote the costing test, updated the existing tests, and ran the parallel-query tests in both protocols and the main suite. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Parallel Query: Allow limit where it is the manager limiting (TPC-H Q3) Claude code figured out it was safe, so lets see. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40012 PQ: merge AVG, MIN and MAX partials after the manager's table spills The manager merges a worker's partial for a group through the plan's own terminal. direct_add() hands the partial to each aggregate, and end_update() either opens the group with reset_field() or folds the partial into it with update_field(). Once the aggregation table spills from HEAP to disk, the terminal becomes end_unique_update(), which calls reset_field() for every row, before the write shows whether the group is new. For a group already in the table it then restores the stored row and calls update_field() with the same partial. reset_field() consumes the partial, so update_field() has to find it again. Item_sum_sum and Item_sum_count keep their direct values for that case and mark it with direct_reseted_field. Two aggregates lacked that: - Item_sum_avg::reset_field() zeroed direct_count, so update_field() added the partial's sum with a count of zero, and the average of such a group came out too high. - Item_sum_min_max::reset_field() cleared direct_added and had no direct_reseted_field, so update_field() compared the stored value with args[0], the manager's own argument over a base-table row the manager never fills, instead of with the partial. Item_sum_avg::reset_field() now leaves direct_count for update_field(), which already clears it. Item_sum_min_max gains direct_reseted_field, set by reset_field() and read and cleared by update_field(). A directly added item also bypasses the FILTER test in update_field(), as in Item_sum_sum, since the partial already reflects the filter. A serial plan never sets direct_added, so it never reaches the new branches. The grouped case already in main.parallel_query_spill has one row per group, so each group gets a single partial and the branch for a group already in the table never ran. The new case groups 20000 rows into 3000 groups, so each worker sends a partial for nearly every group, and the 64K heap limit makes the manager's table spill. Without either of the two changes, its parallel answer differs from the serial one. This commit was prepared with Claude Code (Opus 5.5), which found the wrong averages while letting the workers pre-aggregate on a wide GROUP BY column, traced them and the wrong MIN and MAX values to the reset_field() and update_field() pair under end_unique_update(), and showed with the new test that the AVG change and the MIN/MAX change each fix a failure of their own. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40012 PQ: let the workers pre-aggregate on a GROUP BY column too wide for an on-disk key The gate refused a grouped plan with a GROUP BY column too wide to be a key part, or with a key too long in total, on the grounds that create_tmp_table() would key the aggregation table by a unique constraint, a hash of the row with nothing to look a group up in. Since MDEV-38975 that holds only for a table that starts on disk: a HEAP table indexes a wide or blob key part, and Create_tmp_table::start() skips the fallback when it expects HEAP. Such a plan writes through end_update(), so pwt_plan_group_key() finds its key, and the gate then refused it on the per-column prediction alone. Both predictions are now out of the gate. A plan whose aggregation table starts on disk still has a unique constraint and no end_update() terminal, and the gate still refuses it, with the reason it already gave for that shape. Otherwise the containers are built the way the plan's own table was, and pwt_row_layout::build() checks the finished container for a group index, with plain rows shipped in place of partials if it has none. A worker container with a wide key fills sooner, and a worker that rebuilds it on disk inside end_update() exposed a fault in how memory moves between threads. The container's own MEM_ROOT was thread-specific, so whatever a worker allocated in it was charged to the worker and freed by the manager. pwt_end_send() moved the charge by bracketing put_record() and handing the whole change in the worker's local_memory_used to the manager. But that change also holds memory that stays with the worker: ha_maria::enable_indexes() allocates its HA_CHECK from thd->mem_root, the worker's own root, which ~THD frees later on the worker's thread. With that block counted against the manager, the worker's counter went negative in ~THD: Assertion `(longlong) thd->status_var.local_memory_used >= 0 || !debug_assert_on_not_freed_memory' failed. THD::~THD destroy_background_thd pwt_thread::init_and_run_thread_func A table one thread writes and another frees is not thread-specific memory, which is what HA_OPEN_GLOBAL_TMP_TABLE already says to its engines. Create_tmp_table::start() now gives a cross-thread table a root without MY_THREAD_SPECIFIC, and with nothing left to hand over, the hand-over code is removed: the bracket in pwt_end_send(), the same accounting in pwt_tmp_table_sink::emit_row() and cleanup(), and account_spilled_memory(). Tests: - main.parallel_query_why now asks for the on-disk shape (tmp_memory_table_size=0), which keeps the unique-constraint reason. At the default size the same query runs in the workers. - main.parallel_query_aggregate shows a VARCHAR(4000) GROUP BY that stays in HEAP and pre-aggregates in the workers, with fewer than 1000 tmp-table writes for 20000 rows and the serial answer. Its note on the no-group-index fallback no longer claims a gate refusal for wide keys. - main.parallel_query_spill shows the same kind of key at a 16M heap limit, where the workers rebuild their containers on disk part way through. Without the memory change it fails on the assertion above. A comment that still named accumulate_group(), long since replaced by end_update(), is corrected too. This commit was prepared with Claude Code (Opus 5.5), which traced the changed refusal reason to MDEV-38975, removed the two predictions, found the negative memory counter with the new spill case and located the HA_CHECK allocation with an rr recording, moved the container's root off thread-specific memory, and showed by reverting the memory change on its own that the new spill case fails on the assertion without it. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Petrunia
sergey@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
In Parallel_coordinator, remove partition_id() and m_partition_id. MariaDB's innodb doesn't have those. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-25848 one range slot per ARRAY index Before, every MEMBER OF / JSON_OVERLAPS / JSON_CONTAINS point got its own cloned KEY_PART and key slot, and points were combined only through index_merge (OR) or by picking one slot (AND). Now all points on an ARRAY key part share the original slot: * SEL_ARG_ARRAY holds a list of make_key() points ("the array contains any of them") and the summed records_in_range() estimate and cost, taken once when the point is built. * key_or() of two ARRAY ranges takes the union of the points; key_and() keeps the operand with the lower estimate, the WHERE condition checks the rest. * KEY_PART cloning (get_claimed_key_part()) is now RTREE-only, so ARRAY-only queries size SEL_TREE::keys by param->keys again. * QUICK_RANGE_SELECT_ARRAY gets one QUICK_RANGE per point and passes all points to hlindex_read_first(); json_index reads them in turn and skips trefs it has already returned. * check_quick_select_array() uses the stored estimate, and rejects non-ARRAY SEL_ARGs (e.g. IS NULL) on the ARRAY key part. ORed MEMBER OF and JSON_OVERLAPS now give a single range scan instead of index_merge sort_union, with no MAX_KEY limit on the number of points. Co-Authored-By: Claude Opus 5.5 <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-25848 one range slot per ARRAY index Before, every MEMBER OF / JSON_OVERLAPS / JSON_CONTAINS point got its own cloned KEY_PART and key slot, and points were combined only through index_merge (OR) or by picking one slot (AND). Now all points on an ARRAY key part share the original slot: * SEL_ARG_ARRAY holds a list of make_key() points ("the array contains any of them") and the summed records_in_range() estimate and cost, taken once when the point is built. * key_or() of two ARRAY ranges takes the union of the points; key_and() keeps the operand with the lower estimate, the WHERE condition checks the rest. * KEY_PART cloning (get_claimed_key_part()) is now RTREE-only, so ARRAY-only queries size SEL_TREE::keys by param->keys again. * QUICK_RANGE_SELECT_ARRAY gets one QUICK_RANGE per point and passes all points to hlindex_read_first(); json_index reads them in turn and skips trefs it has already returned. * check_quick_select_array() uses the stored estimate, and rejects non-ARRAY SEL_ARGs (e.g. IS NULL) on the ARRAY key part. ORed MEMBER OF and JSON_OVERLAPS now give a single range scan instead of index_merge sort_union, with no MAX_KEY limit on the number of points. Co-Authored-By: Claude Opus 5.5 <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40012 PQ: refuse a plan that reads a table through a join buffer A worker has no join buffer. setup_worker_jointabs() stripped the cache off every worker JOIN_TAB and joined a row at a time, and pwt_table_conds() handed the worker the buffer's scan filter (cache_select->cond) as well as select_cond so that it still rejected what the buffer would have. The result was correct, but it was not the plan the optimizer costed: BNL, BNLH, BKA and BKAH were chosen for scanning the inner table once per buffer-full of outer rows, and a worker scanned it once per driving row instead. For a full-scan inner table that can be orders of magnitude more work than the serial plan the parallel one is meant to beat. So the gate now refuses the plan, as the prototype did. The check sits in can_run_query_in_workers() beside the other per-table refusals, on tab->cache or tab->use_join_cache, both of which make_join_readinfo() has settled by the time parallel_join_check() runs in optimize_stage2(). The refusal names the table in the optimizer trace. The worker-side handling of a buffered table goes with it: - pwt_table_conds() becomes pwt_table_cond() and no longer reports cache_select->cond. remove_redundant_bnl_scan_conds() only moves conjuncts out of select_cond when the tab has a BNL/BNLH cache, so for every tab the gate now admits select_cond is already the whole condition. A tab can still carry a cache_select without a buffer, because make_join_select() builds the scan filter before check_join_cache_usage() decides, but its cond is then a copy of conjuncts select_cond still holds. The gate no longer asks whether that copy is worker-safe, since no worker evaluates it. - pwt_clone_table_conds(), which cloned both halves and ANDed them, is gone; setup_worker_jointabs() clones the one condition with pwt_clone_rebind(). - setup_worker_jointabs() no longer clears cache, use_join_cache and jbuf_tracker on the worker's copy; pwt_assert_tab_inert() asserts that the manager's tab has no cache instead. The copy's cache_select is still cleared, as it can be set on a tab without a buffer. parallel_query_join: the BNL case now expects the query to run serially, with the same rows. The full-scan inner table case (d3) was getting a BNL buffer under the default join_cache_level, so it runs with join_cache_level=0 to keep covering a worker-side full scan of an inner table. parallel_query_why pins the new refusal reason. This commit was prepared with Claude Code (Opus 5.5), which wrote the gate check, removed the worker-side join buffer handling it made unreachable, updated the tests, reconfigured and rebuilt the worktree, and ran the parallel_query tests in normal and --ps-protocol modes. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PQ: Code tidy ups and fixups of tidyups. parallel_query_aggregate.test: fix statistics impersonalize language in can_parallel_scan_jointab_access() remove dead code (trace_parallel_scan_options) add an assert to ensure we do not use Item_sum_avg without two packed fields minor comment fixups remove enum element NESTED_LOOP_DECLINED and use a local bool instead fix silly code in pwt_row_layout::build_aggregates (throwing away a pointer then searching for it again later); 'one' starts as nullptr, because GCC cannot see that whole_table is unchanged between the two blocks and warns -Wmaybe-uninitialized in an ASAN build remove pwt_batch* worker->manager streaming functions tidy up Parallel_scan_partitioner::Scan_ctx::create_chunks to use create_chunks_unsplit The 'one= nullptr' initialisation was added with Claude Code (Opus 5.5), which traced the -Wmaybe-uninitialized warning to this change and folded the fix in here. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleg Smirnov
olernov@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Incorporate a pointer to the coordinator into worker context | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PQ: separate finding a range's boundaries from cutting the scan up add_scan() walked the tree for one range and turned that range's boundaries into chunks in the same breath. Both halves are per range, and only the first one should be: whether a chunk may be divided again is a question about the scan, and a scan may hold many ranges of very different sizes. So add_scan() now partitions and keeps the boundaries on its Scan_ctx, and the caller adds every range it has before calling create_chunks() once. Behaviour is unchanged -- each range still decides for itself, in Scan_ctx::create_chunks(), exactly as it did before -- and this is the shape that lets that decision move. Measured with r_chunks in ANALYZE FORMAT=JSON, the same 50,000 rows of a million-row table, reached three ways: as one range, as ten equal ranges, and as one large range with nine tiny ones beside it. workers one range ten equal one large + nine tiny 4 chunks 10 103 28 pages 458 899 511 12 chunks 26 432 44 pages 522 2215 575 Ten ranges cost sixteen times the chunks and four times the pages of one range holding the same rows. The cost follows the number of ranges big enough to be partitioned, not how unevenly the rows sit across them, which is why the lopsided third shape is nearly as cheap as the first. Per-worker row counts are within the usual 3:1 tail in all three, so the extra chunks buy nothing -- the pull queue was already balancing the work. Why the decision has not moved with the call: It needs a measure of how much work a range holds, and the boundaries do not carry one. A level-0 walk cuts at the children of the root, so a range narrower than one of those -- which at this fan-out is any range under about a tenth of the table, and so most range scans -- yields exactly one boundary whether it covers ten rows or a hundred thousand. Two rules were built on those counts and both were worse than what they replaced. Deciding from the total number of chunks left the third shape's large range unsplit on a single worker while the other three had ten rows each. Weighting by whole sub-trees, on the reasoning that a range clamps only the first and last it touches, measured zero nearly everywhere and serialised all three shapes. What that rules out is worth keeping: any rule counting boundaries is deciding without knowing the sizes. The existing per-range rule is in that class too -- it survives because its failure is to over-split, which costs pages rather than answers. The number that does measure work is the count of sub-ranges a chunk yields when it is split, which is already computed inside Chunk::split(), where the piece count is decided. This commit was prepared with Claude Code (Opus 5), which took the measurements above, and wrote and discarded both of the rules described. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-42009: Set GLOBAL innodb_log_file_size=ON is not crash safe log_t::header_rewrite(): Before setting log_sys.archive=true, ensure that any garbage entries at the end of the file will be zeroed out so that they cannot be mistaken for newer records. On Linux and Microsoft Windows we will attempt a shortcut for writing zeroes, before falling back to log_file_t::write(). Reviewed by: Alessandro Vetere |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thirunarayanan Balathandayuthapani
thiru@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41336 CHECK TABLE EXTENDED crashes with innodb_force_recovery=3 Problem: ========= After a restart where purge has not run (When innodb_force_recovery=3), purge_sys.head and purge_sys.tail are both unset (trx_no=0). trx_lists_init_at_db_start() calls purge_sys.clone_oldest_view<true>(), which clamped end_view to that 0, so end_view.m_low_limit_id became 0 and changes_visible() returned false for every transaction id. end_view therefore claimed that no history had been purged, although undo log records purged before the restart may already have been freed and reused. CHECK TABLE EXTENDED relies on end_view to decide whether an old version's undo log record is still available. It followed the roll pointer of a purged transaction into a reused undo page and trx_undo_rec_get_pars() hit the assertion Solution: ======== In purge_sys_t::clone_oldest_view<true>(), clamp end_view only if purge has a known position (head.trx_no/tail.trx_no is nonzero). Otherwise keep the oldest view as is, so that the undo log records that purge is allowed to remove are reported as DB_MISSING_HISTORY rather than being read. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PQ: divide a multiple-range scan by the work it holds, not a range at a time Each range of a multiple-range scan divided itself: it compared its own chunk count against the worker count, found itself short, tagged what it had for re-splitting, and the splits that followed were per range as well. Ten ranges did that ten times over and the queue filled with chunks the scan never needed, without dividing the work any more evenly than a single range does. So for a scan of more than one range the work is measured first and divided afterwards. Measuring means walking one level below the sub-trees a level-0 walk finds, which gives a boundary per page: a page is the unit the measure needs, and the sub-trees are not -- a range narrower than one of those yields a single boundary whether it holds ten rows or a hundred thousand, which is why the two rules built on those counts both failed. With every range measured the same way, the chunks the scan should have -- twice the worker count -- are shared out in proportion to pages, and each range coalesces its own boundaries down to its share. Nothing is tagged: the division was made knowing the sizes, so there is nothing left for a worker to correct at run time. A scan of one range keeps the division it had. There is only one thing to divide, it already gets the whole worker count, and leaving it alone keeps this change off the common path. Measured with r_chunks in ANALYZE FORMAT=JSON. Ten ranges whose sizes go up by the golden ratio -- 253, 410, 663, 1073, 1736, 2809, 4545, 7355, 11900, 19255 rows -- against a million-row table. Sizes in that progression because no two of them are a whole multiple of each other, so nothing can come out balanced by accident: workers chunks pages rows on the busiest worker before after before after before after 4 97 18 854 582 14044 14893 8 169 25 1142 610 7686 8116 12 216 34 1330 646 5856 5518 16 261 41 1510 674 4880 4392 Five to six times fewer chunks, a third to a half fewer pages, and the spread across workers no worse -- better from eight workers up, where the busiest worker now has less to do than it did. The same ten ranges at equal sizes go from 103, 223, 432 and 432 chunks to 10, 20, 30 and 50; one large range beside nine tiny ones goes from 28, 36, 44 and 54 to 18, 26, 34 and 44, with the per-worker spread between 1.01 and 3.00 throughout. A single range is unchanged, which is the point of leaving it alone. Not yet measured, and the reason to: the deeper walk reads the pages one level down for every range, before the scan starts, where those reads used to happen inside a re-split and only for the chunks that were tagged. For these range sets it pays for itself several times over. For a range set much larger than these it might not, and that is a cost question rather than a correctness one. This commit was prepared with Claude Code (Opus 5), which took the measurements above, and wrote and discarded two earlier rules -- one counting chunks, one counting whole sub-trees -- before finding that neither count measures work. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40012 PQ: refuse DEFAULT() in an expression a worker evaluates DEFAULT(col) was evaluated against the worker's row. Item_default_value is an Item_field whose field is a private copy made by make_default_field(), reading the share's default_values but carrying the column's own table. Its deep_copy() shares that field, and Pwt_field_rebinder, seeing the manager's table, repointed it at the worker's real column. DEFAULT(col) then read the current row's value, and for an expression default calculate() called set_default() and wrote the default into the row -- wrong results in a release build, a marked_for_write assertion in a debug one. It shows only when the expression also names the column some other way; alone, DEFAULT(col) is table-independent and evaluated at optimize time. The gate now refuses any expression holding a DEFAULT(), found with check_func_default_processor, which only Item_default_value answers. Found by running the main suite with --parallel-worker-threads=4, where type_temporal_innodb lost a "Truncated incorrect BOOLEAN value" warning because the worker's WHERE evaluated to NULL. parallel_query_clone covers a lost row, a gained row and the expression default that crashed, and checks that none of them ran in the workers. This commit was prepared with Claude Code (Opus 5.5), which traced the wrong result from a main-suite sweep with workers enabled to the rebinder, wrote the gate refusal and the test, confirmed the test fails without the refusal, and ran the parallel_query tests in normal and --ps-protocol modes. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-42009: Set GLOBAL innodb_log_file_size=ON is not crash safe log_t::header_rewrite(): Before setting log_sys.archive=true, ensure that any garbage entries at the end of the file will be zeroed out so that they cannot be mistaken for newer records. On Linux and Microsoft Windows we will attempt a shortcut for writing zeroes, before falling back to log_file_t::write(). Reviewed by: Alessandro Vetere |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Petrunia
sergey@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Make declaration order match handler.h. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleg Smirnov
olernov@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Refactor for better object names and more accurate comments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 PQ: a LIKE is clonable, so a worker can evaluate it The gate asks whether every expression a worker would evaluate can be deep-copied without the copy sharing a node with the original, because the copy's Item_field leaves are repointed at the worker's own tables and a shared node would move the manager's item with them. Item_func_like failed that test: its escape is a child it keeps outside args, and Item_func_or_sum::deep_copy() clones args only, so the copy still pointed at the original's escape item. Every LIKE was therefore left to the manager as a residual condition, which for a LIKE on the driving table ships every row of the scan to be filtered serially -- TPC-H Q14's CASE WHEN p_type LIKE 'PROMO%' was declined outright, as an aggregate argument a worker cannot evaluate. Item_func_like::deep_copy() now clones the escape item as well. The TurboBM shift tables and the pattern are read-only, allocated on the statement's mem_root by fix_fields() and never freed by cleanup(), so the copy keeps pointing at them. Q14 on the SF1 scratch data: 566 ms serially, 180 ms with four workers and 113 ms with eight, with the same result. This commit was prepared with Claude Code (Fable 5.1), which found the LIKE in the gate's trace as the manager-evaluated share of a Q14 probe, wrote the deep_copy() override and the test, and measured the query. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41632 UPPER(key_col) IN (constant subquery) does not use the index 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. The rows returned were correct; the symptom is the plan. 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 shallow_copy() on the node being copied. It returns nullptr on out of memory, and when a copied node's shallow_copy() returns nullptr; every Item_func subclass whose shallow_copy() returns nullptr therefore overrides same_select_copy() to share the item, and a future class in that position must do the same or callers silently lose their rewrite. 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. Item_func_interval points its row member at the copied args[0], which it aliases. 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 hides it from arg_count until cleanup(), 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. Its real item is always a column, so the copy is the select list item itself, where the deep copy made a second Item_field for it. Sharing one item between the select list and the WHERE clause of a select is intended here: state that condition processing sets on the item, such as Item_field::item_equal, describes the same column in the same select. The equality is fixed when it is injected. 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. Claude Code (Fable 5.1) then reviewed the commit, corrected the commit message, kept Item_func_case_simple::shallow_copy() protected, added the Item_func_interval override and the nullptr note, and re-ran the tests. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||