[MDEV-13847] Allow ALTER TABLE…ADD SPATIAL INDEX…ALGORITHM=INPLACE Created: 2017-09-19  Updated: 2017-09-29  Resolved: 2017-09-20

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.2.2
Fix Version/s: 10.2.9

Type: Bug Priority: Major
Reporter: Marko Mäkelä Assignee: Marko Mäkelä
Resolution: Fixed Votes: 0
Labels: compat57

Issue Links:
Problem/Incident
causes MDEV-13851 Always check table options in ALTER T... Closed
Relates
relates to MDEV-13923 Assertion `!is_set() || (m_status == ... Closed

 Description   

In the merge of MySQL 5.7.9 to MariaDB 10.2.2, some code was included that prevents ADD SPATIAL INDEX from being executed with ALGORITHM=INPLACE.

Restrictions introduced in MySQL 5.7.9 are unavoidable, but MariaDB should not add its own ones. The MySQL restrictions are:

  1. table-rebuilding operations are not allowed if SPATIAL INDEX survive it
  2. ALTER TABLE…ADD SPATIAL INDEX…LOCK=NONE is not allowed

The fix appears simple:

diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
 
index 352b223ab69..d52eaf456a4 100644
 
--- a/storage/innobase/handler/handler0alter.cc
 
+++ b/storage/innobase/handler/handler0alter.cc
 
@@ -897,28 +897,12 @@ ha_innobase::check_if_supported_inplace_alter(
 
 	DBUG_ASSERT(!m_prebuilt->table->fts || m_prebuilt->table->fts->doc_col
 
 		    < dict_table_get_n_user_cols(m_prebuilt->table));
 
 
-       /* Spatial indexes should use copy method for now.
 
-       TOO: remove this when below ADD_SPATIAL_INDEX supported. */
 
-       for (uint i = 0; i < ha_alter_info->index_add_count; i++) {
 
-               const KEY* key =
 
-                       &ha_alter_info->key_info_buffer[
 
-                               ha_alter_info->index_add_buffer[i]];
 
-               if (key->flags & HA_SPATIAL) {
 
-                       ha_alter_info->unsupported_reason = innobase_get_err_msg(
 
-                             ER_ALTER_OPERATION_NOT_SUPPORTED_REASON_GIS);
 
-
 
-                       DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
 
-               }
 
-       }
 
-
 
-#ifdef MYSQL_SPATIAL_INDEX
 
 	if (ha_alter_info->handler_flags
 
 	    & Alter_inplace_info::ADD_SPATIAL_INDEX) {
 
 		ha_alter_info->unsupported_reason = innobase_get_err_msg(
 
 			ER_ALTER_OPERATION_NOT_SUPPORTED_REASON_GIS);
 
 		online = false;
 
 	}
 
-#endif
 
 
 	if (m_prebuilt->table->fts
 
 	    && innobase_fulltext_exist(altered_table)) {
 
diff --git a/storage/innobase/include/ha_prototypes.h b/storage/innobase/include/ha_prototypes.h
 
index 7b3726b1fef..459304fc712 100644
 
--- a/storage/innobase/include/ha_prototypes.h
 
+++ b/storage/innobase/include/ha_prototypes.h
 
@@ -43,7 +43,6 @@ class THD;
 
 #undef MYSQL_PFS
 
 #undef MYSQL_RENAME_INDEX
 
 #undef MYSQL_REPLACE_TRX_IN_THD
 
-#undef MYSQL_SPATIAL_INDEX
 
 #undef MYSQL_STORE_FTS_DOC_ID



 Comments   
Comment by Marko Mäkelä [ 2017-09-19 ]

jplindst, please take this initial patch and see which other tests need to be adjusted and how.

Comment by Marko Mäkelä [ 2017-09-20 ]

We also have to remove the misleading constant Alter_inplace_info::ADD_SPATIAL_INDEX, which was an alias of Alter_inplace_info::ADD_INDEX.
There are not many references to ADD_SPATIAL_INDEX; it is trivial to remove. So, ADD_INDEX will refer to creating any kind of creating and index (except ADD PRIMARY KEY).
Already in MySQL 5.6, ADD_INDEX could refer to creating a FULLTEXT INDEX. MySQL 5.7 should not have introduced the constant ADD_SPATIAL_INDEX for one special case that is more like an implementation detail of a storage engine.

Comment by Marko Mäkelä [ 2017-09-20 ]

bb-10.2-marko

Comment by Jan Lindström (Inactive) [ 2017-09-20 ]

ok to push.

Generated at Thu Feb 08 08:08:51 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.