> @@ -1228,6 +1228,15 @@ class Query_arena
> { return strdup_root(mem_root,str); }
> inline char *strmake(const char *str, size_t size) const
> { return strmake_root(mem_root,str,size); }
> + inline LEX_CSTRING strcat(const LEX_CSTRING &a, const LEX_CSTRING &b) const
> + {
> + char *buf= (char*)alloc(a.length + b.length + 1);
> + if (unlikely(!buf))
> + return null_clex_str;
> + strncpy(buf, a.str, a.length);
memcpy both ↑ and ↓ (and then you might need to append \0 explicitly)
Ok, ↑ will be faster, but why ↓?
> + strncpy(buf + a.length, b.str, b.length + 1);
> + return {buf, a.length + b.length};
> + }
> inline void *memdup(const void *str, size_t size) const
> { return memdup_root(mem_root,str,size); }
> inline void *memdup_w_gap(const void *str, size_t size, size_t gap) const
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -9869,26 +9869,21 @@ bool online_alter_check_autoinc(const THD *thd, const Alter_info *alter_info,
> */
> for (uint k= 0; k < table->s->keys; k++)
> {
> - const KEY &key= table->s->key_info[k];
> + const KEY &key= table->key_info[k];
This is generally incorrect. User specified keys are in table->s->key_info[]
Keys in table->key_info[] can be modified to match what indexes are actually
created, in particular, for long uniques table->key_info[] differs from
table->s->key_info[]
Maybe, but hopefully, long unique is not HA_NOSAME.
I need table->key[i].key_part[j].field to compare with copy_field.field.
> @@ -9896,25 +9891,63 @@ bool online_alter_check_autoinc(const THD *thd, const Alter_info *alter_info,
> return true;
> }
>
> - const char *old_autoinc= table->found_next_number_field
> - ? table->found_next_number_field->field_name.str
> - : "";
> - bool online= true;
> for (const auto &c: alter_info->create_list)
> {
> - if (c.change.str && c.flags & AUTO_INCREMENT_FLAG)
> + if (c.flags & AUTO_INCREMENT_FLAG)
> {
> - if (my_strcasecmp(system_charset_info, c.change.str, old_autoinc) != 0)
> + if (c.field && !(c.field->flags & AUTO_INCREMENT_FLAG))
> + return false;
> + break;
> + }
> + }
> + return true;
> +}
> +
> +static
> +const char *online_alter_check_supported(const THD *thd,
> + const Alter_info *alter_info,
> + const TABLE *table, bool *online)
> {
> - if (c.create_if_not_exists // check IF EXISTS option
> - && table->find_field_by_name(&c.change) == NULL)
> - continue;
> - online= false;
> + DBUG_ASSERT(!table->s->tmp_table);
that's confusing. *online can be false here already,
then return "DROP SYSTEM VERSIONING" will be wrong
and you use some other condition to decide whether
"DROP SYSTEM VERSIONING" is correct or not.
it'd be much clearer not to call online_alter_check_supported in this case.
and add here DBUG_ASSERT(*online);
Yes, and also some tests failed around this during rebase. I've fixed it in the new patch
b16f52177a84Also agree with all the further following statements.