Hi, Thiru! Thanks! This is, indeed, much easier to follow. Nicely done. Ok to push. On Jan 23, Thirunarayanan Balathandayuthapani wrote:

revision-id: c8248ca9a22 (mariadb-10.0.37-45-gc8248ca9a22) parent(s): 12f362c3338 author: Thirunarayanan Balathandayuthapani <thiru@mariadb.com> committer: Thirunarayanan Balathandayuthapani <thiru@mariadb.com> timestamp: 2019-01-18 12:01:18 +0530 message:

MDEV-17376 Server fails to set ADD_PK_INDEX, DROP_PK_INDEX if unique index nominated as PK

diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 2302026b18b..645746a7ab0 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6513,6 +6519,17 @@ static bool fill_alter_inplace_info(THD *thd, new_field->field->field_index != key_part->fieldnr - 1) goto index_changed; } + + /* + Rebuild the index if following condition get satisfied: + + (i) Old table doesn't have primary key, new table has it and vice-versa + (ii) Primary key changed to another existing index + */ + if ((new_key == new_pk) != + (table_key == &table->key_info[table->s->primary_key]))

I would suggest to rewrite the condition as table_key - table->key_info == table->s->primary_key it means the same thing, but doesn't confuse the reader with a question "what's with a buffer overrun if table->s->primary_key == MAX_KEY?"

+ goto index_changed; + continue;

index_changed:

Regards, Sergei Chief Architect MariaDB and security@mariadb.org