1. 17 Nov, 2023 11 commits
    • Kristian Nielsen's avatar
      MDEV-20523: rpl.create_or_replace_mix, rpl.create_or_replace_statement failed... · 36680b64
      Kristian Nielsen authored
      MDEV-20523: rpl.create_or_replace_mix, rpl.create_or_replace_statement failed in buildbot with wrong result
      
      Wait for the disconnect of the other connection to complete, before running
      SHOW BINLOG EVENTS. Otherwise the DROP TEMPORARY TABLE that is binlogged
      during disconnect may not have appeared yet depending on thread scheduling.
      Signed-off-by: default avatarKristian Nielsen <knielsen@knielsen-hq.org>
      36680b64
    • Kristian Nielsen's avatar
      MDEV-32168: slave_error_param condition is never checked from the wait_for_slave_param.inc · 0258ad54
      Kristian Nielsen authored
      Fix wrong change to rpl.rpl_shutdown_wait_slaves. After shutting down the
      master, slaves may or may not succeed in reconnecting depending on the timing
      on their reconnect relative to master restart. So don't assume all IO threads
      will be running, just restart any slave that needs it.
      Signed-off-by: default avatarKristian Nielsen <knielsen@knielsen-hq.org>
      0258ad54
    • Kristian Nielsen's avatar
      MDEV-32168: slave_error_param condition is never checked from the wait_for_slave_param.inc · 7e394d0b
      Kristian Nielsen authored
      Fix sporadic test failure in rpl.rpl_ssl1. The test incorrectly did a STOP
      SLAVE too early, which could race with the expected 'Access denied' error.
      Signed-off-by: default avatarKristian Nielsen <knielsen@knielsen-hq.org>
      7e394d0b
    • Kristian Nielsen's avatar
      MDEV-32168: slave_error_param condition is never checked from the wait_for_slave_param.inc · 30ec1b3e
      Kristian Nielsen authored
      Fix sporadic test failures in rpl.rpl_set_statement_default_master and
      rpl.rpl_slave_load_tmpdir_not_exist. A race between START and STOP
      SLAVE could leave an error condition that causes test failure after
      MDEV-32168.
      Signed-off-by: default avatarKristian Nielsen <knielsen@knielsen-hq.org>
      30ec1b3e
    • Kristian Nielsen's avatar
      MDEV-32168: slave_error_param condition is never checked from the wait_for_slave_param.inc · 17430d94
      Kristian Nielsen authored
      Test rpl.show_status_stop_slave_race-7126 now fails sporadically
      because it is expected to sometimes (but not always) leave an error
      condition after slave stop. Fix by explicitly allowing the error
      condition in this case.
      Signed-off-by: default avatarKristian Nielsen <knielsen@knielsen-hq.org>
      17430d94
    • Kristian Nielsen's avatar
      MDEV-32168: slave_error_param condition is never checked from the wait_for_slave_param.inc · d95fa7e3
      Kristian Nielsen authored
      Fix a start/stop race that causes occasional test failure after more the more
      strict error check of MDEV-32168.
      Signed-off-by: default avatarKristian Nielsen <knielsen@knielsen-hq.org>
      d95fa7e3
    • Brandon Nesterenko's avatar
      MDEV-32628: Cryptic ERROR message & inconsistent behavior on incorrect SHOW BINLOG EVENTS FROM ... · c42aadc3
      Brandon Nesterenko authored
      Calling SHOW BINLOG EVENTS FROM <offset> with an invalid offset
      writes error messages into the server log about invalid reads. The
      read errors that occur from this command should only be relayed back
      to the user though, and not written into the server log. This is
      because they are read-only and have no impact on server operation,
      and the client only need be informed to correct the parameter.
      
      This patch fixes this by omitting binary log read errors from the
      server when the invocation happens from SHOW BINLOG EVENTS.
      Additionally, redundant error messages are omitted when calling the
      string based read_log_event from the IO_Cache based read_log_event,
      as the later already will report the error of the former.
      
      Reviewed By:
      ============
      Kristian Nielsen <knielsen@knielsen-hq.org>
      Andrei Elkin <andrei.elkin@mariadb.com>
      c42aadc3
    • Marko Mäkelä's avatar
      MDEV-16660: Increase the DEFAULT_THREAD_STACK for ASAN · f5fdb9ce
      Marko Mäkelä authored
      To allow cmake -DWITH_ASAN=ON to work out of the box when using
      newer compilers, we must increase the default thread stack size.
      
      By design, AddressSanitizer will allocate some "sentinel" areas in
      stack frames so that it can better catch buffer overflows, by trapping
      access to memory addresses that reside between stack-allocated variables.
      
      Apparently, some parameters related to this have been changed
      recently, possibly to allow -fsanitize=address to catch more errors.
      f5fdb9ce
    • Yuchen Pei's avatar
      MDEV-30014 Spider should not second guess server when locking / unlocking · 03811978
      Yuchen Pei authored
      This fixes MDEV-30014, MDEV-29456, MDEV-29667, and MDEV-30049.
      
      The server may ask storage engines to unlock when the original sql
      command is not UNLOCK. This patch makes sure that spider honours these
      requests, so that the server has the correct idea which tables are
      locked and which are not.
      
      MDEV-29456, MDEV-29667, MDEV-30049: a later LOCK statement would, as
      the first step, unlock locked tables and clear the OPTION_TABLE_LOCK
      bit in thd->variables.option_bits, as well as locked_tables_list,
      indicating no tables are locked. If Spider does not unlock because the
      sql command is not UNLOCK, and if after this the LOCK statement fails
      to lock any tables, these indications that no tables are locked
      remains, so a later UNLOCK TABLES; statement would not try to unlock
      any table. Causing later statements requiring mdl locks to hang on
      waiting until lock_wait_timeout (default 1h) has passed.
      
      MDEV-30014: when a LOCK statement tries to lock more than one tables,
      say t2 and t3 as in mdev_30014.test, and t2 succeeds but t3 fails, the
      sql layer would try to undo by unlocking t2, and again, if Spider does
      not honour this request, the sql layer would assume t2 has been
      unlocked, but later actions on t2 or t2's remote table could hang on
      waiting for the mdl.
      03811978
    • Yuchen Pei's avatar
      MDEV-29963 MDEV-31357 Spider should clear its lock lists when locking fails · 52a5b16b
      Yuchen Pei authored
      Spider populates its lock lists (a hash) in store_lock(), and normally
      clears them in the actual lock_tables(). However, if lock_tables()
      fails, there's no reset_lock() method for storage engine handlers,
      which can cause bad things to happen. For example, if one of the table
      involved is dropped and recreated, or simply TRUNCATEd, when executing
      LOCK TABLES again, the lock lists would be queried again in
      store_lock(), which could cause access to freed space associated with
      the dropped table.
      52a5b16b
    • Yuchen Pei's avatar
      MDEV-26247 Re-implement spider gbh query rewrite of tables · 17839657
      Yuchen Pei authored
      Spider GBH's query rewrite of table joins is overly complex and
      error-prone. We replace it with something closer to what
      dbug_print() (more specifically, print_join()) does, but catered to
      spider.
      
      More specifically, we replace the body of
      spider_db_mbase_util::append_from_and_tables() with a call to
      spider_db_mbase_util::append_join(), and remove downstream append_X
      functions.
      
      We make it handle const tables by rewriting them as (select 1). This
      fixes the main issue in MDEV-26247.
      
      We also ban semijoin from spider gbh, which fixes MDEV-31645 and
      MDEV-30392, as semi-join is an "internal" join, and "semi join" does
      not parse, and it is different from "join" in that it deduplicates the
      right hand side
      
      Not all queries passed to a group by handler are valid (MDEV-32273),
      for example, a join on expr may refer outer fields not in the current
      context. We detect this during the handler creation when walking the
      join. See also gbh_outer_fields_in_join.test.
      
      It also skips eliminated tables, which fixes MDEV-26193.
      17839657
  2. 16 Nov, 2023 4 commits
    • Yuchen Pei's avatar
      0bacef76
    • Yuchen Pei's avatar
      MDEV-26247 Clean up spider_fields · 2d1e09a7
      Yuchen Pei authored
      Spider gbh query rewrite should get table for fields in a simple way.
      Add a method spider_fields::find_table that searches its table holders
      to find table for a given field. This way we will be able to get rid
      of the first pass during the gbh creation where field_chains and
      field_holders are created.
      
      We also check that the field belongs to a spider table while walking
      through the query, so we could remove
      all_query_fields_are_query_table_members(). However, this requires an
      earlier creation of the table_holder so that tables are added before
      checking. We do that, and in doing so, also decouple table_holder and
      spider_fields
      
      Remove unused methods and fields. Add comments.
      2d1e09a7
    • Yuchen Pei's avatar
      MDEV-26247 Remove some unused spider methods · 8c1dcb25
      Yuchen Pei authored
      Two methods from spider_fields. There are probably more of these
      conn_holder related methods that can be removed
      
      reappend_tables_part()
      reappend_tables()
      8c1dcb25
    • Anel Husakovic's avatar
      MDEV-32168: slave_error_param condition is never checked from the wait_for_slave_param.inc · a7d186a1
      Anel Husakovic authored
      - Reviewer: <knielsen@knielsen-hq.org>
                  <brandon.nesterenko@mariadb.com>
                  <andrei.elkin@mariadb.com>
      a7d186a1
  3. 15 Nov, 2023 4 commits
  4. 14 Nov, 2023 4 commits
    • Kristian Nielsen's avatar
      MDEV-11018: rpl.rpl_mariadb_slave_capability fails sporadically in buildbot · 73a38b68
      Kristian Nielsen authored
      The test was missing a wait_for_binlog_checkpoint.inc, making it non-deterministic
      Signed-off-by: default avatarKristian Nielsen <knielsen@knielsen-hq.org>
      73a38b68
    • Dmitry Shulga's avatar
      MDEV-32733: Two JSON related tests running in PS mode fail on server built... · 93bdb6db
      Dmitry Shulga authored
      MDEV-32733: Two JSON related tests running in PS mode fail on server built with -DWITH_PROTECT_STATEMENT_MEMROOT=YES
      
      The tests main.func_json and json.json_no_table fail on server built with
      the option -DWITH_PROTECT_STATEMENT_MEMROOT=YES by the reason that a memory
      is allocated on the statement's memory root on the second execution of
      a query that uses the function json_contains_path().
      
      The reason that a memory is allocated on second execution of a prepared
      statement that ivokes the function json_contains_path() is that a memory
      allocated on every call of the method Item_json_str_multipath::fix_fields
      
      To fix the issue, memory allocation should be done only once on first
      call of the method Item_json_str_multipath::fix_fields. Simmilar issue
      take place inside the method Item_func_json_contains_path::fix_fields.
      Both methods are modified to make memory allocation only once on its
      first execution and later re-use the allocated memory.
      
      Before this patch the memory referenced by the pointers stored in the array
      tmp_paths were released by the method Item_func_json_contains_path::cleanup
      that is called on finishing execution of a prepared statement. Now that
      memory allocation performed inside the method Item_json_str_multipath::fix_fields
      is done only once, the item clean up has degenerate form and can be
      delegated to the cleanup() method of the base class and memory deallocation
      can be performed in the destructor.
      93bdb6db
    • Daniel Black's avatar
      MDEV-32776 plugin disks getmntinfo64 deprecated on macOS · 5b9a7871
      Daniel Black authored
      The getmntinfo64 interface is deprected in MacOSX12.1.sdk.
      
      Using getmntinfo instead.
      
      Thanks heyingquay for reporting the bug and testing the fix.
      5b9a7871
    • Oleksandr Byelkin's avatar
      0f4df26b
  5. 13 Nov, 2023 2 commits
  6. 10 Nov, 2023 7 commits
    • Oleg Smirnov's avatar
      MDEV-29681 Server crashes when optimizing SQL with ORDER BY · 28cdbab1
      Oleg Smirnov authored
      When parsing statements like (SELECT .. FROM ..) ORDER BY <expr>,
      there is a step LEX::add_tail_to_query_expression_body_ext_parens()
      which calls LEX::wrap_unit_into_derived(). After that the statement
      looks like SELECT * FROM (SELECT .. FROM ..), and parser's
      Lex_order_limit_lock structure (ORDER BY <expr>) is assigned to
      the new SELECT. But what is missing here is that Items in
      Lex_order_limit_lock are left with their original name resolution
      contexts, and fix_fields() later resolves the names incorrectly.
      
      For example, when processing
        (SELECT * FROM t1 JOIN t2 ON a=b) ORDER BY a
      Item_field 'a' in the ORDER BY clause is left with the name resolution
      context of the derived table (first_name_resolution_table='t1'), so
      it is resolved to 't1.a', which is incorrect.
      After LEX::wrap_unit_into_derived() the statement looks like
        SELECT * FROM (SELECT * FROM t1 JOIN t2 ON a=b) AS '__2' ORDER BY a,
      and the name resolution context for Item_field 'a' in the ORDER BY
      must be set to the wrapping SELECT's one.
      
      This commit fixes the issue by changing context for Items in
      Lex_order_limit_lock after LEX::wrap_unit_into_derived().
      28cdbab1
    • Aleksey Midenkov's avatar
      MDEV-29932 Invalid expr in cleanup_session_expr() upon INSERT DELAYED · f7552313
      Aleksey Midenkov authored
      There are two TABLE objects in each thread: first one is created in
      delayed thread by Delayed_insert::open_and_lock_table(), second one is
      created in connection thread by Delayed_insert::get_local_table(). It
      is copied from the delayed thread table.
      
      When the second table is copied copy-assignment operator copies
      vcol_refix_list which is already filled with an item from delayed
      thread. Then get_local_table() adds its own item. Thus both tables
      contains the same list with two items which is wrong. Then connection
      thread finishes and its item freed. Then delayed thread tries to
      access it in vcol_cleanup_expr().
      
      The fix just clears vcol_refix_list in the copied table.
      
      Another problem is that copied table contains the same mem_root, any
      allocations on it will be invalid if the original table is freed (and
      that is indeterministic as it is done in another thread). Since copied
      table is allocated in connection THD and lives not longer than
      thd->mem_root we may assign its mem_root from thd->mem_root.
      
      Third, it doesn't make sense to do open_and_lock_tables() on NULL
      pointer.
      f7552313
    • Aleksey Midenkov's avatar
      MDEV-28127 EXCHANGE PARTITION with non-matching vcol expression segfault · 56e47910
      Aleksey Midenkov authored
      mysql_compare_tables() treated all columns non-virtual. Now it
      properly checks if the columns are virtual and matches expressions.
      56e47910
    • Aleksey Midenkov's avatar
      MDEV-23294 Segfault or assertion upon MyISAM repair · ebb6f575
      Aleksey Midenkov authored
      When computing vcol expression some items use current_thd and that was
      not set in MyISAM repair thread. Since all the repair threads belong
      to one connection and items should not write into THD we can utilize
      table THD for that.
      ebb6f575
    • Aleksey Midenkov's avatar
      MDEV-32082 Server crash in find_field_in_table · 74883f5e
      Aleksey Midenkov authored
      Attempt to resolve FOR SYSTEM_TIME expression as field for derived
      table is done before derived table is fully prepared, so we fail on
      assertion that table_list->table is missing.
      
      Actually Vers_history_point::resolve_unit() is done under the call of
      mysql_derived_prepare() itself (sql_derived.cc:824) and the table is
      assigned later at 867.
      
      The fix disables unit resolution for field type in FOR SYSTEM_TIME
      expression as it does a little sense in any case: making historical
      queries based on variable field values produces the result of multiple
      time points.
      
      fix_fields_if_needed() in resolve_units() was introduced by 46be3198
      74883f5e
    • Aleksey Midenkov's avatar
      MDEV-20545 Assertion col.vers_sys_end() in dict_index_t::vers_history_row · e53e7cd1
      Aleksey Midenkov authored
      Index values for row_start/row_end was wrongly calculated for inplace
      ALTER for some layout of virtual fields.
      
      Possible impact
      
        1. history row is not detected upon build clustered index for
           inplace ALTER which may lead to duplicate key errors on
           auto-increment and FTS index add.
        2. foreign key constraint may falsely fail.
        3. after inplace ALTER before server restart trx-based system
           versioning can cause server crash or incorrect data written upon
           UPDATE.
      e53e7cd1
    • Alexander Barkov's avatar
      MDEV-26743 InnoDB: CHAR+nopad does not work well · 1710b645
      Alexander Barkov authored
      The patch for "MDEV-25440: Indexed CHAR ... broken with NO_PAD collations"
      fixed these scenarios from MDEV-26743:
      - Basic latin letter vs equal accented letter
      - Two letters vs equal (but space padded) expansion
      
      However, this scenario was still broken:
      - Basic latin letter (but followed by an ignorable character)
        vs equal accented letter
      
      Fix:
      When processing for a NOPAD collation a string with trailing ignorable
      characters, like:
        '<non-ignorable><ignorable><ignorable>'
      
      the string gets virtually converted to:
        '<non-ignorable><ignorable><ignorable><space><space><space>...'
      
      After the fix the code works differently in these two cases:
      1. <space> fits into the "nchars" limit
      2. <space> does not fit into the "nchars" limit
      
      Details:
      
      1. If "nchars" is large enough (4+ in this example),
         return weights as follows:
      
        '[weight-for-non-ignorable, 1 char] [weight-for-space-character, 3 chars]'
      
        i.e. the weight for the virtual trailing space character now indicates
        that it corresponds to total 3 characters:
        - two ignorable characters
        - one virtual trailing space character
      
      2. If "nchars" is small (3), then the virtual trailing space character
         does not fit into the "nchar" limit, so return 0x00 as weight, e.g.:
      
        '[weight-for-non-ignorable, 1 char] [0x00, 2 chars]'
      
      Adding corresponding MTR tests and unit tests.
      1710b645
  7. 09 Nov, 2023 2 commits
    • Andrei's avatar
      d6872f9c
    • Alexander Barkov's avatar
      MDEV-29110 mariabackup has wrong or missing plugin-dir default? · 62d80652
      Alexander Barkov authored
      Problem:
      
      The file backup-my.cnf from the backup directory was loaded by
      "mariabackup --prepare" only in case of the explicit --target-dir given.
      It was not loaded from the default directory ./xtrabackup_backupfiles/
      in case if the explicit --target-dir was missing.
      
      In other words, it worked as follows:
      
      1. When started as "mariabackup --prepare --target-dir=DIR", mariabackup:
        a. loads defaults from "DIR/backup-my.cnf"
        b. processes data files in the specified directory DIR
      
      2. When started as "mariabackup --prepare", mariabackup:
        a. does not load defaults from "./xtrabackup_backupfiles/backup-my.cnf"
        b. processes data files in the default directory "./xtrabackup_backupfiles/"
      
      This patch fixes the second scenario, so it works as follows:
      
      2. When started as "mariabackup --prepare", mariabackup:
        a. loads defaults from "./xtrabackup_backupfiles/backup-my.cnf"
        b. processes data files in the default directory "./xtrabackup_backupfiles/"
      
      This change fixes (among others) the problem with the
      "Can't open shared library '/file_key_management.so'" error
      reported when "mariabackup --prepare" is used without --target-dir
      in combinaton with the encryption plugin.
      62d80652
  8. 08 Nov, 2023 2 commits
    • Alexander Barkov's avatar
      MDEV-27744 LPAD in vcol created in ORACLE mode makes table corrupted in non-ORACLE · 2b6d241e
      Alexander Barkov authored
      The crash happened with an indexed virtual column whose
      value is evaluated using a function that has a different meaning
      in sql_mode='' vs sql_mode=ORACLE:
      
      - DECODE()
      - LTRIM()
      - RTRIM()
      - LPAD()
      - RPAD()
      - REPLACE()
      - SUBSTR()
      
      For example:
      
      CREATE TABLE t1 (
        b VARCHAR(1),
        g CHAR(1) GENERATED ALWAYS AS (SUBSTR(b,0,0)) VIRTUAL,
        KEY g(g)
      );
      
      So far we had replacement XXX_ORACLE() functions for all mentioned function,
      e.g. SUBSTR_ORACLE() for SUBSTR(). So it was possible to correctly re-parse
      SUBSTR_ORACLE() even in sql_mode=''.
      
      But it was not possible to re-parse the MariaDB version of SUBSTR()
      after switching to sql_mode=ORACLE. It was erroneously mis-interpreted
      as SUBSTR_ORACLE().
      
      As a result, this combination worked fine:
      
      SET sql_mode=ORACLE;
      CREATE TABLE t1 ... g CHAR(1) GENERATED ALWAYS AS (SUBSTR(b,0,0)) VIRTUAL, ...;
      INSERT ...
      FLUSH TABLES;
      SET sql_mode='';
      INSERT ...
      
      But the other way around it crashed:
      
      SET sql_mode='';
      CREATE TABLE t1 ... g CHAR(1) GENERATED ALWAYS AS (SUBSTR(b,0,0)) VIRTUAL, ...;
      INSERT ...
      FLUSH TABLES;
      SET sql_mode=ORACLE;
      INSERT ...
      
      At CREATE time, SUBSTR was instantiated as Item_func_substr and printed
      in the FRM file as substr(). At re-open time with sql_mode=ORACLE, "substr()"
      was erroneously instantiated as Item_func_substr_oracle.
      
      Fix:
      
      The fix proposes a symmetric solution. It provides a way to re-parse reliably
      all sql_mode dependent functions to their original CREATE TABLE time meaning,
      no matter what the open-time sql_mode is.
      
      We take advantage of the same idea we previously used to resolve sql_mode
      dependent data types.
      
      Now all sql_mode dependent functions are printed by SHOW using a schema
      qualifier when the current sql_mode differs from the function sql_mode:
      
      SET sql_mode='';
      CREATE TABLE t1 ... SUBSTR(a,b,c) ..;
      SET sql_mode=ORACLE;
      SHOW CREATE TABLE t1;   ->   mariadb_schema.substr(a,b,c)
      
      SET sql_mode=ORACLE;
      CREATE TABLE t2 ... SUBSTR(a,b,c) ..;
      SET sql_mode='';
      SHOW CREATE TABLE t1;   ->   oracle_schema.substr(a,b,c)
      
      Old replacement names like substr_oracle() are still understood for
      backward compatibility and used in FRM files (for downgrade compatibility),
      but they are not printed by SHOW any more.
      2b6d241e
    • Marko Mäkelä's avatar
      MDEV-13626 Merge InnoDB test cases from MySQL 5.7 · 228b7e4d
      Marko Mäkelä authored
      This imports and adapts a number of MySQL 5.7 test cases that are
      applicable to MariaDB.
      
      Some tests for old bug fixes are not that relevant because the code
      has been refactored since then (especially starting with
      MariaDB Server 10.6), and the tests would not reproduce the
      original bug if the fix was reverted.
      
      In the test innodb_fts.opt, there are many duplicate MATCH ranks, which
      would make the results nondeterministic. The test was stabilized by
      changing some LIMIT clauses or by adding sorted_result in those cases
      where the purpose of a test was to show that no sorting took place
      in the server.
      
      In the test innodb_fts.phrase, MySQL 5.7 would generate FTS_DOC_ID that
      are 1 larger than in MariaDB. In innodb_fts.index_table the difference is 2.
      This is because in MariaDB, fts_get_next_doc_id() post-increments
      cache->next_doc_id, while MySQL 5.7 pre-increments it.
      
      Reviewed by: Thirunarayanan Balathandayuthapani
      228b7e4d
  9. 07 Nov, 2023 1 commit
    • Monty's avatar
      Ensure that process "State" is properly cleaned after query execution · 2447172a
      Monty authored
      In some cases "SHOW PROCESSLIST" could show "Reset for next command"
      as State, even if the previous query had finished properly.
      
      Fixed by clearing State after end of command and also setting the State
      for the "Connect" command.
      
      Other things:
      - Changed usage of 'thd->set_command(COM_SLEEP)' to
        'thd->mark_connection_idle()'.
      - Changed thread_state_info() to return "" instead of NULL. This is
        just a safety measurement and in line with the logic of the
        rest of the function.
      2447172a
  10. 06 Nov, 2023 2 commits
  11. 05 Nov, 2023 1 commit