When a select containing 32 ranges is made on a table containing a
compound key with 32 parts, the range optimizer can run off the end
of a stack variable, invalidly overwriting subsequent stack variables.
In the struct st_sel_arg_range_seq, we have an array
RANGE_SEQ_ENTRY stack[MAX_REF_PARTS];
MAX_REF_PARTS is 32.
check_quick_select
/ sel_arg_range_seq_init
initialises stack[0] as NOT a key part
/ sel_arg_range_seq_next
iterates through the key parts, adding key part n to stack[n+1]
key part #32 gets referenced by step_down_to(), setting seq->i off the
end of the array.
Fix: RANGE_SEQ_ENTRY stack[MAX_REF_PARTS+1];
The above change exposed an issue with key length calculation on
MS Windows. Calling make_prev_keypart_map(32) caused the resultant
bitmap to be calculated as (1UL << 32) - 1. Using the MSVC compiler
this resulted in an empty key length calculation during
handler::index_read_map, causing an assertion in ha_innobase::index_read().
As we only need 32 bits to represent our key map, we change the type thus
-typedef ulong key_part_map;
+typedef uint32 key_part_map;
We correct make_keypart_map() and make_prev_keypart_map() to call our
overflow safe my_set_bits(). We also correct bka_range_seq_next()
and bkah_range_seq_next() to use make_prev_keypart_map().
We also add some DBUG_ASSERTS in key_part_map processing elsewhere,
exposing some issues in our BNLH implementation. We cap the number of
keyuse parts here, altering the explain output of 2 of our tests.
Numerous places needed bit shift operations altered to use
make*keymap_part and various format strings needed to be corrected.
As UBSAN error, the attempt of evaluating a best_acess_path in the
optimizer was using -nan as its worst_seeks value. This didn't cast to
an integer for a rows estimate value resulting in the UBSAN error.
The blackhole engine had a worst_seeks derived from read_time (same
value). This was derived in the default handler::scan_time as
stats.data_file_length / stats.block_size expression where both where 0.
Corrected this by giving the default handler::scan_time an implementation
that just returns 0 for the case where stats.block_size was 0, to avoid
returning a NaN values for all storage engines that leave their
stats block_size as 0, including the backhole.
Reason:
=======
Following the changes in MDEV-38734, the server no longer marks
all indexed virtual columns during an UPDATE operation.
Consequently, ha_innobase::update() populates the upd_t vector
with old_vrow but omits the actual data for these virtual columns.
Despite this omission, trx_undo_page_report_modify() continues to
write metadata for indexed virtual columns into the undo log. Because
the actual values are missing from the update vector, the undo log
entry is recorded without the historical data for these columns.
When the purge thread processes the undo log to reconstruct a
previous record state for MVCC, it identifies an indexed virtual
column but finds no associated data.
The purge thread incorrectly interprets this missing data as a NULL
value, rather than a "missing/unrecorded" value. The historical
record is reconstructed with an incorrect NULL for the virtual column.
This causes the purge thread to incorrectly identify and purge
records that are not actually delete-marked, leading to abort
of server.
Solution:
=========
ha_innobase::column_bitmaps_signal(): Revert the column-marking
logic to the state prior to commit a4e4a56720, ensuring all
indexed virtual columns are unconditionally marked during an UPDATE.
The previous "optimization" attempted to manually detect indexed
column changes before marking virtual columns. The manual check
for indexed column modifications is redundant. InnoDB already
provides the UPD_NODE_NO_ORD_CHANGE flag within row_upd_step().
This flag is being used in trx_undo_page_report_modify() and
trx_undo_read_undo_rec() should log or read virtual column values.
Refactored column_bitmaps_signal() to accept a mark_for_update
parameter that controls when indexed virtual columns are marked.
TABLE::mark_columns_needed_for_update() is the only place that needs
mark_for_update=true because only UPDATE operations need to mark
indexed virtual columns for InnoDB's undo logging mechanism.
INSERT operation is already handled by
TABLE::mark_virtual_columns_for_write(insert_fl=true).
Even the commit a4e4a56720 changes are
going to affect TABLE::mark_virtual_column_for_write(false) and
It is called during UPDATE operation, and that's when
column_bitmaps_signal() needs to mark indexed virtual columns.
Online DDL has separate code path which is handled by
row_log_mark_virtual_cols() for all DML operations
Problem:
=======
InnoDB DDL does ALTER TABLE PAGE_COMPRESSED=1 because:
1. check_if_supported_inplace_alter() reads srv_file_per_table
during precheck and does allow INSTANT operation.
2. User does change innodb_file_per_table later.
But prepare phase read srv_file_per_table again during execution.
3. If the global variable changed between these phases, causes
server abort for page_compressed tables.
Solution:
=========
- Add a file_per_table member to Alter_inplace_info to
capture the innodb_file_per_table value during the
precheck phase and use that consistent value throughout
the entire ALTER TABLE operation.
- Converted boolean flags (online, ignore, error_if_not_empty,
mdl_exclusive_after_prepare) from individual boolean
members to bitfields packed in a single byte
- Reduced Alter_inplace_info structure size by 24 bytes
...just like the when deleting the .frm file in the same function.
This improves consistency: internal duplicate deletion of .par file
and .frm file both fail silently.
- 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.
See also MDEV-30046.
Idempotent write_row works same as REPLACE: if there is a duplicating
record in the table, then it will be deleted and re-inserted, with the
same update optimization.
The code in Rows:log_event::write_row was basically copy-pasted from
write_record.
What's done:
REPLACE operation was unified across replication and sql. It is now
representred as a Write_record class, that holds the whole state, and allows
re-using some resources in between the row writes.
Replace, IODKU and single insert implementations are split across different
methods, reluting in a much cleaner code.
The entry point is preserved as a single Write_record::write_record() call.
The implementation to call is chosen on the constructor stage.
This allowed several optimizations to be done:
1. The table key list is not iterated for every row. We find last unique key in
the order of checking once and preserve it across the rows. See last_uniq_key().
2. ib_handler::referenced_by_foreign_key acquires a global lock. This call was
done per row as well. Not all the table config that allows optimized replace is
folded into a single boolean field can_optimize. All the fields to check are
even stored in a single register on a 64-bit platform.
3. DUP_REPLACE and DUP_UPDATE cases now have one less level of indirection
4. modified_non_trans_tables is checked and set only when it's really needed.
5. Obsolete bitmap manipulations are removed.
Also:
* Unify replace initialization step across implementations:
add prepare_for_replace and finalize_replace
* alloca is removed in favor of mem_root allocation. This memory is reused
across the rows.
* An rpl-related callback is added to the replace branch, meaning that an extra
check is made per row replace even for the common case. It can be avoided with
templates if considered a problem.
Server-level UNIQUE constraints (namely, WITHOUT OVERLAPS and USING HASH)
only worked with InnoDB in REPEATABLE READ isolation mode, when the
constraint was checked first and then the row was inserted or updated.
Gap locks prevented race conditions when a concurrent connection
could've also checked the constraint and inserted/updated a row
at the same time.
In READ COMMITTED there are no gap locks. To avoid race conditions,
we now check the constraint *after* the row operation. This is
enabled by the HA_CHECK_UNIQUE_AFTER_WRITE table flag that InnoDB
sets in the READ COMMITTED transactions.
Checking the constraint after the row operation is more complex.
First, the constraint will see the current (inserted/updated) row,
and needs to skip it. Second, IGNORE operations become tricky,
as we need to revert the insert/update and continue statement execution.
write_row() (INSERT IGNORE) is reverted with delete_row(). Conveniently
it deletes the current row, that is, the last inserted row.
update_row(a,b) (UPDATE IGNORE) is reverted with a reversed update,
update_row(b,a). Conveniently, it updates the current row too.
Except in InnoDB when the PK is updated - in this case InnoDB internally
performs delete+insert, but does not move the cursor, so the "current"
row is the deleted one and the reverse update doesn't work.
This combination now throws an "unsupported" error and will
be fixed in MDEV-37233
Log tables cannot work with transactional InnoDB or Aria, that is
checked by ALTER TABLE for ER_UNSUPORTED_LOG_ENGINE. But it was
possible to circumvent this check with CREATE TABLE. The patch makes
the check of supported engine common for ALTER TABLE and CREATE TABLE.
This commit fixes a bug where Aria tables are used in
(master->slave1->slave2) and a backup is taken on slave2. In this case
it is possible that the replication position in the backup, stored in
mysql.gtid_slave_pos, will be wrong. This will lead to replication
errors if one is trying to use the backup as a new slave.
Analyze:
Replicated row events are committed with trans_commit_stmt() and
thd->transaction->all.ha_list != 0.
This means that backup_commit_lock is not taken for Aria tables,
which means the rows are committed and binary logged on the slave
under BLOCK_COMMIT which should not happen.
This issue does not occur on the master as thd->transaction->all.ha_list
is == 0 under AUTO_COMMIT, which sets 'is_real_trans' and 'rw_trans'
which in turn causes backup_commit_lock to be taken.
Fixed by checking in ha_check_and_coalesce_trx_read_only() if all handlers
supports rollback and if not, then wait for BLOCK_COMMIT also for
statement commit.
7544fd4cae had to make use of a static array to avoid memory
use-after-free or leak.
Instead, let us make a function returning String, this is the only way
to automatically manage the memory after the function returned.
To make it all correct, move constructor is added. Normally, it is
expected, that the constructor will be elided upon return of an object
by value, but if something goes different, or -fno-elide-constructors is
used, we can have a problem. So this was a move constructor avoids
copy elision-related UB.
dbug_print_row returning char* is still there for convenient use in a
debugger.
Partial commit of the greater MDEV-34348 scope.
MDEV-34348: MariaDB is violating clang-16 -Wcast-function-type-strict
Reviewed By:
============
Marko Mäkelä <marko.makela@mariadb.com>
When a derived table which has distinct values and BLOB fields is
materialized, an index is created over all columns to ensure only
unique values are placed to the result.
This index is created in a special mode HA_UNIQUE_HASH to support BLOBs.
Later the optimizer may incorrectly choose this index to retrieve values
from the derived table, although such type of index cannot be used
for data retrieval.
This commit excludes HA_UNIQUE_HASH indexes from adding to
`JOIN::keyuse` array thus preventing their subsequent usage for
data retrieval
work consistently on replication
Row-based replication does not execute CREATE .. SELECT but instead
CREATE TABLE. CREATE .. SELECT creates implict system fields on
unusual place: in-between declared fields and select fields. That was
done because select_field_pos logic requires select fields go last in
create_list.
So, CREATE .. SELECT on master and CREATE TABLE on slave create system
fields on different positions and replication gets field mismatch.
To fix this we've changed CREATE .. SELECT to create implicit system
fields on usual place in the end and updated select_field_pos for
handling this case.
