1. 29 May, 2023 12 commits
    • Nikita Malyavin's avatar
      MDEV-30984 Online ALTER table is denied with non-informative error messages · 97e8aeff
      Nikita Malyavin authored
      Group all the checks in online_alter_check_supported().
      
      There is now two groups of checks:
      1. A technical availability of online, that is checked before open_tables,
      and affects table_list->lock_type. It's supposed to be safe to make it
      TL_READ even if COPY algorithm will fall back to not-online, since MDL is
      SHARED_UPGRADEABLE anyway.
      2. An 'online' availability for a COPY algorithm. It can be done as late as
      just before the copy_data_between_tables call. The lock_type influence is
      disclosed above, so the only other place it affects is
      Alter_info::supports_lock, where `online` flag is only used to decide
      whether to report the error at the inplace preparation stage. We'd want to
      make that at the last resort, which is COPY preparation, if no algorithm is
      chosen by the user. So it's event better now.
      
      Some changes are required to the autoinc support detection, as the check
      now happens after mysql_prepare_alter_table:
      * alter_info->drop_list is empty
      * instead, dropped columns are in tmp_set
      * alter_info->create_list now has every field that's in the new table.
      * the column definition's change.str will be nonnull whether the column
        remains in the new table (vs whether it was changed, as before).
        But it also has `field` field set.
      * IF EXISTS doesn't have to be dealt anymore
      
      This infers that the changes are now checked in more detail: a field's
      definition shouldn't be changed, vs a field shouldn't be mentioned in
      the CHANGE list, as it was before. This is reflected by the line 193 test.
      97e8aeff
    • Nikita Malyavin's avatar
    • Nikita Malyavin's avatar
      Add const to alloc-related thd methods · 4e2deb34
      Nikita Malyavin authored
      Also update abi declarations. The abi itself is unchanged, since const
      doesn't affect C-style exported name.
      4e2deb34
    • Nikita Malyavin's avatar
      MDEV-30987 main.alter_table_online times out with view-protocol · 75cf3b6a
      Nikita Malyavin authored
      disabie view-protocol for certain places:
      1. While a transaction is in progress: a view gets locked for a
      transaction duration, so DROP VIEW from a servicing conneciton
      deadlocks. We can't just disable servicing connection, as
      CREATE VIEW and DROP VIEW commit implicitly.
      2. For querying Opened_table_definitions: the actual query executed
      is `SELECT * FROM mysqltest_tmp_v`, then the view is dropped
      75cf3b6a
    • Nikita Malyavin's avatar
      MDEV-31059 "Slave SQL" errors upon concurrent DML and erroneous ALTER · 611bbcac
      Nikita Malyavin authored
      Skip more rpl-related error handling.
      Also move the error check inside if (table) -- otherwise the error
      should be handled already.
      611bbcac
    • Nikita Malyavin's avatar
      refactor unpack_row · 2c671c15
      Nikita Malyavin authored
      Online alter and replication paths differ quite noticeably. It's time to
      fix the mess. Online alter has to unpack the old table's data
      (here conv_table), and then convert, while replication requires more
      sophisticated track: handle possible conversions, treat extra replica
      fields, and carefully deduce master's record length.
      
      * Extract unpack_field.
      * handle the unpacking progress through a state machine presented by
        Unpack_record_state struct.
      * Stick most of the online alter unpacking together under a single
        conditional branch.
      2c671c15
    • Nikita Malyavin's avatar
      77d9294e
    • Nikita Malyavin's avatar
      MDEV-31058 ER_KEY_NOT_FOUND upon concurrent CHANGE column autoinc and DML · 6085252c
      Nikita Malyavin authored
      When column is changed to autoinc, ALTER TABLE may update zero/NULL values,
      if NO_AUTO_VALUE_ON_ZERO mode is not enabled.
      
      Forbid this for LOCK=NONE for the unreliable cases.
      The cases are described in online_alter_check_autoinc.
      6085252c
    • Nikita Malyavin's avatar
    • Nikita Malyavin's avatar
      MDEV-30949 Direct leak in binlog_online_alter_end_trans · 706aff97
      Nikita Malyavin authored
      when committing a big transaction, online_alter_cache_log creates a cache
      file. It wasn't properly closed, which was spotted by a memory leak from
      my_register_filename. A temporary file also remained open.
      
      Binlog wasn't affected by this, since it features its own file management.
      
      A proper closing is calling close_cached_file. It deinits io_cache and
      closes the underlying file. After closing, the file is expected to be
      deleted automagically.
      706aff97
    • Nikita Malyavin's avatar
      MDEV-31043 ER_KEY_NOT_FOUND upon concurrent ALTER and transaction · 4c6c187f
      Nikita Malyavin authored
      Non-transactional engines changes are visible immediately the row operation
      succeeds, so we should apply the changes to the online buffer immediately
      after the statement ends.
      4c6c187f
    • Nikita Malyavin's avatar
      MDEV-31033 ER_KEY_NOT_FOUND upon online COPY ALTER on a partitioned table · 0bcbec79
      Nikita Malyavin authored
      The row events were applied "twice": once for the ha_partition, and one
      more time for the underlying storage engine.
      
      There's no such problem in binlog/rpl, because ha_partiton::row_logging
      is normally set to false.
      
      The fix makes the events replicate only when the handler is a root handler.
      We will try to *guess* this by comparing it to table->file. The same
      approach is used in the MDEV-21540 fix, 231feabd. The assumption is made,
      that the row methods are only called for table->file (and never for a
      cloned handler), hence the assertions are added in ha_innobase and
      ha_myisam to make sure that this is true at least for those engines
      
      Also closes MDEV-31040, however the test is not included, since we have no
      convenient way to construct a deterministic version.
      0bcbec79
  2. 05 May, 2023 15 commits
    • Nikita Malyavin's avatar
      MDEV-30945 RPL tests are failing with MSAN use-of-uninitialized-value · 47f5c7ae
      Nikita Malyavin authored
      ...in bitmap_intersect
      
      m_cols_ai was accessed during the Delete event, however this field is only
      related to Updates.
      
      Moving it to Update_rows_event would require too much effort. So instead:
      * Only access m_cols_ai in Update events (conditional branch is added in
        Rows_log_event::do_add_row_data)
      * Clean up m_cols_ai operations in Rows_log_event constructor.
        m_cols_ai.bitmap is first set to NULL, indicating invalid event.
        Then it is initialized:
        -> For Update events, a new bitmap is created.
        -> For other events, debug mode, m_cols_ai.bitmap is set to 1, indicating
           that the value is correct, but it shouldn't be accessed. To make sure
           we'll have a failure, n_bits is also set to 1.
        -> In release mode, m_cols_ai mirrors m_cols, providing extra safety
           in production.
      47f5c7ae
    • Nikita Malyavin's avatar
      clean up Rows_log_event virtual methods · a50bcc2a
      Nikita Malyavin authored
      Modernize declaration to C++11.
      Add missing const.
      Convert TYPE_CODE to constexpr.
      a50bcc2a
    • Nikita Malyavin's avatar
      MDEV-30891 Assertion `!table->versioned(VERS_TRX_ID)' failed · 761a2638
      Nikita Malyavin authored
      Assertion `!table->versioned(VERS_TRX_ID)' failed in
      Write_rows_log_event::binlog_row_logging_function during ONLINE ALTER.
      
      trxid-versioned tables can't be replicated.
      ONLINE ALTER will also be forbidden for these tables.
      761a2638
    • Nikita Malyavin's avatar
      add partition test · 66c3bbb2
      Nikita Malyavin authored
      66c3bbb2
    • Nikita Malyavin's avatar
      MDEV-30985 Replica stops with error on ALTER ONLINE with Geometry Types · ae43b81d
      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.
      ae43b81d
    • Nikita Malyavin's avatar
      MDEV-30924 Server crashes in MYSQL_LOG::is_open upon ALTER vs FUNCTION · 667d0eaf
      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.
      667d0eaf
    • Nikita Malyavin's avatar
      MDEV-30925 Assertion failed in translog_write_record in ONLINE ALTER + Aria · 20c104e0
      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.
      20c104e0
    • Nikita Malyavin's avatar
      MDEV-30902 Server crash in LEX::first_lists_tables_same · 843872f1
      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.
      843872f1
    • Nikita Malyavin's avatar
      fix Run-Time Check Failure on Windows · e6b13455
      Nikita Malyavin authored
      e6b13455
    • Nikita Malyavin's avatar
      fix build · 9bca4c0f
      Nikita Malyavin authored
      9bca4c0f
    • Nikita Malyavin's avatar
    • Nikita Malyavin's avatar
    • Nikita Malyavin's avatar
      MDEV-29069 follow-up: improve DEFAULT rules · 5343d321
      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.
      5343d321
    • Nikita Malyavin's avatar
      c8308952
    • Nikita Malyavin's avatar
      6dc017b8
  3. 17 Apr, 2023 13 commits