maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexey!

Few questions first:

* can a CHECK constraint have the same name as an existing FK?

* what about UNIQUE constraint? it's also a constraint. The standard
  says that DROP CONSTRAINT should work for uniques, and ALTER already
  supports ADD CONSTRAINT UNIQUE (and ADD CONSTRAINT FOREIGN KEY)

Otherwise it's fine, a couple of comments see below.

On Jan 31, Alexey Botchkov wrote:
> revision-id: 43296f4a90cc9281b4ddad498bfab4a4e43131b9 (mariadb-10.2.21-45-g43296f4)
> parent(s): 97930df13c0e403940969ebb47398760b59f753c
> committer: Alexey Botchkov
> timestamp: 2019-01-31 16:23:12 +0400
> message:
> 
> MDEV-17599 ALTER TABLE DROP CONSTRAINT does not work for foreign keys.
> 
> The list of table constraints doesn't include foreign keys.
> So we replace DROP CONSTRAINT with DROP FOREIGN KEY in this case.
> 
> diff --git a/mysql-test/t/alter_table.test b/mysql-test/t/alter_table.test
> index df077c8..659cae1 100644
> --- a/mysql-test/t/alter_table.test
> +++ b/mysql-test/t/alter_table.test
> @@ -2013,5 +2013,16 @@ DROP TABLE t1;
>  
>  
>  --echo #
> +--echo # MDEV-17599 ALTER TABLE DROP CONSTRAINT does not work for foreign keys.
> +--echo #
> +
> +CREATE TABLE t1(id INT PRIMARY KEY, c1 INT) ENGINE= INNODB;
> +CREATE TABLE t2(id INT PRIMARY KEY, c1 INT, CONSTRAINT sid FOREIGN KEY (`c1`) REFERENCES t1 (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION) ENGINE= INNODB;
> +SHOW CREATE TABLE t2;
> +ALTER TABLE t2 DROP CONSTRAINT sid;

please, add a test that drops FK and CHECK constraints in one statement.

> +SHOW CREATE TABLE t2;
> +DROP TABLE t2, t1;
> +
> +--echo #
>  --echo # End of 10.2 tests
>  --echo #
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> index 1b426f8..c080e5a 100644
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -8993,6 +8993,44 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
>  
>    THD_STAGE_INFO(thd, stage_setup);
>  
> +
> +  if (alter_info->flags & Alter_info::ALTER_DROP_CHECK_CONSTRAINT)
> +  {
> +    /*
> +      ALTER TABLE DROP CONSTRAINT
> +      should be replaced with ... DROP FOREIGN KEY
> +      if the constraint is the FOREIGN KEY one.
> +    */
> +
> +    List_iterator<Alter_drop> drop_it(alter_info->drop_list);
> +    Alter_drop *drop;
> +    alter_info->flags&= ~Alter_info::ALTER_DROP_CHECK_CONSTRAINT;
> +
> +    while ((drop= drop_it++))
> +    {
> +      if (drop->type == Alter_drop::CHECK_CONSTRAINT)
> +      {
> +        List <FOREIGN_KEY_INFO> fk_child_key_list;
> +        FOREIGN_KEY_INFO *f_key;
> +        table->file->get_foreign_key_list(thd, &fk_child_key_list);

better do this outside of the loop

> +        List_iterator<FOREIGN_KEY_INFO> fk_key_it(fk_child_key_list);
> +
> +        while ((f_key= fk_key_it++))
> +        {
> +          if (my_strcasecmp(system_charset_info, f_key->foreign_id->str,
> +                drop->name) == 0)
> +          {
> +            drop->type= Alter_drop::FOREIGN_KEY;
> +            alter_info->flags|= Alter_info::DROP_FOREIGN_KEY;
> +            goto do_continue;
> +          }
> +        }
> +      }
> +      alter_info->flags|= Alter_info::ALTER_DROP_CHECK_CONSTRAINT;
> +do_continue:;
> +    }
> +  }
> +
>    handle_if_exists_options(thd, table, alter_info);

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx