MDEV-16334 Incorrect ALTER TABLE for changing column option

commit 2dbeebdb16436e3c2723cd483aaf21d93de799d6 accidentally changed
ALTER_COLUMN_OPTION and ALTER_COLUMN_STORAGE_TYPE to be separate flags.
InnoDB and Mroonga are only checking for the latter;
the example storage engine is checking for the former only.

The impact of this bug should be incorrect operation of Mroonga when
the column options GROONGA_TYPE, FLAGS are changed.

InnoDB does not define any column options, only table options,
so the flag ALTER_COLUMN_OPTION should never have been set.

Also, remove the unused flag ALTER_DROP_HISTORICAL.

4 files changed, 32 insertions, 41 deletions

diff --git a/sql/handler.h b/sql/handler.h
index 648be551f5e..3d1b764bd14 100644
--- a/sql/handler.h
+++ b/sql/handler.h
@@ -631,43 +631,41 @@ typedef ulonglong alter_table_operations;
                                        ALTER_DROP_STORED_COLUMN)
 #define ALTER_COLUMN_DEFAULT          ALTER_CHANGE_COLUMN_DEFAULT
 
-#define ALTER_DROP_HISTORICAL        (1ULL << 35)
-
 // Add non-unique, non-primary index
-#define ALTER_ADD_NON_UNIQUE_NON_PRIM_INDEX  (1ULL << 36)
+#define ALTER_ADD_NON_UNIQUE_NON_PRIM_INDEX  (1ULL << 35)
 
 // Drop non-unique, non-primary index
-#define ALTER_DROP_NON_UNIQUE_NON_PRIM_INDEX (1ULL << 37)
+#define ALTER_DROP_NON_UNIQUE_NON_PRIM_INDEX (1ULL << 36)
 
 // Add unique, non-primary index
-#define ALTER_ADD_UNIQUE_INDEX               (1ULL << 38)
+#define ALTER_ADD_UNIQUE_INDEX               (1ULL << 37)
 
 // Drop unique, non-primary index
-#define ALTER_DROP_UNIQUE_INDEX              (1ULL << 39)
+#define ALTER_DROP_UNIQUE_INDEX              (1ULL << 38)
 
 // Add primary index
-#define ALTER_ADD_PK_INDEX                   (1ULL << 40)
+#define ALTER_ADD_PK_INDEX                   (1ULL << 39)
 
 // Drop primary index
-#define ALTER_DROP_PK_INDEX                  (1ULL << 41)
+#define ALTER_DROP_PK_INDEX                  (1ULL << 40)
 
 // Virtual generated column
-#define ALTER_ADD_VIRTUAL_COLUMN             (1ULL << 42)
+#define ALTER_ADD_VIRTUAL_COLUMN             (1ULL << 41)
 // Stored base (non-generated) column
-#define ALTER_ADD_STORED_BASE_COLUMN         (1ULL << 43)
+#define ALTER_ADD_STORED_BASE_COLUMN         (1ULL << 42)
 // Stored generated column
-#define ALTER_ADD_STORED_GENERATED_COLUMN    (1ULL << 44)
+#define ALTER_ADD_STORED_GENERATED_COLUMN    (1ULL << 43)
 
 // Drop column
-#define ALTER_DROP_VIRTUAL_COLUMN            (1ULL << 45)
-#define ALTER_DROP_STORED_COLUMN             (1ULL << 46)
+#define ALTER_DROP_VIRTUAL_COLUMN            (1ULL << 44)
+#define ALTER_DROP_STORED_COLUMN             (1ULL << 45)
 
 // Rename column (verified; ALTER_RENAME_COLUMN may use original name)
-#define ALTER_COLUMN_NAME          	     (1ULL << 47)
+#define ALTER_COLUMN_NAME          	     (1ULL << 46)
 
 // Change column datatype
-#define ALTER_VIRTUAL_COLUMN_TYPE            (1ULL << 48)
-#define ALTER_STORED_COLUMN_TYPE             (1ULL << 49)
+#define ALTER_VIRTUAL_COLUMN_TYPE            (1ULL << 47)
+#define ALTER_STORED_COLUMN_TYPE             (1ULL << 48)
 
 /**
   Change column datatype in such way that new type has compatible
@@ -675,45 +673,45 @@ typedef ulonglong alter_table_operations;
   possible to perform change by only updating data dictionary
   without changing table rows.
 */
-#define ALTER_COLUMN_EQUAL_PACK_LENGTH       (1ULL << 50)
+#define ALTER_COLUMN_EQUAL_PACK_LENGTH       (1ULL << 49)
 
 // Reorder column
-#define ALTER_STORED_COLUMN_ORDER            (1ULL << 51)
+#define ALTER_STORED_COLUMN_ORDER            (1ULL << 50)
 
 // Reorder column
-#define ALTER_VIRTUAL_COLUMN_ORDER           (1ULL << 52)
+#define ALTER_VIRTUAL_COLUMN_ORDER           (1ULL << 51)
 
 // Change column from NOT NULL to NULL
-#define ALTER_COLUMN_NULLABLE                (1ULL << 53)
+#define ALTER_COLUMN_NULLABLE                (1ULL << 52)
 
 // Change column from NULL to NOT NULL
-#define ALTER_COLUMN_NOT_NULLABLE            (1ULL << 54)
+#define ALTER_COLUMN_NOT_NULLABLE            (1ULL << 53)
 
 // Change column generation expression
-#define ALTER_VIRTUAL_GCOL_EXPR              (1ULL << 55)
-#define ALTER_STORED_GCOL_EXPR               (1ULL << 56)
+#define ALTER_VIRTUAL_GCOL_EXPR              (1ULL << 54)
+#define ALTER_STORED_GCOL_EXPR               (1ULL << 55)
 
 // column's engine options changed, something in field->option_struct
-#define ALTER_COLUMN_OPTION                  (1ULL << 57)
+#define ALTER_COLUMN_OPTION                  (1ULL << 56)
 
 // MySQL alias for the same thing:
-#define ALTER_COLUMN_STORAGE_TYPE            (1ULL << 58)
+#define ALTER_COLUMN_STORAGE_TYPE            ALTER_COLUMN_OPTION
 
 // Change the column format of column
-#define ALTER_COLUMN_COLUMN_FORMAT           (1ULL << 59)
+#define ALTER_COLUMN_COLUMN_FORMAT           (1ULL << 57)
 
 /**
   Changes in generated columns that affect storage,
   for example, when a vcol type or expression changes
   and this vcol is indexed or used in a partitioning expression
 */
-#define ALTER_COLUMN_VCOL                    (1ULL << 60)
+#define ALTER_COLUMN_VCOL                    (1ULL << 58)
 
 /**
   ALTER TABLE for a partitioned table. The engine needs to commit
   online alter of all partitions atomically (using group_commit_ctx)
 */
-#define ALTER_PARTITIONED                    (1ULL << 61)
+#define ALTER_PARTITIONED                    (1ULL << 59)
 
 /*
   Flags set in partition_flags when altering partitions
diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
index de0e587aa4d..21857a8c043 100644
--- a/storage/innobase/handler/handler0alter.cc
+++ b/storage/innobase/handler/handler0alter.cc
@@ -99,7 +99,6 @@ static const alter_table_operations INNOBASE_ALTER_DATA
 /** Operations for altering a table that InnoDB does not care about */
 static const alter_table_operations INNOBASE_INPLACE_IGNORE
 	= ALTER_COLUMN_DEFAULT
-	| ALTER_CHANGE_COLUMN_DEFAULT
 	| ALTER_PARTITIONED
 	| ALTER_COLUMN_COLUMN_FORMAT
 	| ALTER_COLUMN_STORAGE_TYPE
@@ -7205,8 +7204,7 @@ ok_exit:
 		ctx->add_index, ctx->add_key_numbers, ctx->num_to_add_index,
 		altered_table, ctx->defaults, ctx->col_map,
 		ctx->add_autoinc, ctx->sequence, ctx->skip_pk_sort,
-		ctx->m_stage, add_v, eval_table,
-		ha_alter_info->handler_flags & ALTER_DROP_HISTORICAL);
+		ctx->m_stage, add_v, eval_table);
 
 #ifndef DBUG_OFF
 oom:
diff --git a/storage/innobase/include/row0merge.h b/storage/innobase/include/row0merge.h
index f808da1863e..7330031c186 100644
--- a/storage/innobase/include/row0merge.h
+++ b/storage/innobase/include/row0merge.h
@@ -322,7 +322,6 @@ this function and it will be passed to other functions for further accounting.
 @param[in]	add_v		new virtual columns added along with indexes
 @param[in]	eval_table	mysql table used to evaluate virtual column
 				value, see innobase_get_computed_value().
-@param[in]	drop_historical	whether to drop historical system rows
 @return DB_SUCCESS or error code */
 dberr_t
 row_merge_build_indexes(
@@ -341,8 +340,7 @@ row_merge_build_indexes(
 	bool			skip_pk_sort,
 	ut_stage_alter_t*	stage,
 	const dict_add_v_col_t*	add_v,
-	struct TABLE*		eval_table,
-	bool			drop_historical)
+	struct TABLE*		eval_table)
 	MY_ATTRIBUTE((warn_unused_result));
 
 /********************************************************************//**
diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
index c0f360f4cec..afa4ce208e9 100644
--- a/storage/innobase/row/row0merge.cc
+++ b/storage/innobase/row/row0merge.cc
@@ -1700,8 +1700,7 @@ row_merge_read_clustered_index(
 	ut_stage_alter_t*	stage,
 	double 			pct_cost,
 	row_merge_block_t*	crypt_block,
-	struct TABLE*		eval_table,
-	bool			drop_historical)
+	struct TABLE*		eval_table)
 {
 	dict_index_t*		clust_index;	/* Clustered index */
 	mem_heap_t*		row_heap;	/* Heap memory to create
@@ -2298,7 +2297,7 @@ end_of_index:
 		}
 
 		if (old_table->versioned()) {
-			if ((!new_table->versioned() || drop_historical)
+			if (!new_table->versioned()
 			    && clust_index->vers_history_row(rec, offsets)) {
 				continue;
 			}
@@ -4548,7 +4547,6 @@ this function and it will be passed to other functions for further accounting.
 @param[in]	add_v		new virtual columns added along with indexes
 @param[in]	eval_table	mysql table used to evaluate virtual column
 				value, see innobase_get_computed_value().
-@param[in]	drop_historical	whether to drop historical system rows
 @return DB_SUCCESS or error code */
 dberr_t
 row_merge_build_indexes(
@@ -4567,8 +4565,7 @@ row_merge_build_indexes(
 	bool			skip_pk_sort,
 	ut_stage_alter_t*	stage,
 	const dict_add_v_col_t*	add_v,
-	struct TABLE*		eval_table,
-	bool			drop_historical)
+	struct TABLE*		eval_table)
 {
 	merge_file_t*		merge_files;
 	row_merge_block_t*	block;
@@ -4732,7 +4729,7 @@ row_merge_build_indexes(
 		fts_sort_idx, psort_info, merge_files, key_numbers,
 		n_indexes, defaults, add_v, col_map, add_autoinc,
 		sequence, block, skip_pk_sort, &tmpfd, stage,
-		pct_cost, crypt_block, eval_table, drop_historical);
+		pct_cost, crypt_block, eval_table);
 
 	stage->end_phase_read_pk();