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>
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.
Similar to #2480.
567b681 introduced safe_strcpy() to minimize the use of C with
potentially unsafe memory overflow with strcpy() whose use is
discouraged.
Replace instances of strcpy() with safe_strcpy() where possible, limited
here to files in the `sql/` directory.
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer
Amazon Web Services, Inc.
When INSERT does auto-create for t1 all its handler instances are
closed by alter_close_table(). At this time down the stack
maria_close() clears share->state_history. Later when we unlock the
tables Aria transaction manager accesses old share instance (the one
before t1 was closed) and tries to reset its state_history.
The problem is maria_close() didn't remove table from transaction's
list (used_tables). The fix does _ma_remove_table_from_trnman() which
is triggered by HA_EXTRA_PREPARE_FOR_RENAME.
Some fixes related to commit f838b2d799 and
Rows_log_event::do_apply_event() and Update_rows_log_event::do_exec_row()
for system-versioned tables were provided by Nikita Malyavin.
This was required by test versioning.rpl,trx_id,row.
MDEV-33502 Slowdown when running nested statement with many partitions
This change was triggered to help some MariaDB users with close to
10000 bits in their bitmaps.
- Change underlaying storage to be 64 bit instead of 32bit.
- This reduses number of loops to scan bitmaps.
- This can cause some bitmaps to be 4 byte large.
- Ensure that all not used top-bits are always 0 (simplifes code as
the last 64 bit storage is not a special case anymore).
- Use my_find_first_bit() to find the first set bit which is much faster
than scanning trough things byte by byte and then bit by bit.
Other things:
- Added a bool to remember if my_bitmap_init() did allocate the bitmap
array. my_bitmap_free() will only free arrays it did allocate.
This allowed me to remove setting 'bitmap=0' before calling
my_bitmap_free() for cases where the bitmap's where allocated externally.
- my_bitmap_init() sets bitmap to 0 in case of failure.
- Added 'universal' asserts to most bitmap functions.
- Change all remaining calls to bitmap_init() to my_bitmap_init().
- To finish the change from 2014.
- Changed all usage of uint32 in my_bitmap.h to my_bitmap_map.
- Updated bitmap_copy() to handle bitmaps of different size.
- Removed const from bitmap_exists_intersection() as this caused casts
on all usage.
- Removed not used function bitmap_set_above().
- Renamed create_last_word_mask() to create_last_bit_mask() (to match
name changes in my_bitmap.cc)
- Extended bitmap-t with test for more bitmap functions.
prep_alter_part_table upon re-partitioning by system time
memcmp() tries to compare beyond the last member of interval because
sizeof(Vers_part_info::interval) is 80. It is sizeof of variable,
sizeof of type is 76.
Now we compare interval_t struct C++ way.
- agressively -> aggressively
- exising -> existing
- occured -> occurred
- releated -> related
- seperated -> separated
- sucess -> success
- use use -> use
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.
For commands
(1) alter table t1 add partition (partition p2);
(2) alter table t1 add partition (partition px history);
It printed the same error message:
Wrong partitioning type, expected type: `SYSTEM_TIME`
For (1) it is not clear from the syntax that we are trying to add
HASH partition. For (2) it is not clear that the table partitioning is
different than SYSTEM_TIME. Now it prints what type we are trying to
add to what type of partitioning.
Fixed warning unused function rename_field_in_list() for compilation
without partitioning.
For commands
(1) alter table t1 add partition (partition p2);
(2) alter table t1 add partition (partition px history);
It printed the same error message:
Wrong partitioning type, expected type: `SYSTEM_TIME`
For (1) it is not clear from the syntax that we are trying to add
HASH partition. For (2) it is not clear that the table partitioning is
different than SYSTEM_TIME. Now it prints what type we are trying to
add to what type of partitioning.
