get_clone() returns NULL on out-of-memory, but two of its callers in
prep_alter_part_table() dereferenced the result without a check, so the
server crashed:
changed INTERVAL of SYSTEM_TIME partitioning
ADD PARTITION under LOCK TABLES
get_clone() itself did not check the allocation of Vers_part_info and
dereferenced it.
handle_alter_part_error() did not check the clone either. The clone only
kept the DDL log entries after the table is closed. In the branch where
DDL log recovery fails, write_log_completed() released the entries
through lpt->part_info, which the table close may have freed, and
release_log_entries() released the same entries again through the
clone. That branch is not reachable now, as ddl_log_execute_entry()
always returns false.
The fix checks the allocation of Vers_part_info in get_clone() and the
result of get_clone() in prep_alter_part_table() like the other callers
there do. handle_alter_part_error() keeps a copy of DDL_LOG_STATE
instead of the clone and passes it to write_log_completed() and
release_log_entries(), which now take DDL_LOG_STATE. The redundant
release_log_entries() after write_log_completed() is removed.
ALTER TABLE ... PARTITION BY SYSTEM_TIME with a changed INTERVAL or
STARTS on a table already partitioned by SYSTEM_TIME took the fast
ALTER path, which only rewrites the table definition. The history rows
stayed where the old layout put them, so after the ALTER they were in
wrong partitions. An unchanged interval took the copy path instead.
MDEV-29873 replaced memcmp() of the intervals, which is true when they
differ, with interval_t::operator==, which is true when they are equal,
and lost the negation.
The copy path was also broken: it assigned the whole vers_info of the
new partitioning spec to the cloned partition info, now_part and
hist_part included. The ALTER does not list partitions, so now_part
became NULL and check_partition_info() failed with ER_VERS_WRONG_PARTS
for a table with explicit partitions (MDEV-33370 test case).
The fix negates the comparison again (now with interval_t::operator!=),
so a changed interval rebuilds the table and redistributes the history.
It copies only interval, limit and auto_hist from the new spec and keeps
now_part and hist_part of the existing partitions.
Without STARTS the start of the interval defaults to the current time
rounded down to the INTERVAL unit. So ALTER with the same INTERVAL and
no STARTS usually gets a different start and now rebuilds the table.
Vers_part_info::operator=, which copied now_part and hist_part and had
no other callers, is deleted.
Fast ALTER was also chosen when the same statement changed the table
definition, e.g. ADD COLUMN with the same INTERVAL and STARTS, or DROP
COLUMN with LIMIT. It wrote the new .frm, but the partitions kept the
old record layout. Now any change of the table definition
(alter_info->flags) takes the copy path.
CONVERT ... PARTITION passed user-specified partition names to
create_partition_name() with translate=FALSE, so names needing
filename escaping (e.g. `foo-bar`) didn't match the on-disk file.
Pass TRUE to encode them.
The ALTER TABLE.. CONVERT TABLE TO invalided the table cache of the
previously normal table that was moved and which left the query cache refering
to a removed table by the time it was invalidated after the ALTER.
Let's invalidate the query cache at the beginning of the fast path for
all variants of the ALTER TABLE (partitioning).
This prevents a use after free ASAN error that was previous there when the
query_cache_invalidate was use on the source table that was freed from the
table cache.
Alternates considered:
* A query cache invalidate early CONVERT_IN and leave the
fast_end_partition invalidate - still results in ASAN assertion.
* A fast_end_partition with an argument to skip QC clear -
seemed to treat this special ALTER TABLE too specially.
Review: Sanja Byelkin
1. Fix empty part_elem->id in prep_alter_part_table().
On auto-create newly added partition has id 0. It came from
set_up_default_partitions() for new part_info
(thd->work_part_info). vers_update_el_ids() can work only with
unassigned ids (UINT_MAX32), so we assign it explicitly on pushing
into tab_part_info.
2. If range value is out of TIMESTAMP_MAX_VALUE set it to
TIMESTAMP_MAX_VALUE, but only if the history partition is the last
one, otherwise push ER_DATA_OUT_OF_RANGE. Error is to create
multiple out-of-range partitions (e.g. with PARTITIONS clause in
CREATE TABLE).
Wrong "fastpath" boundary conditions. The right boundary is open, the
left boundary is closed. The condition range_value[loc_hist_id - 1] as
left boundary must be checked with <= operator. The same with the
right boundary range_value[loc_hist_id] as the correct way is to check
it with > operator, the right boundary check was rewritten for clear
understanding:
ts < range_value[loc_hist_id]
The below code confirms closed endpoint type for left boundary:
if (range_value[loc_hist_id] <= ts)
min_hist_id= loc_hist_id + 1;
Also the endpoint type (closed for left, open for right) is confirmed
by vers_set_hist_part() for DML:
else if (vers_info->interval.is_set() &&
vers_info->hist_part->range_value <= thd->query_start())
and here (right boundary, rewritten for clarity):
if (thd->query_start() < next->range_value)
{
error= false;
break;
}
- 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.
MDEV-33658 part 1’s refactoring ecaedbe299
introduced a new function init_key_info which (in part) aims to
calculate the total key length; however, it doesn’t account for the
key already having been initialized (as happens when called via
ALTER TABLE .. CONVERT PARTITION .. TO TABLE). This leads to crashes
when this key is later iterated over, because the iterator will try
to iterate over additional key parts which don’t exist because the
length reports as longer than the actual memory owned. The crash
reported by MDEV-36906 highlights this in function key_copy.
To explain how the keys already have been initialized, init_key_info
is called multiple times. That is, init_key_info is called from
mysql_prepare_create_table, which prepares a table and its key
structures for table creation, which is in turn called by
mysql_write_frm when using flags MFRM_WRITE_SHADOW and
MFRM_WRITE_CONVERTED_TO. The
ALTER TABLE .. CONVERT PARTITION .. TO TABLE use case (see function
fast_alter_partition_table), calls mysql_write_frm multiple times with
both of these flags set (first with MFRM_WRITE_CONVERTED_TO and then
with MFRM_WRITE_SHADOW).
Raising it up a level, mysql_prepare_create_table doesn't need to be
called again after it has already been invoked when just writing frms.
Init_key_info is the only place in that function which leads to side
effects, but the rest is redundant and can be skipped on the second
call (i.e. when writing the shadow).
The patch fixes this by skipping the call to mysql_prepare_create_table
in mysql_write_frm in the MFRM_WRITE_SHADOW block when it has already
been called previously. To track whether or not it has been previously
called, we add a new flag for the mysql_write_frm function,
MFRM_ALTER_INFO_PREPARED, which is hard-coded into the function call on
the later invocation.
Test case based on work by Elena Stepanova <elenst@mariadb.com>
Reviewed By:
============
Sergei Golubchik <serg@mariadb.org>
Nikita Malyavin <nikita.malyavin@mariadb.com>
* rpl.rpl_system_versioning_partitions updated for MDEV-32188
* innodb.row_size_error_log_warnings_3 changed error for MDEV-33658
(checks are done in a different order)
create templates
thd->alloc<X>(n) to use instead of (X*)thd->alloc(sizeof(X)*n)
and the same for thd->calloc(). By the default the type is char,
so old usage of thd->alloc(size) works too.
Improve performance of queries like
SELECT * FROM t1 WHERE field = NAME_CONST('a', 4);
by, in this example, replacing the WHERE clause with field = 4
in the case of ref access.
The rewrite is done during fix_fields and we disambiguate this
case from other cases of NAME_CONST by inspecting where we are
in parsing. We rely on THD::where to accomplish this. To
improve performance there, we change the type of THD::where to
be an enumeration, so we can avoid string comparisons during
Item_name_const::fix_fields. Consequently, this patch also
changes all usages of THD::where to conform likewise.
