← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 4949925: MDEV-10421 duplicate CHECK CONSTRAINTs.

 

Hi, Alexey!

Looks good, thanks!
See few comments below, then ok to push!

On Aug 30, Alexey Botchkov wrote:
> revision-id: 49499250c04b631bd908945b7c59d6affaeb2703 (mariadb-10.2.1-9-g4949925)
> parent(s): 08683a726773f8cdf16a4a3dfb3920e5f7842481
> committer: Alexey Botchkov
> timestamp: 2016-08-30 16:43:05 +0400
> message:
> 
> MDEV-10421 duplicate CHECK CONSTRAINTs.
> 
>         mysql_prepare_create_table fixed so it doesn't let duplicating
>         constraint names. Syntax for CONSTRAINT IF NOT EXISTS added
>         and handled in mysql_alter_table.
> 
> diff --git a/sql/field.h b/sql/field.h
> index 13a0e91..7b271b0 100644
> --- a/sql/field.h
> +++ b/sql/field.h
> @@ -606,6 +606,7 @@ class Virtual_column_info: public Sql_alloc
>    /* Flag indicating  that the field is physically stored in the database */
>    bool stored_in_db;
>    bool utf8;                                    /* Already in utf8 */
> +  bool create_if_not_exists;

You apparently forgot to remove create_if_not_exists. It doesn't seem to
be used anywhere.

>    /* The expression to compute the value of the virtual column */
>    Item *expr_item;
>    /* Text representation of the defining expression */
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> index e745fe8..c120fbc 100644
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -6130,6 +6146,38 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info)
>    }
>  #endif /*WITH_PARTITION_STORAGE_ENGINE*/
>  
> +  /* ADD CONSTRAINT IF NOT EXISTS. */
> +  {
> +    List_iterator<Virtual_column_info> it(alter_info->check_constraint_list);
> +    Virtual_column_info *check;
> +    TABLE_SHARE *share= table->s;
> +    uint c;
> +    while ((check=it++))
> +    {
> +      if (!check->flags /* & Alter_info::CHECK_CONSTRAINT_IF_NOT_EXISTS */ &&

why is that? because CHECK_CONSTRAINT_IF_NOT_EXISTS is the only possible
flag now? I'd still prefer it a bit more future-proof, better to check
the flag here.

> +          check->name.length)
> +        continue;
> +      check->flags= 0;
> +      for (c= share->field_check_constraints;
> +           c < share->table_check_constraints ; c++)
> +      {
> +        Virtual_column_info *dup= table->check_constraints[c];
> +        if (dup->name.length == check->name.length &&
> +            my_strcasecmp(system_charset_info,
> +                          check->name.str, dup->name.str) == 0)
> +        {
> +          push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
> +            ER_DUP_CONSTRAINT_NAME, ER_THD(thd, ER_DUP_CONSTRAINT_NAME),
> +            "CHECK", check->name.str);
> +          it.remove();
> +          if (alter_info->check_constraint_list.elements == 0)
> +            alter_info->flags&= ~Alter_info::ALTER_ADD_CHECK_CONSTRAINT;
> +
> +          break;
> +        }
> +      }
> +    }
> +  }
>    DBUG_VOID_RETURN;
>  }

You seem to forgot to add

  #define ER_FK_DUP_NAME ER_DUP_CONSTRAINT_NAME

to mysql.h.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx