The server doesn't use the enforced storage engine in ALTER TABLE
without ENGINE clause to avoid an unwanted engine change.
However, the server tries to use the enforced engine in CREATE
INDEX. As a result, the false positive error is raised. The server
should not apply the enforced engine in CREATE INDEX too.
1032 (Can't find record) could be emitted when ALTER TABLE is execued vs
concurrent DELETE/UPDATE/other DML that would require search on the online
ALTER's side.
Innodb's INPLACE, in comparison, creates a new trx_t and uses it in scope
of the alter table context.
ALTER TABLE class of statements (i.g. CREATE INDEX, OPTIMIZE, etc.) is
expected to be unaffected by the value of current session's transaction
isolation.
This patch save-and-restores thd->tx_isolation and sets in to
ISO_REPEATABLE_READ for almost a whole mysql_alter_table duration, to avoid
any possible side-effect of it. This should be primarily done before the
lock_tables call, to initialize the storage engine's local value correctly
during the store_lock() call.
sql_table.cc: set thd->tx_isolation to ISO_REPEATABLE_READ in
mysql_alter_table and then restore it to the original value in the end of
the call.
The assert's reason was in missed FL_DDL flagging of CREATE-or-REPLACE
Query event.
MDEV-27365 fixes covered only the non-pre-existing table execution branch so
did not see a possibility of implicit commit in
the middle of execution in a rollback branch when the being CREATEd
sequence table is actually replaced.
The pre-existing table branch cleared the DDL modification
flag so the query lost FL_DDL in binlog and its parallel execution
on slave may have ended up with the assert to indicate the query
is raced by a following in binlog order event.
Fixed with applying the MDEV-27365 pattern.
An mtr test is added to cover the rollback situation.
The description test [ pass ] with a generous number of mtr parallel
reties.
Problem was that total order isolation (TOI) is started before
we know sequence implementing storage engine. This led to
situation where table implementing persistent storate
for sequence in case of MyISAM was created on applier causing
errors later in test execution.
Therefore, in both CREATE SEQUENCE and ALTER TABLE to implementing
persistent storage we need to check implementing storage engine
after open_tables and this check must be done in both master
and applier, because if implementing storage engine is MyISAM
it does not support rollback.
Added tests to make sure that if sequence implementing storage
engine is MyISAM or we try to alter it to MyISAM user gets error
and changes are not replicated.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
In MDEV-31086, SET FOREIGN_KEY_CHECKS=0 cannot bypass checks that
make column types of foreign keys incompatible. An unfortunate
consequence is that adding an AUTO_INCREMENT is considered
incompatible in Field_{num,decimal}::is_equal and for the purpose
of FK checks this isn't relevant.
innodb.foreign_key - pragmaticly left wait_until_count_sessions.inc at
end of test to match the second line of test.
Reporter: horrockss@github - https://github.com/MariaDB/mariadb-docker/issues/528
Co-Author: Marko Mäkelä <marko.makela@mariadb.com>
Reviewer: Nikita Malyavin
For the future reader this was attempted:
Removing AUTO_INCREMENT checks from Field_{num,decimal}::is_equals
failed in the following locations (noted for future fixing):
* MyISAM and Aria (not InnoDB) don't adjust AUTO_INCREMENT next number
correctly, hence added a test to main.auto_increment to catch
the next person that attempts this fix.
* InnoDB must perform an ALGORITHM=COPY to populate NULL values of
an original table (MDEV-19190 mtr test period.copy), this requires
ALTER_STORED_COLUMN_TYPE to be set in fill_alter_inplace_info
which doesn't get hit because field->is_equal is true.
* InnoDB must not perform the change inplace (below patch)
* innodb.innodb-alter-timestamp main.partition_innodb test would
also need futher investigation.
InnoDB ha_innobase::check_if_supported_inplace_alter to support the
removal of Field_{num,decimal}::is_equal AUTO_INCREMENT checks would need the following change
diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
index a5ccb1957f3..9d778e2d39a 100644
--- a/storage/innobase/handler/handler0alter.cc
+++ b/storage/innobase/handler/handler0alter.cc
@@ -2455,10 +2455,15 @@ ha_innobase::check_if_supported_inplace_alter(
/* An AUTO_INCREMENT attribute can only
be added to an existing column by ALGORITHM=COPY,
but we can remove the attribute. */
- ut_ad((MTYP_TYPENR((*af)->unireg_check)
- != Field::NEXT_NUMBER)
- || (MTYP_TYPENR(f->unireg_check)
- == Field::NEXT_NUMBER));
+ if ((MTYP_TYPENR((*af)->unireg_check)
+ == Field::NEXT_NUMBER)
+ && (MTYP_TYPENR(f->unireg_check)
+ != Field::NEXT_NUMBER))
+ {
+ ha_alter_info->unsupported_reason = my_get_err_msg(
+ ER_ALTER_OPERATION_NOT_SUPPORTED_REASON_AUTOINC);
+ DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
+ }
With this change the main.auto_increment test for bug #14573, under
innodb, will pass without the 2 --error ER_DUP_ENTRY entries.
The function header comment was updated to reflect the MDEV-31086
changes.
While cleaning up a failed CREATE TABLE LIKE <sequence>, `mysql_rm_table_no_locks`
erroneously attempted to remove all tables involved in the query, including
the source table (sequence).
Fix to temporarily modify `table_list` to ensure that only the intended
table is removed during the cleanup.
Fixed missing initialization of Alter_info()
This could cause crashes in some create table like scenarios
where some generated indexes where automatically dropped.
I also added a test that we do not try to drop from index_stats for
temporary tables.
The MDEV-29693 conflict resolution is from Monty, as well as is
a bug fix where ANALYZE TABLE wrongly built histograms for
single-column PRIMARY KEY.
Also includes a fix for safe_malloc error reporting.
Other things:
- Copied main.log_slow from 10.4 to avoid mtr issue
Disabled test:
- spider/bugfix.mdev_27239 because we started to get
+Error 1429 Unable to connect to foreign data source: localhost
-Error 1158 Got an error reading communication packets
- main.delayed
- Bug#54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED
This part is disabled for now as it fails randomly with different
warnings/errors (no corruption).
Example of what causes the problem:
T1: ANALYZE TABLE starts to collect statistics
T2: ALTER TABLE starts by deleting statistics for all changed fields,
then creates a temp table and copies data to it.
T1: ANALYZE ends and writes to the statistics tables.
T2: ALTER TABLE renames temp table in place of the old table.
Now the statistics from analyze matches the old deleted tables.
Fixed by waiting to delete old statistics until ALTER TABLE is
the only one using the old table and ensure that rename of columns
can handle swapping of column names.
rename_columns_in_stat_table() (former rename_column_in_stat_tables())
now takes a list of columns to rename. It uses the following algorithm
to update column_stats to be able to handle circular renames
- While there are columns to be renamed and it is the first loop or
last rename loop did change something.
- Loop over all columns to be renamed
- Change column name in column_stat
- If fail because of duplicate key
- If this is first change attempt for this column
- Change column name to a temporary column name
- If there was a conflicting row, replace it with the current row.
else
- Remove entry from column list
- Loop over all remaining columns in the list
- Remove the conflicting row
- Change column from temporary name to final name in column_stat
Other things:
- Don't flush tables for every operation. Only flush when all updates
are done.
- Rename of columns was not handled in case of ALGORITHM=copy (old bug).
- Fixed that we do not collect statistics for hidden hash columns
used by UNIQUE constraint on long values.
- Fixed that we do not collect statistics for blob columns referred by
generated virtual columns. This was achieved by storing the fields for
which we want to have statistics in table->has_value_set instead of
in table->read_set.
- Rename of indexes was not handled for persistent statistics.
- This is now handled similar as rename of columns. Renamed columns
are now stored in 'rename_stat_indexes' and handled in
Alter_info::delete_statistics() together with drooped indexes.
- ALTER TABLE .. ADD INDEX may instead of creating a new index rename
an existing generated foreign key index. This was not reflected in
the index_stats table because this was handled in
mysql_prepare_create_table instead instead of in the mysql_alter() code.
Fixed by adding a call in mysql_prepare_create_table() to drop the
changed index.
I also had to change the code that 'marked the index' to be ignored
with code that would not destroy the original index name.
Reviewer: Sergei Petrunia <sergey@mariadb.com>
At the moment we cannot support
wsrep_forced_binlog_format=[MIXED|STATEMENT]
during CREATE TABLE AS SELECT.
Statement will use ROW instead and give
a warning.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
Problem:
Under terms of MDEV-27490, we'll update Unicode version used
to compare identifiers to 14.0.0. Unlike in the old Unicode version,
in the new version a string can grow during lower-case. We cannot
perform check_db_name() inplace any more.
Change summary:
- Allocate memory to store lower-cased identifiers in memory root
- Removing check_db_name() performing both in-place lower-casing and validation
at the same time. Splitting it into two separate stages:
* creating a memory-root lower-cased copy of an identifier
(using new MEM_ROOT functions and Query_arena wrapper methods)
* performing validation on a constant string
(using Lex_ident_fs methods)
Implementation details:
- Adding a mysys helper function to allocate lower-cased strings on MEM_ROOT:
lex_string_casedn_root()
and a Query_arena wrappers for it:
make_ident_casedn()
make_ident_opt_casedn()
- Adding a Query_arena method to perform both MEM_ROOT lower-casing and
database name validation at the same time:
to_ident_db_internal_with_error()
This method is very close to the old (pre-11.3) check_db_name(),
but performs lower-casing to a newly allocated MEM_ROOT
memory (instead of performing lower-casing the original string in-place).
- Adding a Table_ident method which additionally handles derived table names:
to_ident_db_internal_with_error()
- Removing the old check_db_name()
Replacing my_casedn_str() called on local char[] buffer variables
to CharBuffer::copy_casedn() calls.
This is a sub-task for MDEV-31531 Remove my_casedn_str()
Details:
- Adding a helper template class IdentBuffer (a CharBuffer descendant),
which assumes utf8 data. Like CharBuffer, it's initialized to an empty
string in the constructor, but can be populated with lower-cased data
later.
- Adding a helper template class IdentBufferCasedn, which initializes
to lower case right in the constructor.
- Removing char[] buffers, replacing them to IdentBuffer and IdentBufferCasedn.
- Changing the data type of "db" and "table" parameters from
"const char*" to LEX_CSTRING in the following functions:
find_field_in_table_ref()
insert_fields()
set_thd_db()
mysql_grant()
to reuse IdentBuffer easeir.
- This commit is different from 10.6 commit c438284863.
Due to Commit 045757af4c (MDEV-24621),
InnoDB does buffer and pre-sort the records for each index, and build
the indexes one page at a time.
Multiple large insert ignore statment aborts the server during bulk
insert operation. Problem is that InnoDB merge record exceeds
the page size. To avoid this scenario, InnoDB should catch
too big record while buffering the insert operation itself.
row_merge_buf_encode(): returns length of the encoded index record
row_merge_buf_write(): Catches the DB_TOO_BIG_RECORD earlier and
returns error
- HA_EXTRA_IGNORE_INSERT call is being called for every inserted row,
and on partitioned tables on every row * every partition.
This leads to slowness during load..data operation
- Under bulk operation, multiple insert statement error handling
will end up emptying the table. This behaviour introduced by the
commit 8ea923f55b (MDEV-24818).
This makes the HA_EXTRA_IGNORE_INSERT call redundant. We can
use the same behavior for insert..ignore statement as well.
- Removed the extra call HA_EXTRA_IGNORE_INSERT as the solution
to improve the performance of load command.
... upon replicating online ALTER
When an online event is applied and slave_exec_mode is idempotent,
Write_rows_log_event::do_before_row_operations had reset
thd->lex->sql_command to SQLCOM_REPLACE.
This led to that a statement was detected as a row-type during binlogging,
and was logged as not standalone.
So the corresponding Gtid_log_event, when applied on replica, did not exit
early and created a new PSI transaction. Hence the difference with
non-online ALTER.
Adding an auto_increment column online leads to an undefined behavior.
Basically any DEFAULTs that depend on a row order in the table, or on
the non-deterministic (in scope of the ALTER TABLE statement) function
is UB.
For example, NOW() is considered generally non-deterministic
(Item_func_now_utc is marked with VCOL_NON_DETERMINISTIC), but it's fixed
in scope of a single statement.
Same for any other function that depends only on the session/status vars
apart from its arguments.
Only two UB cases are known:
* adding new AUTO_INCREMENT column. Modifying the existing column may be
fine under certain circumstances, see MDEV-31058.
* adding new column with DEFAULT(nextval(...)). Modifying the existing
column is possible, since its value will be always present in the online
event, except for the NULL -> NOT NULL modification
Add a new virtual function that will increase the inserted rows count
for the insert log event and decrease it for the delete event.
Reuses Rows_log_event::m_row_count on the replication side, which was only
set on the logging side.
The deadlock was caused by too strong MDL acquired by the start ALTER.
Replica's ALTER TABLE replication consists of two phases:
1. Start ALTER (SA) -- the event is emittd in the very beginning,
allowing replication start ALTER in parallel
2. Commit ALTER (CA) -- ensures that master finishes successfully
CA is normally received by wait_for_master call.
If parallel DML was run, the following sequence will take place:
|- SA
|- DML
|- CA
If CA is handled after MDL upgrade, it'll will deadlock with DML.
While MDL is shared by the start ALTER wait for its 2nd part
to allow concurrent DMLs to grab the lock.
The fix uses wait_for_master reentrancy -- no need to avoid a second call
in the end of mysql_alter_table.
Since SA and CA are marked with FL_DDL, the DML issued in-between cannot be
rescheduled before or after them. However, SA "commits" (by he call of
write_bin_log_start_alter and, subsequently,
thd->wakeup_subsequent_commits) before the copy stage begins, unlocking
the DMLs to run on this table. That is, these DMLs will be executed
concurrently with the copy stage, making Online alter effective on replicas
as well
Co-authored-by: Nikita Malyavin (nikitamalyavin@gmail.com)
1. Make online disk writes unlimited, same as filesort does.
2. Make proper error handling -- in 32-bit build IO_CACHE capacity limit is
4GB, so it is quite possible to overfill there.
3. Event_log::write_cache complicated with event reparsing, and as it was
proven by QA, contains some mistakes. Rewrite introbuce a simpler and much
faster version, not featuring reparsing and therefore copying a whole
buffer at once. This also disables checksums and crypto.
4. Handle read_log_event errors correctly: error returned is -1 (eof
signal for alter table), and my_error is not called. Call my_error and
always return 1. There's no test for this, since it shouldn't happen,
see the next bullet.
5. An event could be written partially in case of error, if it's bigger
than the IO_CACHE buffer. Restore the position where it was before the
error was emitted.
As a result, online alter is untied of several binlog variables, which was
a second aim of this patch.
