This merges the patch series that fixes a lot of missing validation
checks for read_log_event(). The merge includes a few added fixes that
are needed in new code introduced in 12.3.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Problem: GROUP BY with MIN()/MAX() can return the opposite extremum
when loose index scan chooses among indexes with different ordering.
MDEV-32732 (ec6f320883) added support for
descending indexes in loose index scan. It exposed a previously harmless
incorrect MIN/MAX key-part pointer: index enumeration overwrites
min_max_arg_part even when the candidate is rejected or not selected.
The pointer can then refer to a different index from the chosen plan.
Using its index-specific key_part_flag to determine scan direction
causes the regression.
Fix: Keep the MIN/MAX key-part pointer local to each candidate and
use it for the candidate's gap, infix and predicate checks. Save it
with the chosen index only when that candidate becomes the best plan.
(Cosmetic edits to testcase and commit comment by Sergei Petrunia)
This merges the patch series that fixes a lot of missing validation
checks for read_log_event(). The merge includes a few added fixes that
are needed in new code introduced in 11.8.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
When a versioned table stores its history in partitions, an implicit
query (no FOR SYSTEM_TIME) needs only current rows, which all live in
the current partition. vers_setup_conds() no longer adds a row_end
condition for such a table, and prune_partitions() restricts
read_partitions to the current partition instead.
read_partitions is reset to all partitions every statement in
open_table(), so the restriction must be applied on each execution.
prune_partitions() runs on each execution, also for merged derived
tables and views, where vers_setup_conds() is not called again. Both
functions guard with !vers_conditions.was_set(). In prune_partitions()
!is_set() would not do: set_all() sets type (flips is_set()) but not
orig_type (was_set()), so is_set() would be true on prepared-statement /
stored-procedure re-executions and skip the restriction, leaving history
partitions readable - rows matched then grow on each execute/call.
A query whose time comes from @@system_versioning_asof must still read
history, so the restriction is also skipped when from_sysvar is set.
init_from_sysvar() sets this new flag rather than orig_type, so
was_set(), print() and the write lock check still see only an explicit
FOR SYSTEM_TIME. Like orig_type, the flag persists to prepared-statement
re-execution.
EXPLAIN no longer shows "Using where" for such a query: correctness now
comes from partition selection rather than a row_end filter.
With sql_safe_updates, DELETE without WHERE on such a table is now
rejected even with LIMIT, as for a non-versioned table. With an index
on row_end, UPDATE or DELETE without WHERE (UPDATE also without LIMIT)
is now rejected too: it was accepted as a range scan on row_end. A
versioned table without SYSTEM_TIME partitions keeps the old behavior
(MDEV-41393).
read_partitions is reset to all partitions (or the PARTITION() set) once
per statement in open_table() (set_partition_bitmaps()), and
prune_partitions() only intersects it with lock_partitions and the pruned
set instead of rebuilding it.
mark_all_partitions_as_used() is dropped.
Pure refactoring, results unchanged. It lets a restriction set on
read_partitions before pruning survive it - needed when read_partitions
is pre-restricted, e.g. a versioned table whose history is partitioned
reads only the current partition.
prune_partitions() built the used-partition set directly in
read_partitions: it cleared it, let find_used_partitions*() fill it, and
in the index-merge case used it as scratch too - both buffer and result.
The patch adds PART_PRUNE_PARAM::parts_bitmap and builds the return
into it instead. read_partitions is produced from it only at the end:
copied when the condition was analyzed (IMPOSSIBLE included,
parts_bitmap left empty), or set to all locked partitions via
mark_all_partitions_as_used() when nothing could be pruned.
Pure refactoring: read_partitions ends up identical on every path. It
decouples the pruning scratch from read_partitions, so a later commit can
switch the final bitmap_copy() to bitmap_intersect() - letting pruning
narrow a pre-set read_partitions (e.g. a versioned table selecting only
its current partition) instead of overwriting it.
When an uncommitted transaction inserts rows into a table and
another statement locks rows in the same table (SELECT ... FOR UPDATE)
while computing a MIN or MAX, then:
1. In a Debug build, the server aborts on an assertion
2. In a Release build, the server returns wrong results
These errors occur because, while reading a group of rows for computing
a MAX, the transaction timeout error was swallowed.
Under the scenario described above and captured in the new test at this
commit, QUICK_GROUP_MIN_MAX_SELECT::next_max() emits a lock timeout error
during QUICK_GROUP_MIN_MAX_SELECT::get_next() but the error was suppressed
if we computed a MIN.
The InnoDB storage engine has an unwritten convention that after it has
returned a fatal error (which is any error except HA_ERR_END_OF_FILE
or HA_ERR_KEY_NOT_FOUND), then the SQL layer should not try to make
any further reads. This is because InnoDB might have rolled back
the current transaction already. So in the case of an error, return
immediately from QUICK_GROUP_MIN_MAX_SELECT::get_next().
An incorrectly backported test from 11.x revealed an UBSAN error in 10.11, so
fix that problem by preventing a division-by-zero from happening.
Remove the other incorrectly backported tests and relabel the retained test
in terms of the current ticket.
The code in QUICK_SELECT_DESC::get_next() had the logic that
"if index_last() call has returned nothing, then the table must be empty
so return HA_ERR_END_OF_FILE"
with ICP, index_last() may also return HA_ERR_END_OF_FILE when none of
the rows in the range matched ICP condition. But there can be matching
rows in other ranges, so continue the scan to examine them.
For some range condition x > N, a descending scan will walk off of the
left edge of the range and scan to the beginning of the index if we don't
set an endpoint on the storage engine.
In this patch, the logic to set such an endpoint is used in two places
now, so it's factored to a helper function.
In QUICK_SELECT_DESC::get_next(), when starting to scan a range with
no start endpoint, like "key < 10", clear the end_range on the storage engine.
We could have scanned another range before, like "key BETWEEN 20 and 30"
and after this the engine's end_range points to its left endpoint, the "20".
This is necessary especially when there are multiple ranges considered and
a later range indicates that there's no minimum endpoint (such as in the
attached test case), thus an earlier range endpoint must be removed/cleared.
QUICK_SELECT_DESC::get_next() sets a stale end range on the storage engine.
When checking the first range, it sets last_range to the first range and
then, if it cannot avoid descending scan, it sets the minimum endpoint on
the storage engine. However, get_next() may decide that this range is not
a match after updating the minimum endpoint on the storage engine. In this
case, it will continue to the next range. When continuing, it needs to
reset not only the state of 'last_range' to point to the next range that
it's checking, but it also needs to clear the now-stale end range set on
the storage engine. While this explanation covers the first and second
ranges, the issue at hand extends to the general case of ranges 1...N-1 and N.
Before this change and when get_next() decided that a range was not a
match, it would clear last_range at each loop continuation point.
Rather than clearing the minimum endpoint on the storage engine at
each continuation, we move all loop cleanup to for loop update clause.
This consolidates any logic that needs to be evaluated on loop
continuation to one place and reduces code duplication.
MySQL fixed this same problem at sha b65ca959efd6ec5369165b1849407318b4886634
with a different implementation.
When the optimizer_max_sel_arg_weight is set to 1, a nested query
crashed while tracing.
SEL_ARG object has a field named 'field', that is not set when the
type is other than KEY_RANGE. But, the field was accessed to store
its name, and weight to the trace. This resulted in a crash due to NULL
pointer.
Added a check to access field if the type is KEY_RANGE, and if not, just
trace the type.
Loose Index Scan optimization can only be used when the index it is
uses covers all columns used by the query. So, it would not be
attempted for indexes that are not present in table->covering_keys.
Before commit ed0a723566 (pushed to 11.0), Clustered PK would be
present in table->covering_keys if primary key columns are covering.
However, that commit gave Clustered PKs special treatment: they are
no longer present in table->covering_keys. This caused Loose Scan not
to be considered for Clustered PKs.
Fixed this by making the check in get_best_group_min_max() to allow
Loose Scan if all fields used by the query are parts of the Clustered PK.
Also made changes:
- The logic in JOIN_TAB::save_explain_data() assumed that Loose Scan
could only be used for index-only scans. This is not the case for
clustered PKs.
- One query in group_min_max_innodb started to use Loose Scan. It did so
in 10.11, too.
(Variant 2)
SEL_TREE* tree_or(SEL_TREE *X, SEL_TREE *Y) tries to conserve memory by
reusing object *X for the return value when possible.
MDEV-34620 has added logic to disable construction of index_merge plans
for N-way ORs with large N. That logic interfered with object reuse logic:
for the parameters of:
X = SEL_TREE{ trees=[key1_treeA, key2_treeB]}
Y = SEL_TREE{ trees=[key1_treeC]}
we would decide to reuse object X.
For key1, we would produce key_or(key1_treeA, key1_treeC)
but key2_treeB would be just left there.
Then, we would construct a range scan from key2_treeB.
Fixed by moving the "disable building index_merge plans" logic into a
location where it would not interfere with object reuse logic.
The "FETCH FIRST n ROWS WITH TIES" was not enforced when the SELECT used
"using index for group-by".
This was caused by an optimization which removed the ORDER BY clause
when the GROUP BY clause prescribed a compatible ordering.
Other GROUP BY strategies used workarounds to still handle WITH TIES,
see comment to using_with_ties_and_group_min_max() in this patch for
details. QUICK_GROUP_MIN_MAX_SELECT didn't have a workaround.
Fix this by disabling removal of ORDER BY when
QUICK_GROUP_MIN_MAX_SELECT is used.
Introduces NO_INDEX_MERGE and INDEX_MERGE, which control whether or
not index merge strategies are used during query optimization. Here
is an example query from the tests:
SET optimizer_switch='index_merge_intersection=off';
EXPLAIN SELECT /*+ INDEX_MERGE(t1 f4, f2) */ COUNT(*) FROM t1
WHERE f4 = 'h' AND f2 = 2;
with the hint in place, the query plan will employ the index_merge
intersect strategy (abbreviated EXPLAIN output):
type Extra
index_merge Using intersect(f2,f4); Using where; Using index
The presence of the [NO_]INDEX_MERGE hint overrides the optimizer's
choice of keys during the index merge optimization. As we see in
the above example, keys f2 and f4 and given and the optimizer will
consider only those keys for this query.
When the hint is given without any particular keys, as in
INDEX_MERGE(table), then all keys are considered. In this case, the
cheapest index merge among the keys should be used. When
NO_INDEX_MERGE(table) is given, then index merge is disabled for
that table.
When the hint is given with one or more keys, then only those keys
are considered. In the case of NO_INDEX_MERGE, those keys are
excluded. This can lead to no merged indexes at all, because
there may not be sufficient row-ordered read columns available for
consideration.
The index merge strategies of intersection, union, and sort union
cannot themselves be directly controlled via the hints. In combination
with the optimizer switches for the same, the strategy may be
indirectly controlled but this is not guaranteed.
When the hint directs the optimizer such that insufficient ROR scans
are available, thus leading to a situation where the INDEX_MERGE hint
cannot be honored, the server will emit a warning to that effect.
In the hints module (opt_hints*{cc,h}), this commit adds some
index merge-specific functionality to make interpreting hint state
at callsites in the optimizer cleaner and more intuitive.
Particularly, we add a bit field to the table hints class which
indicates the keys that are marked by an [NO_]INDEX_MERGE hint, if
present. A new function, index_merge_hint (and associated new
helper functions) relies on this field when interpreting index merge
hint state for the optimizer.
If there are no index merges available prior to attemping to find
a suitable union/sort union, then the optimizer will not attempt
it. This change results in optimizer trace output which does not
include the 'analyzing_index_merge_union' block when there are no
merges.
Parts of this implementation based on MySQL commit
ebcb981807e3d91a64782e89d48e1a25622eafea
Introduces code cleanup and small refactorings, such as:
- Flip keys_to_use.is_clear_all() and de-dent
- Rename best_trp to best_read_plan
- Fix key_parts assignment spacing
- find_shortest_key now keeps happy path to the left
- Cleanup in the get_best_ror_intersect, remove_nonrange_trees,
and restore_nonrange_trees functions
- Reorganization of locals within test_quick_select
- De-dent some large code blocks by changing
if (key)
{
LOTS-OF-CODE;
}
into
if (!key)
continue;
LOTS-OF-CODE;
This commit introduces two optimizer hints: ROWID_FILTER and NO_ROWID_FILTER.
They allow explicit control over whether certain indexes are used to build
rowid filters.
Syntax:
[NO_]ROWID_FILTER(tbl_name@query_block_name [index_name [, index_name] ...])
Hints can be applied at two levels:
Table-level (only table name specified): enables or disables rowid filters
for the given table.
Index-level (table and index names specified): forces the server to use
or ignore the listed indexes when building rowid filters.
Examples:
SELECT /*+ rowid_filter(t1)*/ a FROM t1 WHERE a < 'e' AND b > 't';
SELECT /*+ no_rowid_filter(t1)*/ a FROM t1 WHERE a < 'e' AND b > 't';
SELECT /*+ rowid_filter(t1 key_a, key_b)*/ a FROM t1 WHERE a < 'e' AND b > 't';
SELECT /*+ no_rowid_filter(t1 key_c)*/ a FROM t1 WHERE a < 'e' AND b > 't';
===== Interaction with other index hints =====
If [NO_]INDEX, [NO_]JOIN_INDEX hints are specified for the same table,
then only indexes whitelisted by INDEX() or JOIN_INDEX() can be considered
for rowid filters. ROWID_FILTER() hint cannot force the use of indexes that are
disabled by NO_INDEX() / NO_JOIN_INDEX() or omitted from INDEX() / JOIN_INDEX().
For example, if one index should be used for data access and another for
a rowid filter, the filter index must be mentioned in both hints:
SELECT /*+ index(t1 key_a, key_b) rowid_filter(t1 key_b) a FROM t1 ....
- Removed duplicate words, like "the the" and "to to"
- Removed duplicate lines (one double sort line found in mysql.cc)
- Fixed some typos found while searching for duplicate words.
Command used to find duplicate words:
egrep -rI "\s([a-zA-Z]+)\s+\1\s" | grep -v param
Thanks to Artjoms Rimdjonoks for the command and pointing out the
spelling errors.
main/statistics_json.result is updated for f8ba5ced55 (MDEV-36099)
The test uses 'delete from t1' in many places and then populates
the table again. The natural order of rows in a MyISAM table is well
defined and the test was implicitly relying on that.
before f8ba5ced55 delete was deleting rows one by one, using
ha_myisam::delete_row() because the connection was stuck in rbr mode.
This caused rows to be shown in the reverse insertion order (because of
the delete link list).
MDEV-36099 fixes this bug and the server now correctly uses
ha_myisam::delete_all_rows(). This makes rows to be shown in the
insertion order as expected.
Description:
The function get_best_ror_intersect is responsible for selecting
the optimal combination of ROR scans that minimize cost while
improving selectivity. It iteratively adds scans to a selected
set (S), ensuring that each addition results in improved selectivity.
If selectivity improves, the function then evaluates whether
the cost is minimized.
The comment contained some inaccuracies:
- Incorrect Selectivity Condition:
A missing parentheses caused the condition to be misinterpreted,
leading to incorrect logic. The function intends to check whether
adding a scan improves selectivity before including it in the set.
- Loop Condition Issue:
The condition for continuing the loop did not properly reduce R in
the comment which meant it was an infinite loop.
Fix:
The comment of the function is fixed addressing the issues.
The set should include the scan before comparing the
selectivity with the initial set and thus the selectivity condition
in the comment is fixed by properly enclosing the expression in
parentheses to reflect the intended logic.
Ensured that R is properly reduced in each iteration to maintain
correctness.
Change-Id: Ie197af8211a5ef05a5118a33b8b543d354475780
If a table had a primary_key index, non_primary_key index,
and an extended index with primary key in desc order,
then the query with where clause predicates containing these
index fields was picking an index_merge plan which was causing
a wrong result.
The plan shouldn't have contained index_merge in the first place,
because the rows produced by one index have rows ordered by rowid
in ascending order, and the rows coming from the other index
were in a different order.
The solution is to not produce index_merge plan in such scenarios.
is_key_scan_ror() would now return false, for non_primary_key indexes
if any primary key part with a reverse sort is present in them.
