- 15 Aug, 2023 40 commits
-
-
Nikita Malyavin authored
The bug is inherent for row-based replication as well. To reproduce, a virtual (not stored) field of a blob type computed from another field of a different blob type is required. The following happens during an update or delete row event: 1. A row is unpacked. 2. Virtual fields are updated. Field b1 stores the pointer in Field_blob::value and references it in table->record[0]. 3. record[0] is stored to record[1] in Rows_log_event::find_row. 4. A new record is fetched from handler. (e.g. ha_rnd_next) 5. Virtual columns are updated (only non-stored). 6. Field b1 receives new value. Old value is deallocated (Field_blob::val_str). 7. record_compare is called. record[0] and record[1] are compared. 8. record[1] contains a reference to a freed value. record_compare is used in replication to find a matching record for update or delete. Virtual columns that are not stored should be definitely skipped both for correctness, and for this bug fix. STORED virtual columns, on the other hand, may be required and shouldn't be skipped. Stored columns are not affected, since they are not updated after handler's fetch.
-
Nikita Malyavin authored
ASAN showed use-after-free in binlog_online_alter_end_trans, during running through thd->online_alter_cache_list. In online_alter_binlog_get_cache_data, new_cache_data was allocated on thd->mem_root, in case of autocommit=1, but this mem_root could be freed in sp_head::execute, upon using stored functions. It appears that thd->transaction->mem_root exists even in single-stmt transaction mode (i.e autocommit=1), so it can be used in all cases. This mem_root will remain valid till the end of transaction, including commit phase.
-
Nikita Malyavin authored
This is the corner case of ONLINE ALTER vs ha_maria vs App-time Periods. When a Delete_rows_event (or update) is executed, a lookup handler may be created, normally to serve long unique index needs, by a call of handler::prepare_for_insert. This function also creates a lookup handler if an application-time period exists in a table. A difference with a usual call of prepare_for_insert is that transactions are disabled for this table during ALTER TABLE. See mysql_trans_prepare_alter_copy_data call in copy_data_between_tables. Then, ha_maria calls _ma_tmp_disable_logging_for_table during ha_maria::external_lock. It never happened so before, that two handlers would be created for write to a single ha_maria table under transactions disabled. Hence, the fix handles this scenario. It could be done otherwise, by not creating this lookup handler (since it's not used anyway during ONLINE ALTER), but architecturally, two handlers should be supported. Avoiding the creation of lookup handler could be done here additionally, but with a cost of slowing down other more generic cases, with an additional check of online alter table active.
-
Nikita Malyavin authored
ONLINE ALTER TABLE uses binlog events like the replication does. Before it was never used outside of replication, so significant change was required. For example, a single event had a statement-like befavior: it locked the tables, opened it, and closed them in the end. But for ONLINE ALTER we use preopened table. A crash scenario is following: lex->query_tables was set to NULL in restore_empty_query_table_list when alter event is applied. Then lex->query_tables->prev_global was write-accessed in LEX::first_lists_tables_same, leading to a segfault. In replication restore_empty_query_table_list would mean resetting lex before next query or event. In ONLINE ALTER TABLE we reuse a locked table between the events, so we should avoid it. Here the need to reset lex state (or close the tables) can be determined by nonzero rgi->tables_to_lock_count. If no table is locked, then event doesn't own the tables. The same was already done before for rgi->slave_close_thread_tables call.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
-
Nikita Malyavin authored
previously, fields with DEFAULTs were allowed just when expression is deterministic. In case of online alter, we should recursively check that underlying fields of expression also either have explicit values, or have DEFAULT following this validity rule.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
-
Nikita Malyavin authored
We can't rely on keys formed with columns that were added during this ALTER. These columns can be set with non-deterministic values, which can end up with broken or incorrect search. The same applies to the keys that contain reliable columns, but also have bogus ones. Using them can narrow the search, but they're also ignored. Also, added columns shouldn't be considered during the record match. To determine them, table->has_value_set bitmap is used. To fill has_value_set bitmap in the find_key call, extra unpack_row call has been added. For replication case, extra replica columns can be considered for this case. We try to ignore them, too.
-
Sergei Golubchik authored
-
Sergei Golubchik authored
-
Sergei Golubchik authored
use table->pos_in_table_list instead. Also, table->in_use is always set
-
Sergei Golubchik authored
-
Nikita Malyavin authored
-
Nikita Malyavin authored
-
Nikita Malyavin authored
ONLINE ALTER TABLE adds binlog handlerton into ha_list, so any rollback command can end up calling binlog_rollback having no cache_mngr, if binlog is not enabled. The assertion should be fixed in the same manner as DBUG_ASSERT(WSREP(thd))
-
Nikita Malyavin authored
-
Nikita Malyavin authored
1. ER_KEY_NOT_FOUND general replcation problem, already fixed earlier. test added. 2. ER_LOCK_WAIT_TIMEOUT This is a long unique specific problem. Sometimes, lookup_handler is created for to->file. To properly free it, ha_reset should be called. It is usually done by calling close_thread_table, but ALTER TABLE makes it differently. Hence, a single ha_reset call is added to mysql_alter_table. Also, event_mem_root is removed. Normally, no per-event data should be allocated on thd->mem_root, that would mean a leak. And otherwise, lookup_handler is lazily allocated, but its lifetime matches statement, not event.
-
Sergei Golubchik authored
* in online ALTER it must include the complete new row, note that an UPDATE should set all extra columns to their default values, as if UPDATE was completely done before the ALTER. * in rpl WRITE_ROWS_EVENT it must include all extra slave columns, but not existing columns unmarked in the m_cols (sequences do that) * in rpl UPDATE/DELETE events it should follow m_cols_ai also: default values must be updated for WRITE_ROWS_EVENT and for UPDATE/DELETE in the online ALTER mode, see above. Update the result file accordingly. Extend bitmap_copy() to support arguments of different lengths
-
Sergei Golubchik authored
MDEV-28816 Assertion `wsrep_thd_is_applying(thd)' failed in int wsrep_ignored_error_code(Log_event*, int) wsrep expected that any Rows_log_event::do_apply_event() failures can only happen in the applier thread, because no other thread ever applies row events. With online alter it's no longer true.
-
Nikita Malyavin authored
-
Sergei Golubchik authored
-
Sergei Golubchik authored
because online means we'll apply events from the binlog, and ignore means that bad rows will be skipped. So a bad Write_row_log_event will be skipped and a following Update_row_log_event will fail to apply.
-
Nikita Malyavin authored
This test case was also fixed by adding update_virtual_columns.
-
Nikita Malyavin authored
Division by zero is a good example. sql_mode is basically ignored by replication, see Bug#56662. The behavior for ONLINE should remain the same as for non-ONLINE ALTER.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
-
Sergei Golubchik authored
so that table->update_default_fields() would know what to update
-
Nikita Malyavin authored
We shouldn't rely on `fill_extra_persistent_columns`, as it only updates fields which have an index > cols->n_bits (replication bitmap width). Actually, it should never be used, as its approach is error-prone. Normal update_virtual_fields+update_default_fields should be done.
-
Sergei Golubchik authored
-
Sergei Golubchik authored
if ALTER TABLE ... LOCK=xxx is executed under LOCK TABLES, ignore the LOCK clause, because ALTER should not downgrade already taken EXCLUSIVE table lock to SHARED or NONE. This commit preserves the existing behavior (LOCK was de facto ignored), but makes it explicit.
-
Nikita Malyavin authored
ALTER ONLINE TABLE acquires table with TL_READ. Myisam normally acquires TL_WRITE for DML, which makes it hang until table is freed. We deadlock once ALTER upgrades its MDL lock. Solution: Unlock table earlier. We don't need to hold TL_READ once we finished copying. Relay log replication requires no data locks on `from` table.
-
Sergei Golubchik authored
MDEV-28967 Assertion `marked_for_write_or_computed()' failed in Field_new_decimal::store_value / online_alter_read_from_binlog` in the catch-up phase of the online alter we apply row events, they're unpacked into `from->record[0]` and then converted to `to->record[0]`. This needs all fields of `from` to be in the `write_set`. Although practically `Field::unpack()` does not assert the `write_set`, and `Field::reset()` - used when a field value is not present in the after-image - also doesn't assert the `write_set` for many types, `Field_new_decimal::reset()` does.
-
Sergei Golubchik authored
in RBR - only show warnings for values that are to be written into a table, that is, only for the after-image. Don't show data conversion warnings for the before-image.
-
Sergei Golubchik authored
-
Sergei Golubchik authored
* don't disable warnings when catching up * do propagate warnings up like copy_data_between_tables() does
-
Sergei Golubchik authored
-
Nikita Malyavin authored
If online alter fails, TABLE_SHARE can be freed while concurrent transactions still have row events in their online_alter_cache_data. On commit they try'll to flush them, writing to TABLE_SHARE's Cache_flip_event_log, which is already freed. This causes a crash in main.alter_table_online_debug test
-
Sergei Golubchik authored
don't simply set tdc->flushed, use flush_unused(1) that removes opened but unused TABLE instances (that would otherwise prevent TABLE_SHARE from being closed by keeping the ref_count>0).
-