InnoDB: Allow ALTER TABLE to do intermediate COMMIT also when the table

contains auto_increment columns.  (Bug #6633)


innobase/include/lock0lock.h:
  Replaced lock_get_table()
  with lock_get_src_table() and lock_is_table_exclusive()
innobase/lock/lock0lock.c:
  Replaced lock_get_table()
  with lock_get_src_table() and lock_is_table_exclusive()
sql/ha_innodb.cc:
  ha_innobase::write_row(): Improve the ALTER TABLE optimization
  (do intermediate COMMIT also if table contains auto_increment columns)

innobase/include/lock0lock.h

@@ -463,17 +463,32 @@ lock_rec_hash(
 	ulint	space,	/* in: space */
 	ulint	page_no);/* in: page number */
 /*************************************************************************
-Gets the table covered by an IX or IS table lock, if there are no
-other locks on the table. */
+Gets the source table of an ALTER TABLE transaction.  The table must be
+covered by an IX or IS table lock. */
 
 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 */
+lock_get_src_table(
+/*===============*/
+				/* out: the source table of transaction,
+				if it is covered by an IX or IS table lock;
+				dest if there is no source table, and
+				NULL if the transaction is locking more than
+				two tables or an inconsistency is found */
+	trx_t*		trx,	/* in: transaction */
+	dict_table_t*	dest,	/* in: destination of ALTER TABLE */
+	ulint*		mode);	/* out: lock mode of the source table */
+/*************************************************************************
+Determine if the given table is exclusively "owned" by the given
+transaction, i.e., transaction holds LOCK_IX and possibly LOCK_AUTO_INC
+on the table. */
+
+ibool
+lock_table_exclusive(
+/*=================*/
+				/* out: TRUE if table is only locked by trx,
+				with LOCK_IX, and possibly LOCK_AUTO_INC */
+	dict_table_t*	table,	/* in: table */
+	trx_t*		trx);	/* in: transaction */
 /*************************************************************************
 Checks that a transaction id is sensible, i.e., not in the future. */
 

innobase/lock/lock0lock.c

@@ -602,42 +602,125 @@ lock_get_wait(
 }
 
 /*************************************************************************
-Gets the table covered by an IX or IS table lock, if there are no
-other locks on the table. */
+Gets the source table of an ALTER TABLE transaction.  The table must be
+covered by an IX or IS table lock. */
 
 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 */
+lock_get_src_table(
+/*===============*/
+				/* out: the source table of transaction,
+				if it is covered by an IX or IS table lock;
+				dest if there is no source table, and
+				NULL if the transaction is locking more than
+				two tables or an inconsistency is found */
+	trx_t*		trx,	/* in: transaction */
+	dict_table_t*	dest,	/* in: destination of ALTER TABLE */
+	ulint*		mode)	/* out: lock mode of the source table */
 {
-	dict_table_t*	table;
-	ulint		lock_mode;
+	dict_table_t*	src;
+	lock_t*		lock;
 
-	table = NULL;
+	src = 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;
+	for (lock = UT_LIST_GET_FIRST(trx->trx_locks);
+	     lock;
+	     lock = UT_LIST_GET_NEXT(trx_locks, lock)) {
+		lock_table_t*	tab_lock;
+		ulint		lock_mode;
+		if (!(lock_get_type(lock) & LOCK_TABLE)) {
+			/* We are only interested in table locks. */
+			continue;
+		}
+		tab_lock = &lock->un_member.tab_lock;
+		if (dest == tab_lock->table) {
+			/* We are not interested in the destination table. */
+			continue;
+		} else if (!src) {
+			/* This presumably is the source table. */
+			src = tab_lock->table;
+			if (UT_LIST_GET_LEN(src->locks) != 1 ||
+			    UT_LIST_GET_FIRST(src->locks) != lock) {
+				/* We only support the case when
+				there is only one lock on this table. */
+				return(NULL);
+			}
+		} else if (src != tab_lock->table) {
+			/* The transaction is locking more than
+			two tables (src and dest): abort */
+			return(NULL);
+		}
+
+		/* Check that the source table is locked by
+		LOCK_IX or LOCK_IS. */
+		lock_mode = lock_get_mode(lock);
+		switch (lock_mode) {
+		case LOCK_IX:
+		case LOCK_IS:
+			if (*mode != LOCK_NONE && *mode != lock_mode) {
+				/* There are multiple locks on src. */
+				return(NULL);
+			}
+			*mode = lock_mode;
+			break;
 		}
 	}
-	return(table);
+
+	if (!src) {
+		/* No source table lock found: flag the situation to caller */
+		src = dest;
+	}
+
+	return(src);
+}
+
+/*************************************************************************
+Determine if the given table is exclusively "owned" by the given
+transaction, i.e., transaction holds LOCK_IX and possibly LOCK_AUTO_INC
+on the table. */
+
+ibool
+lock_is_table_exclusive(
+/*====================*/
+				/* out: TRUE if table is only locked by trx,
+				with LOCK_IX, and possibly LOCK_AUTO_INC */
+	dict_table_t*	table,	/* in: table */
+	trx_t*		trx)	/* in: transaction */
+{
+	lock_t*	lock;
+	bool	ok	= FALSE;
+
+	ut_ad(table && trx);
+
+	for (lock = UT_LIST_GET_FIRST(table->locks);
+	     lock;
+	     lock = UT_LIST_GET_NEXT(locks, &lock->un_member.tab_lock)) {
+		if (lock->trx != trx) {
+			/* A lock on the table is held
+			by some other transaction. */
+			return(FALSE);
+		}
+
+		if (!(lock_get_type(lock) & LOCK_TABLE)) {
+			/* We are interested in table locks only. */
+			continue;
+		}
+
+		switch (lock_get_mode(lock)) {
+		case LOCK_IX:
+			ok = TRUE;
+			break;
+		case LOCK_AUTO_INC:
+			/* It is allowed for trx to hold an
+			auto_increment lock. */
+			break;
+		default:
+			/* Other table locks than LOCK_IX are not allowed. */
+			return(FALSE);
+		}
+	}
+
+	return(ok);
 }
 
 /*************************************************************************

sql/ha_innodb.cc

@@ -2325,29 +2325,44 @@ ha_innobase::write_row(
 		intermediate COMMIT, since writes by other transactions are
 		being blocked by a MySQL table lock TL_WRITE_ALLOW_READ. */
 
-		dict_table_t*	table;
+		dict_table_t*	src_table;
 		ibool		mode;
 
 		num_write_row = 0;
 
 		/* Commit the transaction.  This will release the table
 		locks, so they have to be acquired again. */
-		switch (prebuilt->trx->mysql_n_tables_locked) {
-		case 1:
+
+		/* Altering an InnoDB table */
+		/* Get the source table. */
+		src_table = lock_get_src_table(
+				prebuilt->trx, prebuilt->table, &mode);
+		if (!src_table) {
+		no_commit:
+			/* Unknown situation: do not commit */
+			/*
+			ut_print_timestamp(stderr);
+			fprintf(stderr,
+				"  InnoDB error: ALTER TABLE is holding lock"
+				" on %lu tables!\n",
+				prebuilt->trx->mysql_n_tables_locked);
+			*/
+			;
+		} else if (src_table == prebuilt->table) {
+			/* Source table is not in InnoDB format:
+			no need to re-acquire locks on it. */
+
 			/* Altering to InnoDB format */
 			innobase_commit(user_thd, prebuilt->trx);
 			/* Note that this transaction is still active. */
 			user_thd->transaction.all.innodb_active_trans = 1;
 			/* 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) {
+		} else {
+			/* Ensure that there are no other table locks than
+			LOCK_IX and LOCK_AUTO_INC on the destination table. */
+			if (!lock_is_table_exclusive(prebuilt->table,
+							prebuilt->trx)) {
 				goto no_commit;
 			}
 
@@ -2357,21 +2372,9 @@ ha_innobase::write_row(
 			/* 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);
+			row_lock_table_for_mysql(prebuilt, src_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;
 		}
 	}