database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-07-30 16:24:05 +03:00
Code Activity

MDEV-22506 Malformed error message for ER_KEY_CONTAINS_PERIOD_FIELDS

Browse Source 
Though this is an error message task, the problem was deep in the
`mysql_prepare_create_table` implementation. The problem is described as
follows:

1. `append_system_key_parts` was called before
`mysql_prepare_create_table`, though key name generation was done close to
the latest stage of the latter.
2. We can't move `append_system_key_parts` in the end, because system keys
should be appended before some checks done.
3. If the checks from `append_system_key_parts` are moved to the end of
`mysql_prepare_create_table`, then some other inappropriate errors are
issued. like `ER_DUP_FIELDNAME`.

To have key name specified in error message, name generation should be done
before the checks, which consequenced in more changes.

The final design for key initialization in `mysql_prepare_create_table`
follows. The initialization is done in three phases:
1. Calculate a total number of keys created with respect to keys ignored.
 Allocate KEY* buffer.
2. Generate unique names; calculate a total number of key parts.
 Make early checks. Allocate KEY_PART_INFO* buffer.
3. Initialize key parts, make the rest of the checks.
This commit is contained in:
Nikita Malyavin
2020-10-20 15:21:28 +10:00
parent a79c6e369e
commit e618f7e9f6
2 changed files with 110 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

200
sql/sql_table.cc
View File

@ -75,9 +75,8 @@ static int copy_data_between_tables(THD *, TABLE *,TABLE *,
ha_rows *, ha_rows *,
Alter_info::enum_enable_or_disable,
Alter_table_ctx *);
static bool append_system_key_parts(THD *thd, HA_CREATE_INFO *create_info,
Alter_info *alter_info, KEY **key_info,
uint key_count);
static int append_system_key_parts(THD *thd, HA_CREATE_INFO *create_info,
Key *key);
static int mysql_prepare_create_table(THD *, HA_CREATE_INFO *, Alter_info *,
uint *, handler *, KEY **, uint *, int);
static uint blob_length_by_type(enum_field_types type);
@ -1823,10 +1822,6 @@ bool mysql_write_frm(ALTER_PARTITION_PARAM_TYPE *lpt, uint flags)
strxmov(shadow_frm_name, shadow_path, reg_ext, NullS);
if (flags & WFRM_WRITE_SHADOW)
{
if (append_system_key_parts(lpt->thd, lpt->create_info, lpt->alter_info,
&lpt->key_info_buffer, 0))
DBUG_RETURN(true);
if (mysql_prepare_create_table(lpt->thd, lpt->create_info, lpt->alter_info,
&lpt->db_options, lpt->table->file,
&lpt->key_info_buffer, &lpt->key_count,
@ -3557,7 +3552,6 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
Create_field *sql_field,*dup_field;
uint field,null_fields,max_key_length;
ulong record_offset= 0;
KEY *key_info;
KEY_PART_INFO *key_part_info;
int field_no,dup_no;
int select_field_pos,auto_increment=0;
@ -3875,6 +3869,57 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
thd->abort_on_warning= sav_abort_on_warning;
}
}
KEY *key_info= *key_info_buffer= (KEY*)thd->calloc(sizeof(KEY) * (*key_count));
if (!*key_info_buffer)
DBUG_RETURN(true); // Out of memory
key_iterator.rewind();
while ((key=key_iterator++))
{
if (key->name.str == ignore_key || key->type == Key::FOREIGN_KEY)
continue;
/* Create the key->ame based on the first column (if not given) */
if (key->type == Key::PRIMARY)
{
if (primary_key)
{
my_message(ER_MULTIPLE_PRI_KEY, ER_THD(thd, ER_MULTIPLE_PRI_KEY),
MYF(0));
DBUG_RETURN(true);
}
key_name=primary_key_name;
primary_key=1;
}
else if (!(key_name= key->name.str))
{
auto field_name= key->columns.elem(0)->field_name;
it.rewind();
while ((sql_field=it++) &&
lex_string_cmp(system_charset_info,
&field_name,
&sql_field->field_name));
if (sql_field)
field_name= sql_field->field_name;
key_name=make_unique_key_name(thd, field_name.str,
*key_info_buffer, key_info);
}
if (check_if_keyname_exists(key_name, *key_info_buffer, key_info))
{
my_error(ER_DUP_KEYNAME, MYF(0), key_name);
DBUG_RETURN(true);
}
key_info->name.str= (char*) key_name;
key_info->name.length= strlen(key_name);
key->name= key_info->name;
int parts_added= append_system_key_parts(thd, create_info, key);
if (parts_added < 0)
DBUG_RETURN(true);
key_parts += parts_added;
key_info++;
}
tmp=file->max_keys();
if (*key_count > tmp)
{
@ -3882,11 +3927,11 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
DBUG_RETURN(TRUE);
}
(*key_info_buffer)= key_info= (KEY*) thd->calloc(sizeof(KEY) * (*key_count));
key_part_info=(KEY_PART_INFO*) thd->calloc(sizeof(KEY_PART_INFO)*key_parts);
if (!*key_info_buffer || ! key_part_info)
DBUG_RETURN(TRUE); // Out of memory
if (!key_part_info)
DBUG_RETURN(true); // Out of memory
key_info= *key_info_buffer;
key_iterator.rewind();
key_number=0;
for (; (key=key_iterator++) ; key_number++)
@ -4257,32 +4302,6 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
key_length+= key_part_length;
key_part_info++;
/* Create the key name based on the first column (if not given) */
if (column_nr == 0)
{
if (key->type == Key::PRIMARY)
{
if (primary_key)
{
my_message(ER_MULTIPLE_PRI_KEY, ER_THD(thd, ER_MULTIPLE_PRI_KEY),
MYF(0));
DBUG_RETURN(TRUE);
}
key_name=primary_key_name;
primary_key=1;
}
else if (!(key_name= key->name.str))
key_name=make_unique_key_name(thd, sql_field->field_name.str,
*key_info_buffer, key_info);
if (check_if_keyname_exists(key_name, *key_info_buffer, key_info))
{
my_error(ER_DUP_KEYNAME, MYF(0), key_name);
DBUG_RETURN(TRUE);
}
key_info->name.str= (char*) key_name;
key_info->name.length= strlen(key_name);
}
}
if (!key_info->name.str || check_column_name(key_info->name.str))
{
@ -4715,72 +4734,75 @@ bool Column_definition::sp_prepare_create_field(THD *thd, MEM_ROOT *mem_root)
}
static bool append_system_key_parts(THD *thd, HA_CREATE_INFO *create_info,
Alter_info *alter_info, KEY **key_info,
uint key_count)
/**
Appends key parts generated by mariadb server.
Adds row_end in UNIQUE keys for system versioning,
and period fields for WITHOUT OVERLAPS.
@param thd Thread data
@param create_info Table create info
@param key Parsed key
@return a number of key parts added to key.
*/
static int append_system_key_parts(THD *thd, HA_CREATE_INFO *create_info,
Key *key)
{
const Lex_ident &row_start_field= create_info->vers_info.as_row.start;
const Lex_ident &row_end_field= create_info->vers_info.as_row.end;
DBUG_ASSERT(!create_info->versioned() || (row_start_field && row_end_field));
List_iterator<Key> key_it(alter_info->key_list);
Key *key= NULL;
if (create_info->versioned())
int result = 0;
if (create_info->versioned() && (key->type == Key::PRIMARY
|| key->type == Key::UNIQUE))
{
while ((key=key_it++))
Key_part_spec *key_part=NULL;
List_iterator<Key_part_spec> part_it(key->columns);
while ((key_part=part_it++))
{
if (key->type != Key::PRIMARY && key->type != Key::UNIQUE)
continue;
Key_part_spec *key_part=NULL;
List_iterator<Key_part_spec> part_it(key->columns);
while ((key_part=part_it++))
{
if (row_start_field.streq(key_part->field_name) ||
row_end_field.streq(key_part->field_name))
break;
}
if (!key_part)
key->columns.push_back(new (thd->mem_root)
if (row_start_field.streq(key_part->field_name) ||
row_end_field.streq(key_part->field_name))
break;
}
if (!key_part)
{
key->columns.push_back(new (thd->mem_root)
Key_part_spec(&row_end_field, 0, true));
result++;
}
key_it.rewind();
}
while ((key=key_it++))
if (key->without_overlaps)
{
if (key->without_overlaps)
DBUG_ASSERT(key->type == Key::PRIMARY || key->type == Key::UNIQUE);
if (!create_info->period_info.is_set()
|| !key->period.streq(create_info->period_info.name))
{
DBUG_ASSERT(key->type == Key::PRIMARY || key->type == Key::UNIQUE);
if (!create_info->period_info.is_set()
|| !key->period.streq(create_info->period_info.name))
{
my_error(ER_PERIOD_NOT_FOUND, MYF(0), key->period.str);
return true;
}
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(key->columns);
while (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);
return true;
}
}
key->columns.push_back(new (thd->mem_root)
Key_part_spec(&period_end, 0));
key->columns.push_back(new (thd->mem_root)
Key_part_spec(&period_start, 0));
my_error(ER_PERIOD_NOT_FOUND, MYF(0), key->period.str);
return -1;
}
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(key->columns);
while (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);
return -1;
}
}
const auto &period= create_info->period_info.period;
key->columns.push_back(new (thd->mem_root)
Key_part_spec(&period.end, 0, true));
key->columns.push_back(new (thd->mem_root)
Key_part_spec(&period.start, 0, true));
result += 2;
}
return false;
return result;
}
handler *mysql_create_frm_image(THD *thd, const LEX_CSTRING &db,
@ -5018,10 +5040,6 @@ handler *mysql_create_frm_image(THD *thd, const LEX_CSTRING &db,
}
#endif
if (append_system_key_parts(thd, create_info, alter_info, key_info,
*key_count))
goto err;
if (mysql_prepare_create_table(thd, create_info, alter_info, &db_options,
file, key_info, key_count, create_table_mode))
goto err;