Re: 331430a6a73: MDEV-33658 2/2 Cannot add a foreign key on a table with a matching long UNIQUE
Hi, Nikita, On Jan 01, Nikita Malyavin wrote:
diff --git a/sql/sql_class.cc b/sql/sql_class.cc index de688b4cb31..2ffceb3932b 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -217,6 +218,16 @@ Foreign_key::Foreign_key(const Foreign_key &rhs, MEM_ROOT *mem_root)
bool is_foreign_key_prefix(Key *a, Key *b) { + ha_key_alg a_alg= a->key_create_info.algorithm; + ha_key_alg b_alg= b->key_create_info.algorithm; + + // The real algorithm will be BTREE is none was given by user.
Technically, this is not correct, the algorithm will be whatever the engine wants. For MEMORY engine it'll be HASH. But only InnoDB can do foreign keys now, and for InnoDB th3 default key algorithm is BTREE. Let's reflect it in the comment, like, for example (also fixed a typo "is" -> "if") // The real algorithm in InnoDB will be BTREE if none was given by user
+ a_alg= a_alg == HA_KEY_ALG_UNDEF ? HA_KEY_ALG_BTREE : a_alg; + b_alg= b_alg == HA_KEY_ALG_UNDEF ? HA_KEY_ALG_BTREE : b_alg; + + if (a_alg != b_alg) + return false; + /* Ensure that 'a' is the generated key */ if (a->generated) { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 3f035defbd3..d7aeb5d7cbf 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3556,6 +3556,303 @@ key_add_part_check_null(const handler *file, KEY *key_info, }
+static +my_bool key_check_without_overlaps(THD *thd, HA_CREATE_INFO *create_info, + Alter_info *alter_info, + Key &key) +{ + DBUG_ENTER("key_check_without_overlaps"); + if (!key.without_overlaps) + DBUG_RETURN(FALSE); + // append_system_key_parts is already called, so we should check all the + // columns except the last two. + const auto &period_start= create_info->period_info.period.start; + const auto &period_end= create_info->period_info.period.end; + List_iterator<Key_part_spec> part_it_forwarded(key.columns); + List_iterator<Key_part_spec> part_it(key.columns); + part_it_forwarded++; + part_it_forwarded++; + while (part_it_forwarded++) + { + Key_part_spec *key_part= part_it++; + + if (period_start.streq(key_part->field_name) + || period_end.streq(key_part->field_name)) + { + my_error(ER_KEY_CONTAINS_PERIOD_FIELDS, MYF(0), key.name.str, + key_part->field_name.str); + DBUG_RETURN(TRUE); + } + }
It was simpler to have this check inside append_system_key_parts(), why did you move it?
+ + if (key.key_create_info.algorithm == HA_KEY_ALG_HASH || + key.key_create_info.algorithm == HA_KEY_ALG_LONG_HASH) + + { + my_error(ER_KEY_CANT_HAVE_WITHOUT_OVERLAPS, MYF(0), key.name.str); + DBUG_RETURN(TRUE); + } <...> + KEY_CREATE_INFO *key_cinfo= &key.key_create_info; + + if (is_hash_field_needed) + { + if (key_cinfo->algorithm == HA_KEY_ALG_UNDEF) + key_cinfo->algorithm= HA_KEY_ALG_LONG_HASH;
Perhaps you shouldn't set key algorithm in add_hash_field() then? or, rather replace the assignment with an assert?
+ + if (key_cinfo->algorithm != HA_KEY_ALG_HASH && + key_cinfo->algorithm != HA_KEY_ALG_LONG_HASH) + { + my_error(ER_TOO_LONG_KEY, MYF(0), max_key_length); + DBUG_RETURN(TRUE); + } + } + } + } + DBUG_RETURN(FALSE); +} + + /* Preparation for table creation
Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org
participants (1)
-
Sergei Golubchik