1. 07 Aug, 2024 1 commit
  2. 02 Aug, 2024 1 commit
  3. 30 Jul, 2024 2 commits
    • Hugo Wen's avatar
      MDEV-34625 Fix undefined behavior of using uninitialized member variables · 811614d4
      Hugo Wen authored
      Commit a8a75ba2 causes the MariaDB server to crash, usually with signal
      11, at random code locations due to invalid pointer values during any
      table operation. This issue occurs when the server is built with -O3 and
      other customized compiler flags.
      
      For example, the command `use db1;` causes server to crash in the
      `check_table_access` function at line sql_parse.cc:7080 because
      `tables->correspondent_table` is an invalid pointer value of 0x1.
      
      The crashes are due to undefined behavior from using uninitialized
      variables. The problematic commit a8a75ba2 introduces code that
      allocates memory and sets it to 0 using thd->calloc before initializing
      it with a placement new operation.
      This process depends on setting memory to 0 to initialize member
      variables not explicitly set in the constructor. However, the compiler
      can optimize out the memset/bfill, leading to uninitialized values and
      unpredictable issues.
      
      Once a constructor function initializes an object, any uninitialized
      variables within that object are subject to undefined behavior. The
      state of memory before the constructor runs, whether it involves
      memset or was used for other purposes, is irrelevant after the
      placement new operation.
      
      This behavior can be demonstrated with this
      [test](https://gcc.godbolt.org/z/5n87z1raG) I wrote to examine the
      assembly code. The code in MariaDB can be abstracted to the following,
      though it has many layers wrapped around it and more complex logic,
      causing slight differences in optimization in the MariaDB build.
      To summarize, on x86, the memset in the following code is optimized out
      with both -O2 and -O3 in GCC 13, and is only preserved in the much older
      GCC 4.9.
      
          struct S {
            int i;     // uninitialized in consturctor
            S() {};
          };
          int bar() {
            void *buf = malloc(sizeof(S));
            memset(buf, 0, sizeof(S));       // optimized out
            S* s = new(buf) S;
            return s->i;
          }
      
      With GCC13 -O3:
      
          bar():
                sub     rsp, 8
                mov     edi, 4
                call    malloc
                mov     eax, DWORD PTR [rax]
                add     rsp, 8
                ret
      
      With GCC4.9 -O3
      
          bar():
                sub     rsp, 8
                mov     edi, 4
                call    malloc
                mov     DWORD PTR [rax], 0
                xor     eax, eax
                add     rsp, 8
                ret
      
      Now we ensure the constructor initializes variables correctly by running
      the reset() function in the constructor to perform the memset/bfill(0)
      operation. After applying the fix, the crash is gone.
      
      All new code of the whole pull request, including one or several files
      that are either new files or modified ones, are contributed under the
      BSD-new license. I am contributing on behalf of my employer Amazon Web
      Services.
      811614d4
    • Thirunarayanan Balathandayuthapani's avatar
      MDEV-34357 InnoDB: Assertion failure in file ./storage/innobase/page/page0zip.cc line 4211 · ee5f7692
      Thirunarayanan Balathandayuthapani authored
      During InnoDB root page split, InnoDB does the following
      1) First move the root records to the new page(p1)
      2) Empty the root, insert the node pointer to the root page
      3) Split the new page and make it as child nodes.
      4) Finds the split record, allocate another new page(p2)
      to the index
      5) InnoDB stores the record(ret) predecessor to the supremum
      record of the page (p2).
      6) In page_copy_rec_list_start(), move the records from p1 to p2
      upto the split record
      6) Given table is a compressed row format page, InnoDB attempts to
      compress the page p2 and failed (due to innodb_compression_level = 0)
      7) Since the compression fails, InnoDB gets the number of preceding
      records(ret_pos) of a record (ret) on the page (p2)
      8) Page (p2) is a new page, ret points to infimum record.
      ret_pos can be 0. InnoDB have wrong condition that ret_pos shouldn't
      be 0 and returns corruption. InnoDB has similar wrong check in
      page_copy_rec_list_end()
      ee5f7692
  4. 29 Jul, 2024 1 commit
    • Monty's avatar
      MDEV-34664: Add an option to fix InnoDB's doubling of secondary index cardinalities · 4bf7c966
      Monty authored
      (With trivial fixes by sergey@mariadb.com)
      Added option fix_innodb_cardinality to optimizer_adjust_secondary_key_costs
      
      Using fix_innodb_cardinality disables the 'divide by 2' of rec_per_key_int
      in InnoDB that in effect doubles the Cardinality for secondary keys.
      This has the biggest effect for indexes where a few rows has the same key
      value. Using this may also cause table scans for very small tables (which
      in some cases may be better than an index scan).
      
      The user visible effect is that 'SHOW INDEX FROM table_name' will for
      InnoDB show the true Cardinality (and not 2x the real value). It will
      also allow the optimizer to chose a better index in some cases as the
      division by 2 could have a bad effect for tables with 2-5 identical values
      per key.
      
      A few notes about using fix_innodb_cardinality:
      - It has direct affect for SHOW INDEX FROM table_name. SHOW INDEX
        will also update the statistics in table share.
      - The effect of fix_innodb_cardinality for query plans or EXPLAIN
        is only visible after first open of the table. This is why one must
        do a flush tables or use SHOW INDEX for the option to take effect.
      - Using fix_innodb_cardinality can thus affect all user in their query
        plans if they are using the same tables.
      
      Because of this, it is strongly recommended that one uses
      optimizer_adjust_secondary_key_costs=fix_innodb_cardinality mainly
      in configuration files to not cause issues for other users.
      4bf7c966
  5. 23 Jul, 2024 1 commit
  6. 22 Jul, 2024 1 commit
  7. 20 Jul, 2024 1 commit
  8. 19 Jul, 2024 3 commits
    • Andrei's avatar
      MDEV-15393 gtid_slave_pos duplicate key errors after mysqldump restore · b8f92ade
      Andrei authored
      When mysqldump is run to dump the `mysql` system database, it generates
      INSERT statements into the table `mysql.gtid_slave_pos`.
      After running the backup script
      those inserts did not produce the expected gtid state on slave. In
      particular the maximum of mysql.gtid_slave_pos.sub_id did not make
      into
         rpl_global_gtid_slave_state.last_sub_id
      
      an in-memory object that is supposed to match the current state of the
      table. And that was regardless of whether --gtid option was specified
      or not. Later when the backup recipient server starts as slave
      in *non-gtid* mode this desychronization may lead to a duplicate key
      error.
      
      This effect is corrected for --gtid mode mysqldump/mariadb-dump only
      as the following.  The fixes ensure the insert block of the dump
      script is followed with a "summing-up" SET @global.gtid_slave_pos
      assignment.
      
      For the implemenation part, note a deferred print-out of
      SET-gtid_slave_pos and associated comments is prefered over relocating
      of the entire blocks if (opt_master,slave_data &&
      do_show_master,slave_status) ...  because of compatiblity
      concern. Namely an error inside do_show_*() is handled in the new code
      the same way, as early as, as before.
      
      A regression test can be run in how-to-reproduce mode as well.
      One affected mtr test observed.
      rpl_mysqldump_slave.result "mismatch" shows now the new deferring print
      of SET-gtid_slave_pos policy in action.
      b8f92ade
    • Oleksandr Byelkin's avatar
      New CC 3.3 · a94fd874
      Oleksandr Byelkin authored
      a94fd874
    • Oleksandr Byelkin's avatar
      Fix view protocol · b8b6cab2
      Oleksandr Byelkin authored
      b8b6cab2
  9. 18 Jul, 2024 2 commits
  10. 17 Jul, 2024 21 commits
    • Brandon Nesterenko's avatar
      MDEV-33921: Fix rpl_xa_empty_transaction.test · a061ae10
      Brandon Nesterenko authored
      The test was missing a save_master_gtid.inc on the master,
      leading to the slave thinking it was in sync after executing
      sync_with_master_gtid.inc, despite not having executed the
      latest transaction. This skipped transaction, XA COMMIT,
      was supposed to error-to-be-ignored because its XID could not
      be found, but be thrown out because the replication filters
      would filter out the target database. However, if the slave
      was able to stop before executing the transaction, then
      the replication filer is reset (to empty), and when the
      slave is later restarted, that transactions error would
      no longer be ignored.
      
      Additionally, as the test cases added in MDEV-33921 rely
      on GTID synchronization, the test cases now force
      master_use_gtid=slave_pos for consistency
      a061ae10
    • Sergei Golubchik's avatar
      MDEV-34353 Revert "don't wait indefinitely for signal handler in --bootstrap" · 36b867ad
      Sergei Golubchik authored
      This reverts commit 938b9293. Not needed after 90d376e0.
      36b867ad
    • Sergei Golubchik's avatar
      MDEV-34539 Invalid "use" and "Schema" in slow query log file with multi-line schema · 8d813f08
      Sergei Golubchik authored
      quote a database name in the slow log
      8d813f08
    • Sergei Golubchik's avatar
      MDEV-34530 dead code in the thr_rwlock.c · f12634f5
      Sergei Golubchik authored
      remove it
      f12634f5
    • Sergei Golubchik's avatar
      MDEV-34434 Hide password passed on commandline from xtrabackup_info · 7ba12d42
      Sergei Golubchik authored
      refine mariadb-backup password zapping check
      7ba12d42
    • Sergei Golubchik's avatar
      d2051816
    • Sergei Golubchik's avatar
      MDEV-34318 mariadb-dump SQL syntax error with MAX_STATEMENT_TIME against Percona MySQL server · d60f5c11
      Sergei Golubchik authored
      protect MariaDB conditional comments from a bug
      in Percona MySQL comment parser
      d60f5c11
    • Sergei Golubchik's avatar
      MDEV-32155 MariaDB Server crashes with ill-formed partitions · dea5746d
      Sergei Golubchik authored
      for ALTER_PARTITION_ADMIN (CHECK/REPAIR/LOAD INDEX/CACHE INDEX/etc)
      partitioning marks affected partitions with PART_ADMIN state.
      
      The assumption is that the server will call a corresponding
      method of ha_partition which will reset the state back to PART_NORMAL.
      
      This assumption is invalid, the server is not required to do so,
      indeed, in CHECK ... FOR UPGRADE the server might decide early that
      the table is fine and won't call ha_partition::check(), leaving
      partitions in the wrong state. It will thus leak into the next
      statement confusing the engine about what it is doing (see
      ha_partition::create_handler_file()), causing a crash later.
      
      Let's force all partitions into PART_NORMAL state after the admin
      operation succeeded, in case it did so without consulting the engine.
      dea5746d
    • Sergei Golubchik's avatar
    • Rucha Deodhar's avatar
      MDEV-32456: incorrect result of gis function in view protocol · 1f28350b
      Rucha Deodhar authored
      There are 3 diff in result:
      1) NULL value from SELECT
      Due to incorrect truncating of the hex value, incorrect value is
      written instead of original value to the view frm. This results in reading
      incorrect value from frm, so eventual result is NULL.
      2) 'Name_exp1' in column name (in gis.test)
      This was because the identifier in SELECT is longer than 64 characters,
      so 'Name_exp1' alias is also written to the view frm.
      3)diff in explain extended
      This was because the query plan for view protocol doesn't
      contain database name. As a fix, disable view protocol for that particular
      query.
      1f28350b
    • Souradeep Saha's avatar
      Refactor import * with only required imports · 4a89f79b
      Souradeep Saha authored
      Import only the required functions instead of all the functions from the
      module to reduce the unnecessary functions in the namespace and prevent
      shadowing. Note: All code changes are non-functional.
      
      All new code of the whole pull request, including one or several
      files that are either new files or modified ones, are contributed
      under the BSD-new license. I am contributing on behalf of my
      employer Amazon Web Services, Inc.
      4a89f79b
    • Robin Newhouse's avatar
      GitLab CI Upgrade CentOS 8 to CentOS 9 build · 008bddaa
      Robin Newhouse authored
      > After May 31, 2024, CentOS Stream 8 will be archived and no further
      updates will be provided. [1]
      
      CentOS Stream 8 is now EOL and should be updated to using CentOS Stream
      9 for compatibility testing in GitLab CI.
      
      [1] https://blog.centos.org/2023/04/end-dates-are-coming-for-centos-stream-8-and-centos-linux-7/
      https://www.centos.org/centos-linux-eol/
      
      All new code of the whole pull request, including one or several files
      that are either new files or modified ones, are contributed under the
      BSD-new license. I am contributing on behalf of my employer Amazon Web
      Services.
      008bddaa
    • Oleksandr Byelkin's avatar
      new CC version · e7c2e25b
      Oleksandr Byelkin authored
      e7c2e25b
    • Oleksandr Byelkin's avatar
      new PCRE2-10.44 · 7478fabc
      Oleksandr Byelkin authored
      7478fabc
    • Alexander Barkov's avatar
      MDEV-28345 ASAN: use-after-poison or unknown-crash in my_strtod_int from... · b777b749
      Alexander Barkov authored
      MDEV-28345 ASAN: use-after-poison or unknown-crash in my_strtod_int from charset_info_st::strntod or test_if_number
      
      This patch fixes two problems:
      
      - The code inside my_strtod_int() in strings/dtoa.c could test the byte
        behind the end of the string when processing the mantissa.
        Rewriting the code to avoid this.
      
      - The code in test_if_number() in sql/sql_analyse.cc called my_atof()
        which is unsafe and makes the called my_strtod_int() look behind
        the end of the string if the input string is not 0-terminated.
        Fixing test_if_number() to use my_strtod() instead, passing the correct
        end pointer.
      b777b749
    • Sutou Kouhei's avatar
      MDEV-21166 Add Mroonga initialized check to Mroonga UDFs · 383d53ed
      Sutou Kouhei authored
      Mroonga UDFs can't be used without loading Mroonga.
      383d53ed
    • PinkFreud's avatar
      MDEV-34604 mytop - fix specifying filters in .mytop · 49dff5a4
      PinkFreud authored
      Specifying filters (filter_status, filter_user, etc) in the mytop config
      previously wouldn't work, because any filter specified here was added to
      the config hash as a literal string.
      
      This change fixes that - if filter_* is defined in the config and matches
      an existing filter_* key, then run the value through StringOrRegex() and
      assign to the config hash.
      49dff5a4
    • Yuchen Pei's avatar
      MDEV-30408 Reset explicit_limit in exists2in · 03a35037
      Yuchen Pei authored
      Item_exists_subselect::fix_length_and_dec() sets explicit_limit to 1.
      In the exists2in transformation it resets select_limit to NULL. For
      consistency we should reset explicity_limit too.
      
      This fixes a bug where spider table returns wrong results for queries
      that gets through exists2in transformation when semijoin is off.
      03a35037
    • Yuchen Pei's avatar
      MDEV-32627 [fixup] Spider: Fix conn key length · 8416fd32
      Yuchen Pei authored
      To avoid off-by-one in spider_get_share.
      
      And document conn key and conn key length.
      8416fd32
    • Yuchen Pei's avatar
      MDEV-32627 Distinguish between absence of a keyword and empty value for the keyword · 20d99f3f
      Yuchen Pei authored
      Distinguish them in two place:
      
      when constructing connection key in create_conn_key and
      spider_create_conn for both ordinary queries and spider_direct_sql
      
      For spider_create_conn and spider_udf_direct_sql_create_conn, the
      created conn gets assigned a field from the source object if and only
      if source->field is non-null.
      
      For spider_create_conn_keys and spider_udf_direct_sql_create_conn_key,
      we update the encoding so that absence of keyword and keyword with an
      empty value result in different keys. More specifically, if the i-th
      keyword has a value, the corresponding part in the conn key begins
      with the char \i, followed by the possibly empty value and ends with a
      \0. If the i-th keyword is not specified, then it does not get a
      mention in the conn key.
      
      As part of this change, we also update table param / option parsing to
      recognise a singleton empty string when creating an string list,
      instead of writing it off as NULL.
      20d99f3f
    • Yuchen Pei's avatar
      MDEV-32627 Spider: add tests for connection param overriding · 6f3baec4
      Yuchen Pei authored
      When connection parameters are specified in COMMENT, CONNECTION and
      SERVER, they are overriden in the following order:
      
      COMMENT > CONNECTION > SERVER
      6f3baec4
  11. 16 Jul, 2024 6 commits
    • Tuukka Pasanen's avatar
      MDEV-33837: Workaround chown warnings · 62dfd0c0
      Tuukka Pasanen authored
      Blindly recursive chown is not way to do it.
      This Workaround is not much better than just chown -R but
      there is small adjustment just chown MariaDB statedir and logdir
      then with find only chown those files that are not correctly
      owned.
      
      Fixes lintian warnings:
       * W: mariadb-server: recursive-privilege-change "chown -R" [postinst:*]
       * W: mariadb-server: recursive-privilege-change "chown -R" [postinst:*]
      62dfd0c0
    • Sergei Petrunia's avatar
      MDEV-30623: Fix the testcase · e644e130
      Sergei Petrunia authored
      - Fix view-protocol: long expressions in SELECT
        list should have "expr AS column_name".
      
      - Also, moved the test from subselect*test to
        suite/json/t/json_table.test.
      e644e130
    • Oleg Smirnov's avatar
      MDEV-33010 Crash when pushing condition with CHARSET()/COERCIBILITY() into derived table · 972879f4
      Oleg Smirnov authored
      Based on the current logic, objects of classes Item_func_charset and
      Item_func_coercibility (responsible for CHARSET() and COERCIBILITY()
      functions) are always considered constant.
      However, SQL syntax allows their use in a non-constant manner, such as
      CHARSET(t1.a), COERCIBILITY(t1.a).
      
      In these cases, the `used_tables()` parameter corresponds to table names
      in the function parameters, creating an inconsistency: the item is marked
      as constant but accesses tables. This leads to crashes when
      conditions with CHARSET()/COERCIBILITY() are pushed into derived tables.
      
      This commit addresses the issue by setting `used_tables()` to 0 for
      `Item_func_charset` and `Item_func_coercibility`. Additionally, the items
      now store the return values during the preparation phase and return
      them during the execution phase. This ensures that the items do not call
      its arguments methods during the execution and are truly constant.
      
      Reviewer: Alexander Barkov <bar@mariadb.com>
      972879f4
    • Yuchen Pei's avatar
      MDEV-34421 Check the SQL command when resolving storage engine · 384ec03e
      Yuchen Pei authored
      ENGINE_SUBSTITUTION only applies to CREATE TABLE and ALTER TABLE, and
      Storage_engine_name::resolve_storage_engine_with_error() could be
      called when executing any sql command.
      384ec03e
    • Yuchen Pei's avatar
      MDEV-34541 Clean up spider self reference check · 132270d3
      Yuchen Pei authored
      SPIDER_CONN::loop_check_meraged_first is useless, because all
      SPIDER_CONN_LOOP_CHECKs are in SPIDER_CONN::loop_check_queue, which in
      spider_db_conn::fin_loop_check() is iterated over.
      
      This fixes the use-after-free issue when there are three spider tables
      sharing the same remote, and their corresponding
      SPIDER_CONN_LOOP_CHECKs getting merged in
      spider_conn_queue_and_merge_loop_check()
      
      This also fixes MDEV-34555
      132270d3
    • Yuchen Pei's avatar
      MDEV-29962 Spider: creates connections if needed before lock_tables() · 0bb98628
      Yuchen Pei authored
      Same cause as MDEV-31996. This prevents reuse of conns freed
      previously, e.g. during the commit of previous statement.
      
      It does not occur in 10.4 because of the commit for MDEV-19002 removed
      the call to spider_check_trx_and_get_conn().
      0bb98628