MDEV -18463: Don't allow multiple table CONSTRAINTs with the same name.

[an3l]

MariaDB [test]> create table tpk (id INT NOT NULL AUTO_INCREMENT PRIMARY KEY,name VARCHAR(100) NOT NULL);
MariaDB [test]> create table tfk (id INT, c1 INT, c2 INT NOT NULL, CONSTRAINT sid FOREIGN KEY (`c1`) REFERENCES tpk (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION, CONSTRAINT sid check (c2>15));
--
Query OK, 0 rows affected (0.23 sec)
 
MariaDB [test]> show create table tfk;
+-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
\| Table \| Create Table                                                                                                                                                                                                                                                                                                   \|
+-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
\| tfk   \| CREATE TABLE `tfk` (
`id` int(11) DEFAULT NULL,
`c1` int(11) DEFAULT NULL,
`c2` int(11) NOT NULL,
KEY `sid` (`c1`),
CONSTRAINT `sid` FOREIGN KEY (`c1`) REFERENCES `tpk` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION,
CONSTRAINT `sid` CHECK (`c2` > 15)
) ENGINE=InnoDB DEFAULT CHARSET=latin1 \|
+-------+-------------------------------------------------------------------------------------------

This shouldn't be allowed!

[an3l]

This is ok

MariaDB [test]> create table tcheck(t int, constraint a check(t>10),constraint a check(t<0));
ERROR 1826 (HY000): Duplicate CHECK constraint name 'a'

We have 2 elements in alter_info->check_constraint_list

(gdb) bt
+bt
#0  mysql_prepare_create_table (thd=0x7fff60000cf8, create_info=0x7ffff4bcb940, alter_info=0x7ffff4bcb890, db_options=0x7ffff4bc9d2c, file=0x7fff600117c8, key_info_buffer=0x7ffff4bcb320, key_count=0x7ffff4bcb314, create_table_mode=0) at sql/sql_table.cc:4196
#1  0x0000555555bc2324 in mysql_create_frm_image (thd=0x7fff60000cf8, db=0x7fff60010ce8 "test", table_name=0x7fff600106a0 "t", create_info=0x7ffff4bcb940, alter_info=0x7ffff4bcb890, create_table_mode=0, key_info=0x7ffff4bcb320, key_count=0x7ffff4bcb314, frm=0x7ffff4bcb330) a
t sql/sql_table.cc:4650
#2  0x0000555555bc2d95 in create_table_impl (thd=0x7fff60000cf8, orig_db=0x7fff60010ce8 "test", orig_table_name=0x7fff600106a0 "t", db=0x7fff60010ce8 "test", table_name=0x7fff600106a0 "t", path=0x7ffff4bcb340 "./test/t", options=..., create_info=0x7ffff4bcb940, alter_info=0x
7ffff4bcb890, create_table_mode=0, is_trans=0x7ffff4bcb59e, key_info=0x7ffff4bcb320, key_count=0x7ffff4bcb314, frm=0x7ffff4bcb330) at sql/sql_table.cc:4896
#3  0x0000555555bc33f9 in mysql_create_table_no_lock (thd=0x7fff60000cf8, db=0x7fff60010ce8 "test", table_name=0x7fff600106a0 "t", create_info=0x7ffff4bcb940, alter_info=0x7ffff4bcb890, is_trans=0x7ffff4bcb59e, create_table_mode=0) at sql/sql_table.cc:5016
#4  0x0000555555bc366f in mysql_create_table (thd=0x7fff60000cf8, create_table=0x7fff600106d8, create_info=0x7ffff4bcb940, alter_info=0x7ffff4bcb890) at sql/sql_table.cc:5081
#5  0x0000555555afba8c in mysql_execute_command (thd=0x7fff60000cf8) at sql/sql_parse.cc:4041
#6  0x0000555555b08459 in mysql_parse (thd=0x7fff60000cf8, rawbuf=0x7fff60010570 "create table t(t int, constraint a foreign key (t) references tpk(id),constraint a check(t<0))", length=94, parser_state=0x7ffff4bcc230, is_com_multi=false, is_next_command=false) at sql/sql_pa
rse.cc:8013
#7  0x0000555555af5e79 in dispatch_command (command=COM_QUERY, thd=0x7fff60000cf8, packet=0x7fff60008259 "create table t(t int, constraint a foreign key (t) references tpk(id),constraint a check(t<0))", packet_length=94, is_com_multi=false, is_next_command=false) at sql/sql_
parse.cc:1831
#8  0x0000555555af4845 in do_command (thd=0x7fff60000cf8) at sql/sql_parse.cc:1385
#9  0x0000555555c42392 in do_handle_one_connection (connect=0x5555578ded58) at sql/sql_connect.cc:1335
#10 0x0000555555c42112 in handle_one_connection (arg=0x5555578ded58) at sql/sql_connect.cc:1241
#11 0x00007ffff6a956db in start_thread (arg=0x7ffff4bcd700) at pthread_create.c:463
#12 0x00007ffff5e7f88f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

gdb: sql_table.cc:4196
+p alter_info->check_constraint_list
$16 = {
  <base_list> = {
    <Sql_alloc> = {<No data fields>},
    members of base_list:
    first = 0x7fff60011458,
    last = 0x7fff60011468,
    elements = 2
  }, <No data fields>}

This is not ok

MariaDB [test]> create table t(t int, constraint a foreign key (t) references tpk(id),constraint a check(t<0));

We have only 1 element in list

gdb: 
+p alter_info->check_constraint_list
$17 = {
  <base_list> = {
    <Sql_alloc> = {<No data fields>},
    members of base_list:
    first = 0x7fff60011350,
    last = 0x7fff60011350,
    elements = 1
  }, <No data fields>}

Proposal 1 using the grammar

This should be solved in sql/sql_yacc.yy

sql_yacc.yy: 6101
constraint_def:
         opt_constraint check_constraint
         {
           Lex->add_constraint(&$1, $2, FALSE);
         }
       ;
sql_yacc.yy: 6131
opt_constraint:
          /* empty */ { $$= null_lex_str; }
        | constraint { $$= $1; }
        ;

constraint:
          CONSTRAINT opt_ident { $$=$2; }
        ;

We can clearly see that in function add_constraint() constraint (actually CHECK constraint) is added to the list, but I couldn't see the same for other types of constraints.

sql_lex.h:3109 add_constraint() => added to alter_info.check_constraint_list
  // Add a constraint as a part of CREATE TABLE or ALTER TABLE
  bool add_constraint(LEX_STRING *name, Virtual_column_info *constr,
                      bool if_not_exists)

In key_def: rule is added grammar for foreign key

| opt_constraint FOREIGN KEY_SYM opt_if_not_exists opt_ident
{
            if (Lex->check_add_key($4) ||
               !(Lex->last_key= (new (thd->mem_root)
                                 Key(Key::MULTIPLE, $1.str ? $1 : $5,
                                     HA_KEY_ALG_UNDEF, true, $4))))
              MYSQL_YYABORT;
            Lex->option_list= NULL;
          }

Conclusion:

add_constraint() is using Virtual_column_info* as a parameter (expression) and foreign key constraint is not using the same so we can not use this approach. I have to use a list of all constraints not only check!

[an3l]

Proposal 2

Solve problem while preparing the create of table in mysql_prepare_create_table sql/sql_table.cc According to synopsis of function above we have

SYNOPSIS
    mysql_prepare_create_table()
      thd                       Thread object.
      create_info               Create information (like MAX_ROWS).
      alter_info                List of columns and indexes to create
      db_options          INOUT Table options (like HA_OPTION_PACK_RECORD).
      file                      The handler for the new table.
      key_info_buffer     OUT   An array of KEY structs for the indexes.
      key_count           OUT   The number of elements in the array.
      create_table_mode         C_ORDINARY_CREATE, C_ALTER_TABLE,
                                C_CREATE_SELECT, C_ASSISTED_DISCOVERY

  DESCRIPTION
    Prepares the table and key structures for table creation.

  NOTES
    sets create_info->varchar if the table has a varchar

  RETURN VALUES
    FALSE    OK
    TRUE     error

2 check constraints

So when we have 2 check constraints we can see in alter_info that exist 2 elements in check_constraint_list and 0 in key_list and create_list :

+p *alter_info
$8 = {
  drop_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x555556fde3e0 <end_of_list>, 
      last = 0x7ffff4bcb890, 
      elements = 0
    }, <No data fields>}, 
  alter_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x555556fde3e0 <end_of_list>, 
      last = 0x7ffff4bcb8a8, 
      elements = 0
    }, <No data fields>}, 
  key_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x555556fde3e0 <end_of_list>, 
      last = 0x7ffff4bcb8c0, 
      elements = 0
    }, <No data fields>}, 
  create_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x7fff64011448, 
      last = 0x7fff64011448, 
      elements = 1
    }, <No data fields>}, 
  check_constraint_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x7fff64011458, 
      last = 0x7fff64011468, 
      elements = 2
    }, <No data fields>}, 
  flags = 0, 
  keys_onoff = Alter_info::LEAVE_AS_IS, 
  partition_names = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x555556fde3e0 <end_of_list>, 
      last = 0x7ffff4bcb910, 
      elements = 0
    }, <No data fields>}, 
  num_parts = 0, 
  requested_algorithm = Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT, 
  requested_lock = Alter_info::ALTER_TABLE_LOCK_DEFAULT
}

1 foreign key and 1 check constraint

In this scenario we have 2 elements in key_list and 1 element in create_list.

(gdb) p *alter_info
+p *alter_info
$9 = {
  drop_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x555556fde3e0 <end_of_list>, 
      last = 0x7ffff4bcb890, 
      elements = 0
    }, <No data fields>}, 
  alter_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x555556fde3e0 <end_of_list>, 
      last = 0x7ffff4bcb8a8, 
      elements = 0
    }, <No data fields>}, 
  key_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x7fff64011320, 
      last = 0x7fff64011330, 
      elements = 2
    }, <No data fields>}, 
  create_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x7fff64011340, 
      last = 0x7fff64011340, 
      elements = 1
    }, <No data fields>}, 
  check_constraint_list = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x7fff64011350, 
      last = 0x7fff64011350, 
      elements = 1
    }, <No data fields>}, 
  flags = 2097152, 
  keys_onoff = Alter_info::LEAVE_AS_IS, 
  partition_names = {
    <base_list> = {
      <Sql_alloc> = {<No data fields>}, 
      members of base_list: 
      first = 0x555556fde3e0 <end_of_list>, 
      last = 0x7ffff4bcb910, 
      elements = 0
    }, <No data fields>}, 
  num_parts = 0, 
  requested_algorithm = Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT, 
  requested_lock = Alter_info::ALTER_TABLE_LOCK_DEFAULT
}

Questions

1)When I was debugging I saw that 1 element is of type foreign and second one is type multiple, is this correct behavior ??? Need to ask! 2) Why create_list has 1 element ? 3) When using iterator bellow I found it is null, why? To iterate over the key_list and create_list one can use

        /* Check that there's no repeating key constraint names. */
        List_iterator_fast<Key> key_it(alter_info->key_list);
        Key *key;
        while ((key= key_it++))
        {
          if (check->name.length == key->name.length &&
              my_strcasecmp(system_charset_info,
                            check->name.str, key->name.str) == 0 && key->type == Key::FOREIGN_KEY)
          {
            my_error(ER_DUP_CONSTRAINT_NAME, MYF(0), "CHECK", check->name.str);
            DBUG_RETURN(TRUE);
          }
        }
/* Not used
    List_iterator<Create_field> it(alter_info->create_list);
    Create_field *sql_field;
    while ((sql_field=it++)) {}
*/

Applying above change we are obtaining the right result

MariaDB [test]> create table tt(t int, constraint a foreign key (t) references tpk(id),constraint a check(t<0));
ERROR 1826 (HY000): Duplicate CHECK constraint name 'a'
MariaDB [test]> create table t(t int, constraint a foreign key (t) references tpk(id),constraint x check(t<0));
Query OK, 0 rows affected (0.04 sec)
MariaDB [test]> create table tcheck(t int, constraint a check(t>10),constraint a check(t<0));
ERROR 1826 (HY000): Duplicate CHECK constraint name 'a'

Wrote the test in mysql-test/t/check_constraint .test and run ./mtr check_constraint --manual-gdb.