-
Dyre Tjeldvoll authored
Bug#19635706 `!!NEW_CLUSTERED == (INNOBASE_NEED_REBUILD(HA_ALTER_INFO) || ADD_FTS_DOC_ID)' Problem: Adding a unique index to a NOT NULL POINT column would trigger a warning and the key would not be promoted. Creating a unique index on a different NON-NULL column in the same table would then trigger an assert. The issue was introduced in the fix for Bug 17665767 which added special handling of unique indices on NON NULL POINT columns when reading frm-files, which ensured that such indices were promoted to primary keys. Since TABLE_SHARE::primary_key is set when reading the frm file, it was set to the index of the POINT column. But such columns were not excluded when setting the HA_KEY_HAS_PART_KEY_SEG bitfield, and since this is used by ALTER TABLE (sql and innodb) to determine if a key is promotable, the result was an inconsistency. Specifically, the warning was issued because TABLE_SHARE::primary_key was different from MAX_KEY when no key had been promoted. On the second alter the sql layer trusts TABLE_SHARE::primary_key (via candidate_key) to imply that the table already has a primary key, and so does not set Alter_inplace_info::ADD_PK_INDEX. This then triggers an assert in Innodb which has created a new clustered index, but sees innobase_need_rebuild(ha_alter_info) returning false (because Alter_inplace_info::ADD_UNIQUE_INDEX is not covered by the INNOBASE_ALTER_REBUILD mask, but Alter_inplace_info::ADD_PK_INDEX is). Solution: Use the algorithm from frm reading also when setting the HA_KEY_HAS_PART_KEY_SEG bit field, i.e. do not consider keys on POINT columns as partial. Note! This is not a good(tm) solution as too many code fragments need to be kept in sync manually. A better solution would be remove the dangerous non-persistent HA_KEY_HAS_PART_KEY_SEG bit field and create an is_key_partial() predicate which could provide this info from persisted data. Unfortunately, it looks like this approach will require more refactoring than what is reasonable, now that we're on the verge of getting a new data dictionary. (cherry picked from commit 2a3907076385512b1c4dbd8e2c0d21739031c573)
Dyre Tjeldvoll authoredBug#19635706 `!!NEW_CLUSTERED == (INNOBASE_NEED_REBUILD(HA_ALTER_INFO) || ADD_FTS_DOC_ID)' Problem: Adding a unique index to a NOT NULL POINT column would trigger a warning and the key would not be promoted. Creating a unique index on a different NON-NULL column in the same table would then trigger an assert. The issue was introduced in the fix for Bug 17665767 which added special handling of unique indices on NON NULL POINT columns when reading frm-files, which ensured that such indices were promoted to primary keys. Since TABLE_SHARE::primary_key is set when reading the frm file, it was set to the index of the POINT column. But such columns were not excluded when setting the HA_KEY_HAS_PART_KEY_SEG bitfield, and since this is used by ALTER TABLE (sql and innodb) to determine if a key is promotable, the result was an inconsistency. Specifically, the warning was issued because TABLE_SHARE::primary_key was different from MAX_KEY when no key had been promoted. On the second alter the sql layer trusts TABLE_SHARE::primary_key (via candidate_key) to imply that the table already has a primary key, and so does not set Alter_inplace_info::ADD_PK_INDEX. This then triggers an assert in Innodb which has created a new clustered index, but sees innobase_need_rebuild(ha_alter_info) returning false (because Alter_inplace_info::ADD_UNIQUE_INDEX is not covered by the INNOBASE_ALTER_REBUILD mask, but Alter_inplace_info::ADD_PK_INDEX is). Solution: Use the algorithm from frm reading also when setting the HA_KEY_HAS_PART_KEY_SEG bit field, i.e. do not consider keys on POINT columns as partial. Note! This is not a good(tm) solution as too many code fragments need to be kept in sync manually. A better solution would be remove the dangerous non-persistent HA_KEY_HAS_PART_KEY_SEG bit field and create an is_key_partial() predicate which could provide this info from persisted data. Unfortunately, it looks like this approach will require more refactoring than what is reasonable, now that we're on the verge of getting a new data dictionary. (cherry picked from commit 2a3907076385512b1c4dbd8e2c0d21739031c573)
Loading