[MDEV-13851] Always check table options in ALTER TABLE…ALGORITHM=INPLACE Created: 2017-09-20  Updated: 2017-09-20  Resolved: 2017-09-20

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

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

Issue Links:
Problem/Incident
is caused by MDEV-11974 Disable encryption of spatial indexes... Closed
is caused by MDEV-13847 Allow ALTER TABLE…ADD SPATIAL INDEX…A... Closed

 Description   

The test encryption.innodb-spatial-index creates a table with ENCRYPTED=YES, and then expects an ADD SPATIAL INDEX, ALGORITHM=COPY operation to fail. However, an operation with FORCE, ALGORITHM=COPY is just fine (in both 10.1 and 10.2):

--source include/have_innodb.inc
 
--source include/have_file_key_management_plugin.inc
 
 
 
CREATE TABLE t1 (pk INT PRIMARY KEY AUTO_INCREMENT,
 
c VARCHAR(256), coordinate POINT NOT NULL) ENCRYPTED=YES ENGINE=INNODB;
 
ALTER TABLE t1 FORCE, ALGORITHM=COPY;
 
ALTER TABLE t1 FORCE, ALGORITHM=INPLACE;
 
DROP TABLE t1;

In my opinion, ALTER TABLE...ALGORITHM=COPY must always succeed, and it must always preserve those table attributes that were not specified in the ALTER.

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

MDEV-11974 disallows the creation of encrypted tables with SPATIAL INDEX, because encryption uses a field that is used for something else (SSN, split sequence number) in the InnoDB 5.7 R-tree index implementation.

So, the bug is not that ADD SPATIAL INDEX, ALGORITHM=COPY is refused, but it is that ADD SPATIAL INDEX, ALGORITHM=INPLACE is not refused when the table is encrypted.

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

This (so far untested) fix should take care of it:

diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
 
index 186665b8c8e..ee8e06ea19f 100644
 
--- a/storage/innobase/handler/handler0alter.cc
 
+++ b/storage/innobase/handler/handler0alter.cc
 
@@ -5606,10 +5606,7 @@ ha_innobase::prepare_inplace_alter_table(
 
 		/* The clustered index is corrupted. */
 
 		my_error(ER_CHECK_NO_SUCH_TABLE, MYF(0));
 
 		DBUG_RETURN(true);
 
-	}
 
-
 
-	if (ha_alter_info->handler_flags
 
-	    & Alter_inplace_info::CHANGE_CREATE_OPTION) {
 
+	} else {
 
 		const char* invalid_opt = info.create_options_are_invalid();
 
 
 		/* Check engine specific table options */

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

With the above patch, we do get one test failure:

CURRENT_TEST: innodb_zip.create_options
 
mysqltest: At line 252: query 'ALTER TABLE t1 ADD COLUMN f1 INT' failed: 1478: Table storage engine 'InnoDB' does not support the create option 'ROW_FORMAT'

I think that this is the proper thing to do. The context:

CREATE TABLE t1 ( i INT ) ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=4;
 
SET GLOBAL innodb_file_format=Antelope;
 
ALTER TABLE t1 ADD COLUMN f1 INT;

This would even force a table rebuild, creating a new .ibd file.
However, the create options would be validated for any form of ALTER TABLE, such as renaming columns or changing default values. That may feel a bit excessive.

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