Skip to content

Commit

Permalink
MDEV-22602 Disable UPDATE CASCADE for SQL constraints
Browse files Browse the repository at this point in the history
CHECK constraint is checked by check_expression() which walks its
items and gets into Item_field::check_vcol_func_processor() to check
for conformity with foreign key list.

WITHOUT OVERLAPS is checked for same conformity in
mysql_prepare_create_table().

Long uniques are already impossible with InnoDB foreign keys. See
ER_CANT_CREATE_TABLE in test case.

2 accompanying bugs fixed (test main.constraints failed):

1. check->name.str lived on SP execute mem_root while "check" obj
itself lives on SP main mem_root. On second SP execute check->name.str
had garbage data. Fixed by allocating from thd->stmt_arena->mem_root
which is SP main mem_root.

2. CHECK_CONSTRAINT_IF_NOT_EXISTS value was mixed with
VCOL_FIELD_REF. VCOL_FIELD_REF is assigned in check_expression() and
then detected as CHECK_CONSTRAINT_IF_NOT_EXISTS in
handle_if_exists_options().

Existing cases for MDEV-16932 in main.constraints cover both fixes.
  • Loading branch information
midenok committed Jun 12, 2020
1 parent 02c255d commit 762bf7a
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 24 deletions.
18 changes: 18 additions & 0 deletions mysql-test/suite/innodb/r/foreign_key.result
Original file line number Diff line number Diff line change
Expand Up @@ -766,3 +766,21 @@ ALTER TABLE t1 ADD FOREIGN KEY (a) REFERENCES t1 (b);
ERROR HY000: Can't create table `test`.`t1` (errno: 150 "Foreign key constraint is incorrectly formed")
DROP TABLE t1;
# End of 10.5 tests
#
# MDEV-22602 Disable UPDATE CASCADE for SQL constraints
#
# TODO: enable them after MDEV-16417 is finished
create or replace table t1 (a int primary key) engine=innodb;
create or replace table t2 (a int, check(a > 0), foreign key(a) references t1(a) on update cascade) engine=innodb;
ERROR HY000: Function or expression 'a' cannot be used in the CHECK clause of `CONSTRAINT_1`
create or replace table t1 (f1 int, f2 date, f3 date, key(f1,f3,f2)) engine=innodb;
create or replace table t2 (
a int, s date, e date,
period for p (s, e),
primary key (a, p without overlaps),
foreign key (a, e, s) references t1 (f1, f3, f2) on delete cascade on update cascade) engine=innodb;
ERROR HY000: Key `PRIMARY` cannot have WITHOUT OVERLAPS
create or replace table t1 (a varchar(4096) unique) engine=innodb;
create or replace table t2 (pk int primary key, a varchar(4096) unique, foreign key(a) references t1(a) on update cascade) engine=innodb;
ERROR HY000: Can't create table `test`.`t2` (errno: 150 "Foreign key constraint is incorrectly formed")
drop table t1;
23 changes: 23 additions & 0 deletions mysql-test/suite/innodb/t/foreign_key.test
Original file line number Diff line number Diff line change
Expand Up @@ -741,4 +741,27 @@ DROP TABLE t1;

--echo # End of 10.5 tests

--echo #
--echo # MDEV-22602 Disable UPDATE CASCADE for SQL constraints
--echo #
--echo # TODO: enable them after MDEV-16417 is finished
create or replace table t1 (a int primary key) engine=innodb;
--error ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED
create or replace table t2 (a int, check(a > 0), foreign key(a) references t1(a) on update cascade) engine=innodb;

create or replace table t1 (f1 int, f2 date, f3 date, key(f1,f3,f2)) engine=innodb;
--error ER_KEY_CANT_HAVE_WITHOUT_OVERLAPS
create or replace table t2 (
a int, s date, e date,
period for p (s, e),
primary key (a, p without overlaps),
foreign key (a, e, s) references t1 (f1, f3, f2) on delete cascade on update cascade) engine=innodb;

# FK on long unique is already disabled
create or replace table t1 (a varchar(4096) unique) engine=innodb;
--error ER_CANT_CREATE_TABLE
create or replace table t2 (pk int primary key, a varchar(4096) unique, foreign key(a) references t1(a) on update cascade) engine=innodb;

drop table t1;

--source include/wait_until_count_sessions.inc
4 changes: 2 additions & 2 deletions sql/field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10191,11 +10191,12 @@ void Column_definition::create_length_to_internal_length_newdecimal()


bool check_expression(Virtual_column_info *vcol, const LEX_CSTRING *name,
enum_vcol_info_type type)
enum_vcol_info_type type, Alter_info *alter_info)

{
bool ret;
Item::vcol_func_processor_result res;
res.alter_info= alter_info;

if (!vcol->name.length)
vcol->name= *name;
Expand All @@ -10204,7 +10205,6 @@ bool check_expression(Virtual_column_info *vcol, const LEX_CSTRING *name,
Walk through the Item tree checking if all items are valid
to be part of the virtual column
*/
res.errors= 0;
ret= vcol->expr->walk(&Item::check_vcol_func_processor, 0, &res);
vcol->flags= res.errors;

Expand Down
3 changes: 2 additions & 1 deletion sql/field.h
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ static inline const char *vcol_type_name(enum_vcol_info_type type)
#define VCOL_AUTO_INC 16
#define VCOL_IMPOSSIBLE 32
#define VCOL_NOT_VIRTUAL 64 /* Function can't be virtual */
#define VCOL_CHECK_CONSTRAINT_IF_NOT_EXISTS 128

#define VCOL_NOT_STRICTLY_DETERMINISTIC \
(VCOL_NON_DETERMINISTIC | VCOL_TIME_FUNC | VCOL_SESSION_FUNC)
Expand Down Expand Up @@ -5719,7 +5720,7 @@ int set_field_to_null(Field *field);
int set_field_to_null_with_conversions(Field *field, bool no_conversions);
int convert_null_to_field_value_or_error(Field *field);
bool check_expression(Virtual_column_info *vcol, const LEX_CSTRING *name,
enum_vcol_info_type type);
enum_vcol_info_type type, Alter_info *alter_info= NULL);

/*
The following are for the interface with the .frm file
Expand Down
31 changes: 31 additions & 0 deletions sql/item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1493,6 +1493,37 @@ bool mark_unsupported_function(const char *w1, const char *w2,
}


bool Item_field::check_vcol_func_processor(void *arg)
{
context= 0;
vcol_func_processor_result *res= (vcol_func_processor_result *) arg;
if (res && res->alter_info)
{
for (Key &k: res->alter_info->key_list)
{
if (k.type != Key::FOREIGN_KEY)
continue;
Foreign_key *fk= (Foreign_key*) &k;
if (fk->update_opt != FK_OPTION_CASCADE)
continue;
for (Key_part_spec& kp: fk->columns)
{
if (!lex_string_cmp(system_charset_info, &kp.field_name, &field_name))
{
return mark_unsupported_function(field_name.str, arg, VCOL_IMPOSSIBLE);
}
}
}
}
if (field && (field->unireg_check == Field::NEXT_NUMBER))
{
// Auto increment fields are unsupported
return mark_unsupported_function(field_name.str, arg, VCOL_FIELD_REF | VCOL_AUTO_INC);
}
return mark_unsupported_function(field_name.str, arg, VCOL_FIELD_REF);
}


Query_fragment::Query_fragment(THD *thd, sp_head *sphead,
const char *start, const char *end)
{
Expand Down
16 changes: 5 additions & 11 deletions sql/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -2056,6 +2056,9 @@ class Item: public Value_source,
{
uint errors; /* Bits of possible errors */
const char *name; /* Not supported function */
Alter_info *alter_info;
vcol_func_processor_result() :
errors(0), name(NULL), alter_info(NULL) {}
};
struct func_processor_rename
{
Expand Down Expand Up @@ -3506,16 +3509,7 @@ class Item_field :public Item_ident,
bool switch_to_nullable_fields_processor(void *arg);
bool update_vcol_processor(void *arg);
bool rename_fields_processor(void *arg);
bool check_vcol_func_processor(void *arg)
{
context= 0;
if (field && (field->unireg_check == Field::NEXT_NUMBER))
{
// Auto increment fields are unsupported
return mark_unsupported_function(field_name.str, arg, VCOL_FIELD_REF | VCOL_AUTO_INC);
}
return mark_unsupported_function(field_name.str, arg, VCOL_FIELD_REF);
}
bool check_vcol_func_processor(void *arg);
bool set_fields_as_dependent_processor(void *arg)
{
if (!(used_tables() & OUTER_REF_TABLE_BIT))
Expand Down Expand Up @@ -6070,7 +6064,7 @@ class Item_copy :public Item,
table_map used_tables() const { return (table_map) 1L; }
bool const_item() const { return 0; }
bool is_null() { return null_value; }
bool check_vcol_func_processor(void *arg)
bool check_vcol_func_processor(void *arg)
{
return mark_unsupported_function("copy", arg, VCOL_IMPOSSIBLE);
}
Expand Down
5 changes: 0 additions & 5 deletions sql/sql_alter.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,6 @@ class Alter_info
List<Alter_rename_key> alter_rename_key_list;
// List of columns, used by both CREATE and ALTER TABLE.
List<Create_field> create_list;

enum flags_bits
{
CHECK_CONSTRAINT_IF_NOT_EXISTS= 1
};
List<Virtual_column_info> check_constraint_list;
// Type of ALTER TABLE operation.
alter_table_operations flags;
Expand Down
3 changes: 1 addition & 2 deletions sql/sql_lex.h
Original file line number Diff line number Diff line change
Expand Up @@ -4331,8 +4331,7 @@ struct LEX: public Query_tables_list
bool if_not_exists)
{
constr->name= name;
constr->flags= if_not_exists ?
Alter_info::CHECK_CONSTRAINT_IF_NOT_EXISTS : 0;
constr->flags= if_not_exists ? VCOL_CHECK_CONSTRAINT_IF_NOT_EXISTS : 0;
alter_info.check_constraint_list.push_back(constr);
return false;
}
Expand Down
30 changes: 28 additions & 2 deletions sql/sql_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4292,9 +4292,31 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
key_info->algorithm == HA_KEY_ALG_LONG_HASH)

{
without_overlaps_err:
my_error(ER_KEY_CANT_HAVE_WITHOUT_OVERLAPS, MYF(0), key_info->name.str);
DBUG_RETURN(true);
}
key_iterator2.rewind();
while ((key2 = key_iterator2++))
{
if (key2->type != Key::FOREIGN_KEY)
continue;
DBUG_ASSERT(key != key2);
Foreign_key *fk= (Foreign_key*) key2;
if (fk->update_opt != FK_OPTION_CASCADE)
continue;
for (Key_part_spec& kp: key->columns)
{
for (Key_part_spec& kp2: fk->columns)
{
if (!lex_string_cmp(system_charset_info, &kp.field_name,
&kp2.field_name))
{
goto without_overlaps_err;
}
}
}
}
create_info->period_info.unique_keys++;
}

Expand Down Expand Up @@ -4383,7 +4405,11 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
while ((check= c_it++))
{
if (!check->name.length || check->automatic_name)
{
if (check_expression(check, &check->name, VCOL_CHECK_TABLE, alter_info))
DBUG_RETURN(TRUE);
continue;
}

{
/* Check that there's no repeating table CHECK constraint names. */
Expand Down Expand Up @@ -5538,7 +5564,7 @@ static bool make_unique_constraint_name(THD *thd, LEX_CSTRING *name,
if (!check) // Found unique name
{
name->length= (size_t) (real_end - buff);
name->str= thd->strmake(buff, name->length);
name->str= strmake_root(thd->stmt_arena->mem_root, buff, name->length);
return (name->str == NULL);
}
}
Expand Down Expand Up @@ -6666,7 +6692,7 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info,

while ((check=it++))
{
if (!(check->flags & Alter_info::CHECK_CONSTRAINT_IF_NOT_EXISTS) &&
if (!(check->flags & VCOL_CHECK_CONSTRAINT_IF_NOT_EXISTS) &&
check->name.length)
continue;
check->flags= 0;
Expand Down
1 change: 0 additions & 1 deletion sql/table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3574,7 +3574,6 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table,
to be part of the virtual column
*/
Item::vcol_func_processor_result res;
res.errors= 0;

int error= func_expr->walk(&Item::check_vcol_func_processor, 0, &res);
if (unlikely(error || (res.errors & VCOL_IMPOSSIBLE)))
Expand Down

0 comments on commit 762bf7a

Please sign in to comment.