Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-23672

Assertion `v.v_indexes.empty()' failed in dict_table_t::instant_column

Details

    Description

      10.5.6 compiled with debug and ASAN
      origin/10.5 938db04898ee9029421f5251751239255dd81c15 2020-09-03T16:40:42+03:00
       
      suite/innodb/t/TBR-234.test
      --------------------------------------------
      --source include/have_innodb.inc
      CREATE TABLE IF NOT EXISTS t4 (
          col_int INTEGER,
          col_text TEXT,
          col_int_g INTEGER GENERATED ALWAYS AS (col_int),
          col_text_g TEXT GENERATED ALWAYS AS (SUBSTR(col_text,1,499)) )
      ENGINE = InnoDB ROW_FORMAT = Redundant  ;
      ALTER TABLE t4 MODIFY COLUMN col_text TEXT NOT NULL ;
      ALTER TABLE t4 ADD UNIQUE ( col_int_g ) ;
      ALTER TABLE t4 MODIFY COLUMN col_text TEXT NULL ;
      DROP TABLE t4;
       
      ./mtr --mem --suite=innodb TBR-234
      ...
      mariadbd: storage/innobase/handler/handler0alter.cc:572: bool dict_table_t::instant_column(const dict_table_t&, const ulint*): Assertion `v.v_indexes.empty()' failed.
      ...
      Query (0x62b0000a1238): ALTER TABLE t4 MODIFY COLUMN col_text TEXT NULL
      Connection ID (thread ID): 4
      Status: NOT_KILLED
       
      Thread 1 (Thread 0x7f9c4b170300 (LWP 19070)):
      #0  __pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at ../sysdeps/unix/sysv/linux/pthread_kill.c:57
      #1  0x0000557d93de28ed in my_write_core (sig=sig@entry=6) at mysys/stacktrace.c:424
      #2  0x0000557d92611436 in handle_fatal_signal (sig=<optimized out>) at sql/signal_handler.cc:330
      #3  <signal handler called>
      #4  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
      #5  0x00007f9c59635f5d in __GI_abort () at abort.c:90
      #6  0x00007f9c5962bf17 in __assert_fail_base (fmt=<optimized out>, assertion=assertion@entry=0x557d946ac000 "v.v_indexes.empty()", file=file@entry=0x557d9469fb60 "storage/innobase/handler/handler0alter.cc", line=line@entry=572, function=function@entry=0x557d946b4300 <dict_table_t::instant_column(dict_table_t const&, unsigned long const*)::__PRETTY_FUNCTION__> "bool dict_table_t::instant_column(const dict_table_t&, const ulint*)") at assert.c:92
      #7  0x00007f9c5962bfc2 in __GI___assert_fail (assertion=assertion@entry=0x557d946ac000 "v.v_indexes.empty()", file=file@entry=0x557d9469fb60 "storage/innobase/handler/handler0alter.cc", line=line@entry=572, function=function@entry=0x557d946b4300 <dict_table_t::instant_column(dict_table_t const&, unsigned long const*)::__PRETTY_FUNCTION__> "bool dict_table_t::instant_column(const dict_table_t&, const ulint*)") at assert.c:101
      #8  0x0000557d93299119 in dict_table_t::instant_column (this=0x6190000e1620, table=..., col_map=<optimized out>) at storage/innobase/handler/handler0alter.cc:572
      #9  0x0000557d9329a7ef in ha_innobase_inplace_ctx::instant_column (this=this@entry=0x62b0000a3090) at storage/innobase/handler/handler0alter.cc:1051
      #10 0x0000557d9329dc12 in innobase_instant_try (ha_alter_info=ha_alter_info@entry=0x7f9c4b1698e0, ctx=ctx@entry=0x62b0000a3090, altered_table=altered_table@entry=0x7f9c4b169e20, table=table@entry=0x6190000e7e98, trx=trx@entry=0x7f9c50d3cb90) at storage/innobase/handler/handler0alter.cc:5634
      #11 0x0000557d932a2e29 in commit_try_norebuild (ha_alter_info=ha_alter_info@entry=0x7f9c4b1698e0, ctx=ctx@entry=0x62b0000a3090, altered_table=altered_table@entry=0x7f9c4b169e20, old_table=<optimized out>, trx=trx@entry=0x7f9c50d3cb90, table_name=<optimized out>) at storage/innobase/handler/handler0alter.cc:10198
      #12 0x0000557d93270dcf in ha_innobase::commit_inplace_alter_table (this=<optimized out>, altered_table=<optimized out>, ha_alter_info=<optimized out>, commit=<optimized out>) at storage/innobase/handler/handler0alter.cc:10919
      #13 0x0000557d926401bb in handler::ha_commit_inplace_alter_table (this=0x61d000260cb8, altered_table=altered_table@entry=0x7f9c4b169e20, ha_alter_info=ha_alter_info@entry=0x7f9c4b1698e0, commit=commit@entry=true) at sql/handler.cc:4843
      #14 0x0000557d92042267 in mysql_inplace_alter_table (thd=thd@entry=0x62b00009a218, table_list=0x62b0000a1370, table=table@entry=0x6190000e7e98, altered_table=<optimized out>, ha_alter_info=<optimized out>, target_mdl_request=<optimized out>, alter_ctx=<optimized out>) at sql/sql_table.cc:8040
      #15 0x0000557d92078e3b in mysql_alter_table (thd=thd@entry=0x62b00009a218, new_db=<optimized out>, new_name=new_name@entry=0x62b00009f060, create_info=create_info@entry=0x7f9c4b16bae0, table_list=<optimized out>, table_list@entry=0x62b0000a1370, alter_info=alter_info@entry=0x7f9c4b16b9e0, order_num=<optimized out>, order=<optimized out>, ignore=<optimized out>, if_exists=<optimized out>) at sql/sql_table.cc:10570
      #16 0x0000557d922266fd in Sql_cmd_alter_table::execute (this=<optimized out>, thd=0x62b00009a218) at sql/sql_alter.cc:534
      #17 0x0000557d91d88c7d in mysql_execute_command (thd=thd@entry=0x62b00009a218) at sql/sql_parse.cc:5952
      #18 0x0000557d91da10f5 in mysql_parse (thd=thd@entry=0x62b00009a218, rawbuf=<optimized out>, length=<optimized out>, parser_state=parser_state@entry=0x7f9c4b16e000, is_com_multi=is_com_multi@entry=false, is_next_command=is_next_command@entry=false) at sql/sql_parse.cc:7994
      #19 0x0000557d91d66522 in dispatch_command (command=command@entry=COM_QUERY, thd=thd@entry=0x62b00009a218, packet=packet@entry=0x629000285219 "ALTER TABLE t4 MODIFY COLUMN col_text TEXT NULL ", packet_length=packet_length@entry=48, is_com_multi=is_com_multi@entry=false, is_next_command=is_next_command@entry=false) at sql/sql_parse.cc:1867
      #20 0x0000557d91d61367 in do_command (thd=0x62b00009a218) at sql/sql_parse.cc:1348
      #21 0x0000557d9220b9e1 in do_handle_one_connection (connect=<optimized out>, connect@entry=0x6080000027b8, put_in_cache=put_in_cache@entry=true) at sql/sql_connect.cc:1410
      #22 0x0000557d9220ccac in handle_one_connection (arg=arg@entry=0x6080000027b8) at sql/sql_connect.cc:1312
      #23 0x0000557d92f973fa in pfs_spawn_thread (arg=0x615000008c98) at storage/perfschema/pfs.cc:2201
      #24 0x00007f9c5a4db7fc in start_thread (arg=0x7f9c4b170300) at pthread_create.c:465
      #25 0x00007f9c59711b5f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
      

      No replay of the assert

      • on 10.4 or 10.2
      • tables with ROW_FORMAT != Redundant

      Attachments

        Issue Links

          Activity

            Please do not exceed 79 columns line length.

            The following chunk of your fix seems to indicate another problem:

            @@ -603,7 +605,6 @@ inline bool dict_table_t::instant_column(const dict_table_t& table,
             
             	for (unsigned i = 0; i < n_v_def; i++) {
             		dict_v_col_t& v = v_cols[i];
            -		DBUG_ASSERT(v.v_indexes.empty());
             		v.n_v_indexes = 0;
             		v.base_col = static_cast<dict_col_t**>(
             			mem_heap_dup(heap, v.base_col,
            

            The field n_v_indexes looks like a redundant alias for v_indexes.size(). Can we remove n_v_indexes altogether? I think that the idea of preserving dict_v_col_t::v_indexes should be sound. The data stored in dict_v_idx_t will remain unaffected by changes to column metadata.

            Could you add an UPDATE and CHECK TABLE to the test case? Also test with multiple rows with NULL in the UNIQUE column.

            marko Marko Mäkelä added a comment - Please do not exceed 79 columns line length. The following chunk of your fix seems to indicate another problem: @@ -603,7 +605,6 @@ inline bool dict_table_t::instant_column(const dict_table_t& table, for (unsigned i = 0; i < n_v_def; i++) { dict_v_col_t& v = v_cols[i]; - DBUG_ASSERT(v.v_indexes.empty()); v.n_v_indexes = 0; v.base_col = static_cast<dict_col_t**>( mem_heap_dup(heap, v.base_col, The field n_v_indexes looks like a redundant alias for v_indexes.size() . Can we remove n_v_indexes altogether? I think that the idea of preserving dict_v_col_t::v_indexes should be sound. The data stored in dict_v_idx_t will remain unaffected by changes to column metadata. Could you add an UPDATE and CHECK TABLE to the test case? Also test with multiple rows with NULL in the UNIQUE column.

            The latest version looks OK to me, but please stage&push it to 10.4, not 10.5.

            marko Marko Mäkelä added a comment - The latest version looks OK to me, but please stage&push it to 10.4, not 10.5.

            After the failing debug assertion was removed, AddressSanitizer would fail with heap-use-after-free on the test case. I believe that the originally reported bug is partly caused by the fact that the SQL layer asks InnoDB to create an index that it thinks that the column modification is affecting (in fact, it is not because virtual columns cannot be NOT NULL), but does not ask to drop the old index.

            I restored the debug assertion and effectively disabled the test case.

            marko Marko Mäkelä added a comment - After the failing debug assertion was removed, AddressSanitizer would fail with heap-use-after-free on the test case. I believe that the originally reported bug is partly caused by the fact that the SQL layer asks InnoDB to create an index that it thinks that the column modification is affecting (in fact, it is not because virtual columns cannot be NOT NULL ), but does not ask to drop the old index. I restored the debug assertion and effectively disabled the test case.

            Sorry, I misinterpreted the constants, referring to the wrong set of #define. The correct ones are as follows:

            // Set for ALTER [COLUMN] ... SET DEFAULT ... | DROP DEFAULT
            #define ALTER_CHANGE_COLUMN_DEFAULT (1ULL <<  8)
            // Change column from NOT NULL to NULL
            #define ALTER_COLUMN_NULLABLE                (1ULL << 52)
            

            So, there does not seem to be any bug in the SQL layer. Only a small fixup to the InnoDB code is needed. And I hereby approve that.

            marko Marko Mäkelä added a comment - Sorry, I misinterpreted the constants, referring to the wrong set of #define . The correct ones are as follows: // Set for ALTER [COLUMN] ... SET DEFAULT ... | DROP DEFAULT #define ALTER_CHANGE_COLUMN_DEFAULT (1ULL << 8) … // Change column from NOT NULL to NULL #define ALTER_COLUMN_NULLABLE (1ULL << 52) So, there does not seem to be any bug in the SQL layer. Only a small fixup to the InnoDB code is needed. And I hereby approve that.

            People

              midenok Aleksey Midenkov
              mleich Matthias Leich
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.