InnoDB: Make intermediate COMMITs in ALTER TABLE more robust (Bug #6633)

innobase/include/lock0lock.h:
  Replaced lock_get_ix_table() with lock_get_table().
innobase/lock/lock0lock.c:
  Replaced lock_get_ix_table() with lock_get_table().
innobase/include/row0mysql.h:
  row_lock_table_for_mysql(): Added parameter mode.
innobase/row/row0mysql.c:
  row_lock_table_for_mysql(): Added parameter mode.
sql/ha_innodb.cc:
  ha_innobase::write_row(): Make ALTER TABLE commits more robust:
  account for conversions from non-InnoDB format,
  do not attempt to commit if there are other than
  a single IX or IS lock on the source table, and
  the source table is in InnoDB format.  This
  prohibits intermediate commits for OPTIMIZE TABLE
  if the table contains an auto_increment field.

innobase/include/lock0lock.h

@@ -463,13 +463,17 @@ lock_rec_hash(
 	ulint	space,	/* in: space */
 	ulint	page_no);/* in: page number */
 /*************************************************************************
-Gets the table covered by an IX table lock. */
+Gets the table covered by an IX or IS table lock, if there are no
+other locks on the table. */
 
 dict_table_t*
-lock_get_ix_table(
+lock_get_table(
-/*==============*/
+/*===========*/
-			/* out: the table covered by the lock */
+			/* out: the table covered by the lock,
-	lock_t*	lock);	/* in: table lock */
+			or NULL if it is not an IX or IS table lock,
+			or there are other locks on the table */
+	lock_t*	lock,	/* in: lock */
+	ulint*	mode);	/* out: lock mode of table */
 /*************************************************************************
 Checks that a transaction id is sensible, i.e., not in the future. */
 

innobase/include/row0mysql.h

@@ -177,10 +177,12 @@ row_lock_table_for_mysql(
 					/* out: error code or DB_SUCCESS */
 	row_prebuilt_t*	prebuilt,	/* in: prebuilt struct in the MySQL
 					table handle */
-	dict_table_t*	table);		/* in: table to LOCK_IX, or NULL
+	dict_table_t*	table,		/* in: table to lock, or NULL
 					if prebuilt->table should be
 					locked as LOCK_TABLE_EXP |
 					prebuilt->select_lock_type */
+	ulint		mode);		/* in: lock mode of table */
+					   
 /*************************************************************************
 Does an insert for MySQL. */
 

innobase/lock/lock0lock.c

@@ -395,19 +395,6 @@ lock_rec_get_nth_bit(
 	return(ut_bit_get_nth(b, bit_index));
 }	
 
-/*************************************************************************
-Gets the table covered by an IX table lock. */
-
-dict_table_t*
-lock_get_ix_table(
-/*==============*/
-			/* out: the table covered by the lock */
-	lock_t*	lock)	/* in: table lock */
-{
-	ut_a(lock->type_mode == (LOCK_TABLE | LOCK_IX));
-	return(lock->un_member.tab_lock.table);
-}
-
 /*************************************************************************/
 
 #define lock_mutex_enter_kernel()	mutex_enter(&kernel_mutex)
@@ -614,6 +601,45 @@ lock_get_wait(
 	return(FALSE);
 }
 
+/*************************************************************************
+Gets the table covered by an IX or IS table lock, if there are no
+other locks on the table. */
+
+dict_table_t*
+lock_get_table(
+/*===========*/
+			/* out: the table covered by the lock,
+			or NULL if it is not an IX or IS table lock,
+			or there are other locks on the table */
+	lock_t*	lock,	/* in: lock */
+	ulint*	mode)	/* out: lock mode of table */
+{
+	dict_table_t*	table;
+	ulint		lock_mode;
+
+	table = NULL;
+	*mode = LOCK_NONE;
+
+	if (lock_get_type(lock) != LOCK_TABLE) {
+		return(table);
+	}
+
+	lock_mode = lock_get_mode(lock);
+	switch (lock_mode) {
+	case LOCK_IS:
+	case LOCK_IX:
+		*mode = lock_mode;
+		table = lock->un_member.tab_lock.table;
+		if (UT_LIST_GET_LEN(table->locks) != 1 ||
+		    UT_LIST_GET_FIRST(table->locks) != lock) {
+			/* We only support the case when
+			there is only one lock on this table. */
+			table = NULL;
+		}
+	}
+	return(table);
+}
+
 /*************************************************************************
 Sets the wait flag of a lock and the back pointer in trx to lock. */
 UNIV_INLINE

innobase/row/row0mysql.c

@@ -782,10 +782,11 @@ row_lock_table_for_mysql(
 					/* out: error code or DB_SUCCESS */
 	row_prebuilt_t*	prebuilt,	/* in: prebuilt struct in the MySQL
 					table handle */
-	dict_table_t*	table)		/* in: table to LOCK_IX, or NULL
+	dict_table_t*	table,		/* in: table to lock, or NULL
 					if prebuilt->table should be
 					locked as LOCK_TABLE_EXP |
 					prebuilt->select_lock_type */
+	ulint		mode)		/* in: lock mode of table */
 {
 	trx_t*		trx 		= prebuilt->trx;
 	que_thr_t*	thr;
@@ -819,7 +820,7 @@ run_again:
 	trx_start_if_not_started(trx);
 
 	if (table) {
-		err = lock_table(0, table, LOCK_IX, thr);
+		err = lock_table(0, table, mode, thr);
 	} else {
 		err = lock_table(LOCK_TABLE_EXP, prebuilt->table,
 			prebuilt->select_lock_type, thr);

sql/ha_innodb.cc

@@ -2324,20 +2324,55 @@ ha_innobase::write_row(
 		position in the source table need not be adjusted after the
 		intermediate COMMIT, since writes by other transactions are
 		being blocked by a MySQL table lock TL_WRITE_ALLOW_READ. */
-		ut_a(prebuilt->trx->mysql_n_tables_locked == 2);
+
-		ut_a(UT_LIST_GET_LEN(prebuilt->trx->trx_locks) >= 2);
+		dict_table_t*	table;
-		dict_table_t* table = lock_get_ix_table(
+		ibool		mode;
-				UT_LIST_GET_FIRST(prebuilt->trx->trx_locks));
+
 		num_write_row = 0;
+
 		/* Commit the transaction.  This will release the table
 		locks, so they have to be acquired again. */
-		innobase_commit(user_thd, prebuilt->trx);
+		switch (prebuilt->trx->mysql_n_tables_locked) {
-		/* Note that this transaction is still active. */
+		case 1:
-		user_thd->transaction.all.innodb_active_trans = 1;
+			/* Altering to InnoDB format */
-		/* Re-acquire the IX table lock on the source table. */
+			innobase_commit(user_thd, prebuilt->trx);
-		row_lock_table_for_mysql(prebuilt, table);
+			/* Note that this transaction is still active. */
-		/* We will need an IX lock on the destination table. */
+			user_thd->transaction.all.innodb_active_trans = 1;
-	        prebuilt->sql_stat_start = TRUE;
+			/* We will need an IX lock on the destination table. */
+		        prebuilt->sql_stat_start = TRUE;
+			break;
+		case 2:
+			/* Altering an InnoDB table */
+			ut_a(UT_LIST_GET_LEN(prebuilt->trx->trx_locks) >= 2);
+			table = lock_get_table(
+				UT_LIST_GET_FIRST(prebuilt->trx->trx_locks),
+				&mode);
+			if (!table) {
+				goto no_commit;
+			}
+
+			/* Commit the transaction.  This will release the table
+			locks, so they have to be acquired again. */
+			innobase_commit(user_thd, prebuilt->trx);
+			/* Note that this transaction is still active. */
+			user_thd->transaction.all.innodb_active_trans = 1;
+			/* Re-acquire the table lock on the source table. */
+			row_lock_table_for_mysql(prebuilt, table, mode);
+			/* We will need an IX lock on the destination table. */
+		        prebuilt->sql_stat_start = TRUE;
+			break;
+		default:
+		no_commit:
+			/* Unknown situation: do nothing (no commit) */
+			/*
+			ut_print_timestamp(stderr);
+			fprintf(stderr,
+				"  InnoDB error: ALTER TABLE is holding lock"
+				" on %lu tables!\n",
+				prebuilt->trx->mysql_n_tables_locked);
+			*/
+			break;
+		}
 	}
 
 	num_write_row++;
@@ -5015,7 +5050,8 @@ ha_innobase::external_lock(
 			if (thd->in_lock_tables &&
 			    thd->variables.innodb_table_locks) {
 				ulint	error;
-				error = row_lock_table_for_mysql(prebuilt, 0);
+				error = row_lock_table_for_mysql(prebuilt,
+							NULL, LOCK_TABLE_EXP);
 
 				if (error != DB_SUCCESS) {
 					error = convert_error_code_to_mysql(