I fixed a few more bits , commit is https://github.com/MariaDB/server/commit/e7d1cce05f154936aefe18bec4b9a9eb376... <http://e7d1cce05f15> On Wed, 5 Jul 2023 at 20:54, Nikita Malyavin <nikitamalyavin@gmail.com> wrote:
On Sat, 24 Jun 2023 at 18:51, Sergei Golubchik <serg@mariadb.org> wrote:
@@ -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 b16f52177a84 <https://github.com/MariaDB/server/commit/b16f52177a84ae6c3b1fe4ca40c879f6c0dab270> Also agree with all the further following statements.
-- Yours truly, Nikita Malyavin
-- Yours truly, Nikita Malyavin