Apply InnoDB snapshot innodb-5.1-ss2858, part 15. Fixes

Bug #39830: Table autoinc value not updated on first insert.
Bug #35498: Cannot get table test/table1 auto-inccounter value in ::info
Bug #36411: Failed to read auto-increment value from storage engine" in 5.1.24 auto-inc

Detailed revision comments:

r2854 | sunny | 2008-10-23 08:30:32 +0300 (Thu, 23 Oct 2008) | 13 lines
branches/5.1: Backport changes from branches/zip r2725

Simplify the autoinc initialization code. This removes the
non-determinism related to reading the table's autoinc value for the first
time. This change has also reduced the sizeof dict_table_t by sizeof(ibool)
bytes because we don't need the dict_table_t::autoinc_inited field anymore.

rb://16

storage/innobase/dict/dict0dict.c

@@ -422,8 +422,7 @@ dict_table_autoinc_lock(
 }
 
 /************************************************************************
-Initializes the autoinc counter. It is not an error to initialize an already
-initialized counter. */
+Unconditionally set the autoinc counter. */
 
 void
 dict_table_autoinc_initialize(
@@ -433,7 +432,6 @@ dict_table_autoinc_initialize(
 {
 	ut_ad(mutex_own(&table->autoinc_mutex));
 
-	table->autoinc_inited = TRUE;
 	table->autoinc = value;
 }
 
@@ -447,32 +445,25 @@ dict_table_autoinc_read(
 				/* out: value for a new row, or 0 */
 	dict_table_t*	table)	/* in: table */
 {
-	ib_longlong	value;
-
 	ut_ad(mutex_own(&table->autoinc_mutex));
 
-	if (!table->autoinc_inited) {
-
-		value = 0;
-	} else {
-		value = table->autoinc;
-	}
-
-	return(value);
+	return(table->autoinc);
 }
 
 /************************************************************************
 Updates the autoinc counter if the value supplied is greater than the
-current value. If not inited, does nothing. */
+current value. */
 
 void
-dict_table_autoinc_update(
-/*======================*/
+dict_table_autoinc_update_if_greater(
+/*=================================*/
 
 	dict_table_t*	table,	/* in: table */
 	ib_ulonglong	value)	/* in: value which was assigned to a row */
 {
-	if (table->autoinc_inited && value > table->autoinc) {
+	ut_ad(mutex_own(&table->autoinc_mutex));
+
+	if (value > table->autoinc) {
 
 		table->autoinc = value;
 	}

storage/innobase/dict/dict0mem.c

@@ -89,7 +89,7 @@ dict_mem_table_create(
 
 	mutex_create(&table->autoinc_mutex, SYNC_DICT_AUTOINC_MUTEX);
 
-	table->autoinc_inited = FALSE;
+	table->autoinc = 0;
 
 	/* The number of transactions that are either waiting on the
 	AUTOINC lock or have been granted the lock. */

storage/innobase/handler/ha_innodb.cc

@@ -953,7 +953,9 @@ innobase_next_autoinc(
 	/* Should never be 0. */
 	ut_a(increment > 0);
 
-	if (offset <= 1) {
+	if (max_value <= current) {
+		next_value = max_value;
+	} else if (offset <= 1) {
 		/* Offset 0 and 1 are the same, because there must be at
 		least one node in the system. */
 		if (max_value - current <= increment) {
@@ -978,6 +980,8 @@ innobase_next_autoinc(
 		} else {
 			next_value *= increment;
 
+			ut_a(max_value >= next_value);
+
 			/* Check for overflow. */
 			if (max_value - next_value <= offset) {
 				next_value = max_value;
@@ -987,6 +991,8 @@ innobase_next_autoinc(
 		}
 	}
 
+	ut_a(next_value <= max_value);
+
 	return(next_value);
 }
 
@@ -2343,6 +2349,44 @@ normalize_table_name(
 #endif
 }
 
+/************************************************************************
+Set the autoinc column max value. This should only be called once from
+ha_innobase::open(). Therefore there's no need for a covering lock. */
+
+ulong
+ha_innobase::innobase_initialize_autoinc()
+/*======================================*/
+{
+	dict_index_t*	index;
+	ulonglong	auto_inc;
+	const char*	col_name;
+	ulint		error = DB_SUCCESS;
+	dict_table_t*	innodb_table = prebuilt->table;
+
+	col_name = table->found_next_number_field->field_name;
+	index = innobase_get_index(table->s->next_number_index);
+
+	/* Execute SELECT MAX(col_name) FROM TABLE; */
+	error = row_search_max_autoinc(index, col_name, &auto_inc);
+
+	if (error == DB_SUCCESS) {
+
+		/* At the this stage we dont' know the increment
+		or the offset, so use default inrement of 1. */
+		++auto_inc;
+
+		dict_table_autoinc_initialize(innodb_table, auto_inc);
+
+	} else {
+		ut_print_timestamp(stderr);
+		fprintf(stderr, "  InnoDB: Error: (%lu) Couldn't read "
+			"the MAX(%s) autoinc value from the "
+			"index (%s).\n", error, col_name, index->name);
+	}
+
+	return(ulong(error));
+}
+
 /*********************************************************************
 Creates and opens a handle to a table which already exists in an InnoDB
 database. */
@@ -2534,6 +2578,26 @@ retry:
 
 	info(HA_STATUS_NO_LOCK | HA_STATUS_VARIABLE | HA_STATUS_CONST);
 
+	/* Only if the table has an AUTOINC column. */
+	if (prebuilt->table != NULL && table->found_next_number_field != NULL) {
+		ulint	error;
+
+		dict_table_autoinc_lock(prebuilt->table);
+
+		/* Since a table can already be "open" in InnoDB's internal
+		data dictionary, we only init the autoinc counter once, the
+		first time the table is loaded. We can safely reuse the
+		autoinc value from a previous MySQL open. */
+		if (dict_table_autoinc_read(prebuilt->table) == 0) {
+
+			error = innobase_initialize_autoinc();
+			/* Should always succeed! */
+			ut_a(error == DB_SUCCESS);
+		}
+
+		dict_table_autoinc_unlock(prebuilt->table);
+	}
+
 	DBUG_RETURN(0);
 }
 
@@ -3401,7 +3465,7 @@ min value of the autoinc interval. Once that is fixed we can get rid of
 the special lock handling.*/
 
 ulong
-ha_innobase::innobase_autoinc_lock(void)
+ha_innobase::innobase_lock_autoinc(void)
 /*====================================*/
 					/* out: DB_SUCCESS if all OK else
 					error code */
@@ -3466,7 +3530,7 @@ ha_innobase::innobase_reset_autoinc(
 {
 	ulint		error;
 
-	error = innobase_autoinc_lock();
+	error = innobase_lock_autoinc();
 
 	if (error == DB_SUCCESS) {
 
@@ -3491,11 +3555,11 @@ ha_innobase::innobase_set_max_autoinc(
 {
 	ulint		error;
 
-	error = innobase_autoinc_lock();
+	error = innobase_lock_autoinc();
 
 	if (error == DB_SUCCESS) {
 
-		dict_table_autoinc_update(prebuilt->table, auto_inc);
+		dict_table_autoinc_update_if_greater(prebuilt->table, auto_inc);
 
 		dict_table_autoinc_unlock(prebuilt->table);
 	}
@@ -3705,7 +3769,7 @@ no_commit:
 			update the table upper limit. Note: last_value
 			will be 0 if get_auto_increment() was not called.*/
 
-			if (auto_inc < col_max_value
+			if (auto_inc <= col_max_value
 			    && auto_inc > prebuilt->autoinc_last_value) {
 set_max_autoinc:
 				ut_a(prebuilt->autoinc_increment > 0);
@@ -3965,7 +4029,7 @@ ha_innobase::update_row(
 		col_max_value = innobase_get_int_col_max_value(
 			table->next_number_field);
 
-		if (auto_inc < col_max_value && auto_inc != 0) {
+		if (auto_inc <= col_max_value && auto_inc != 0) {
 
 			ulonglong	need;
 			ulonglong	offset;
@@ -4020,30 +4084,6 @@ ha_innobase::delete_row(
 
 	ha_statistic_increment(&SSV::ha_delete_count);
 
-	/* Only if the table has an AUTOINC column */
-	if (table->found_next_number_field && record == table->record[0]) {
-		ulonglong	dummy = 0;
-
-		/* First check whether the AUTOINC sub-system has been
-		initialized using the AUTOINC mutex. If not then we
-		do it the "proper" way, by acquiring the heavier locks. */
-		dict_table_autoinc_lock(prebuilt->table);
-
-		if (!prebuilt->table->autoinc_inited) {
-			dict_table_autoinc_unlock(prebuilt->table);
-
-			error = innobase_get_auto_increment(&dummy);
-
-			if (error == DB_SUCCESS) {
-				dict_table_autoinc_unlock(prebuilt->table);
-			} else {
-				goto error_exit;
-			}
-		} else  {
-			dict_table_autoinc_unlock(prebuilt->table);
-		}
-	}
-
 	if (!prebuilt->upd_node) {
 		row_get_prebuilt_update_vector(prebuilt);
 	}
@@ -4058,7 +4098,6 @@ ha_innobase::delete_row(
 
 	innodb_srv_conc_exit_innodb(trx);
 
-error_exit:
 	error = convert_error_code_to_mysql(error, user_thd);
 
 	/* Tell the InnoDB server that there might be work for
@@ -6135,16 +6174,7 @@ ha_innobase::info(
 	}
 
 	if (flag & HA_STATUS_AUTO && table->found_next_number_field) {
-		ulonglong	auto_inc;
-
-		if (innobase_read_and_init_auto_inc(&auto_inc) != 0) {
-			sql_print_error("Cannot get table %s auto-inc"
-					"counter value in ::info\n",
-					ib_table->name);
-			auto_inc = 0;
-		}
-
-		stats.auto_increment_value = auto_inc;
+ 		stats.auto_increment_value = innobase_peek_autoinc();
 	}
 
 	prebuilt->trx->op_info = (char*)"";
@@ -7475,157 +7505,59 @@ ha_innobase::store_lock(
 	return(to);
 }
 
-/***********************************************************************
-This function initializes the auto-inc counter if it has not been
-initialized yet. This function does not change the value of the auto-inc
-counter if it already has been initialized. In parameter ret returns
-the value of the auto-inc counter. */
+/*******************************************************************************
+Read the next autoinc value. Acquire the relevant locks before reading
+the AUTOINC value. If SUCCESS then the table AUTOINC mutex will be locked
+on return and all relevant locks acquired. */
 
-int
-ha_innobase::innobase_read_and_init_auto_inc(
-/*=========================================*/
-						/* out: 0 or generic MySQL
-						error code */
-        ulonglong*	value)			/* out: the autoinc value */
+ulong
+ha_innobase::innobase_get_autoinc(
+/*==============================*/
+					/* out: DB_SUCCESS or error code */
+	ulonglong*	value)		/* out: autoinc value */
 {
-	ulonglong	auto_inc;
-	ibool		stmt_start;
-	int		mysql_error = 0;
-	dict_table_t*	innodb_table = prebuilt->table;
-	ibool		trx_was_not_started	= FALSE;
-
-	ut_a(prebuilt);
-	ut_a(prebuilt->table);
-
-	/* Remember if we are in the beginning of an SQL statement.
-	This function must not change that flag. */
-	stmt_start = prebuilt->sql_stat_start;
-
-	/* Prepare prebuilt->trx in the table handle */
-	update_thd(ha_thd());
-
-	if (prebuilt->trx->conc_state == TRX_NOT_STARTED) {
-		trx_was_not_started = TRUE;
-	}
-
-	/* In case MySQL calls this in the middle of a SELECT query, release
-	possible adaptive hash latch to avoid deadlocks of threads */
-
-	trx_search_latch_release_if_reserved(prebuilt->trx);
-
-	dict_table_autoinc_lock(prebuilt->table);
-
-	auto_inc = dict_table_autoinc_read(prebuilt->table);
-
-	/* Was the AUTOINC counter reset during normal processing, if
-	so then we simply start count from 1. No need to go to the index.*/
-	if (auto_inc == 0 && innodb_table->autoinc_inited) {
-		++auto_inc;
-		dict_table_autoinc_initialize(innodb_table, auto_inc);
-	}
-
-	if (auto_inc == 0) {
-		dict_index_t* index;
-		const char* autoinc_col_name;
-
-		ut_a(!innodb_table->autoinc_inited);
-
-		index = innobase_get_index(table->s->next_number_index);
-
-		autoinc_col_name = table->found_next_number_field->field_name;
-
-		prebuilt->autoinc_error = row_search_max_autoinc(
-			index, autoinc_col_name, &auto_inc);
-
-		if (prebuilt->autoinc_error == DB_SUCCESS) {
-			if (auto_inc < ~0x0ULL) {
-				++auto_inc;
-			}
-			dict_table_autoinc_initialize(innodb_table, auto_inc);
-		} else {
-			ut_print_timestamp(stderr);
-			fprintf(stderr, "  InnoDB: Error: (%lu) Couldn't read "
-				"the max AUTOINC value from the index (%s).\n",
-				prebuilt->autoinc_error, index->name);
-
-			mysql_error = 1;
-		}
-	}
-
-	*value = auto_inc;
-
-	dict_table_autoinc_unlock(prebuilt->table);
+ 	*value = 0;
+ 
+	prebuilt->autoinc_error = innobase_lock_autoinc();
 
-	/* Since MySQL does not seem to call autocommit after SHOW TABLE
-	STATUS (even if we would register the trx here), we commit our
-	transaction here if it was started here. This is to eliminate a
-	dangling transaction. If the user had AUTOCOMMIT=0, then SHOW
-	TABLE STATUS does leave a dangling transaction if the user does not
-	himself call COMMIT. */
+	if (prebuilt->autoinc_error == DB_SUCCESS) {
 
-	if (trx_was_not_started) {
+		/* Determine the first value of the interval */
+		*value = dict_table_autoinc_read(prebuilt->table);
 
-		innobase_commit_low(prebuilt->trx);
+		/* It should have been initialized during open. */
+		ut_a(*value != 0);
 	}
-
-	prebuilt->sql_stat_start = stmt_start;
-
-	return(mysql_error);
+  
+	return(ulong(prebuilt->autoinc_error));
 }
 
-/*******************************************************************************
-Read the next autoinc value, initialize the table if it's not initialized.
-On return if there is no error then the tables AUTOINC lock is locked.*/
+/***********************************************************************
+This function reads the global auto-inc counter. It doesn't use the 
+AUTOINC lock even if the lock mode is set to TRADITIONAL. */
 
-ulint
-ha_innobase::innobase_get_auto_increment(
-/*=====================================*/
-	ulonglong*	value)		/* out: autoinc value */
+ulonglong
+ha_innobase::innobase_peek_autoinc()
+/*================================*/
+					/* out: the autoinc value */
 {
-	*value = 0;
-
-	/* Note: If the table is not initialized when we attempt the
-	read below. We initialize the table's auto-inc counter  and
-	always do a reread of the AUTOINC value. */
-	do {
-		/* We need to send the correct error code to the client
-		because handler::get_auto_increment() doesn't allow a way
-		to return the specific error for why it failed. */
-		prebuilt->autoinc_error = innobase_autoinc_lock();
-
-		if (prebuilt->autoinc_error == DB_SUCCESS) {
-			ulonglong	autoinc;
-
-			/* Determine the first value of the interval */
-			autoinc = dict_table_autoinc_read(prebuilt->table);
-
-			/* We need to initialize the AUTO-INC value, for
-			that we release all locks.*/
-			if (autoinc == 0) {
-				trx_t*		trx;
+	ulonglong	auto_inc;
+	dict_table_t*	innodb_table;
 
-				trx = prebuilt->trx;
-				dict_table_autoinc_unlock(prebuilt->table);
+	ut_a(prebuilt != NULL);
+	ut_a(prebuilt->table != NULL);
 
-				/* If we had reserved the AUTO-INC
-				lock in this SQL statement we release
-				it before retrying.*/
-				row_unlock_table_autoinc_for_mysql(trx);
+	innodb_table = prebuilt->table;
 
-				/* Just to make sure */
-				ut_a(!trx->auto_inc_lock);
+	dict_table_autoinc_lock(innodb_table);
 
-				/* Will set prebuilt->autoinc_error if there
-				is a problem during init. */
-				innobase_read_and_init_auto_inc(&autoinc);
+	auto_inc = dict_table_autoinc_read(innodb_table);
 
-			} else {
-				*value = autoinc;
-			}
-		}
-	} while (*value == 0 && prebuilt->autoinc_error == DB_SUCCESS);
+	ut_a(auto_inc > 0);
 
-	return(prebuilt->autoinc_error);
+	dict_table_autoinc_unlock(innodb_table);
+ 
+	return(auto_inc);
 }
 
 /*******************************************************************************
@@ -7652,7 +7584,7 @@ ha_innobase::get_auto_increment(
 	/* Prepare prebuilt->trx in the table handle */
 	update_thd(ha_thd());
 
-	error = innobase_get_auto_increment(&autoinc);
+	error = innobase_get_autoinc(&autoinc);
 
 	if (error != DB_SUCCESS) {
 		*first_value = (~(ulonglong) 0);
@@ -7717,7 +7649,7 @@ ha_innobase::get_auto_increment(
 		ut_a(prebuilt->autoinc_last_value >= *first_value);
 
 		/* Update the table autoinc variable */
-		dict_table_autoinc_update(
+		dict_table_autoinc_update_if_greater(
 			prebuilt->table, prebuilt->autoinc_last_value);
 	} else {
 		/* This will force write_row() into attempting an update
@@ -7755,6 +7687,11 @@ ha_innobase::reset_auto_increment(
 		DBUG_RETURN(error);
 	}
 
+	/* The next value can never be 0. */
+	if (value == 0) {
+		value = 1;
+	}
+
 	innobase_reset_autoinc(value);
 
 	DBUG_RETURN(0);

storage/innobase/handler/ha_innodb.h

@@ -72,13 +72,15 @@ class ha_innobase: public handler
 	int update_thd(THD* thd);
 	int change_active_index(uint keynr);
 	int general_fetch(uchar* buf, uint direction, uint match_mode);
-	int innobase_read_and_init_auto_inc(ulonglong* ret);
-	ulong innobase_autoinc_lock();
+	ulong innobase_lock_autoinc();
+	ulonglong innobase_peek_autoinc();
 	ulong innobase_set_max_autoinc(ulonglong auto_inc);
 	ulong innobase_reset_autoinc(ulonglong auto_inc);
-	ulong innobase_get_auto_increment(ulonglong* value);
+	ulong innobase_get_autoinc(ulonglong* value);
+	ulong innobase_update_autoinc(ulonglong	auto_inc);
+	ulong innobase_initialize_autoinc();
 	dict_index_t* innobase_get_index(uint keynr);
-	ulonglong innobase_get_int_col_max_value(const Field* field);
+ 	ulonglong innobase_get_int_col_max_value(const Field* field);
 
 	/* Init values for the class: */
  public:

storage/innobase/include/dict0dict.h

@@ -178,8 +178,7 @@ dict_table_autoinc_lock(
 /*====================*/
 	dict_table_t*	table);	/* in: table */
 /************************************************************************
-Initializes the autoinc counter. It is not an error to initialize an already
-initialized counter. */
+Unconditionally set the autoinc counter. */
 
 void
 dict_table_autoinc_initialize(
@@ -196,12 +195,12 @@ dict_table_autoinc_read(
 				/* out: value for a new row, or 0 */
 	dict_table_t*	table);	/* in: table */
 /************************************************************************
-Updates the autoinc counter if the value supplied is equal or bigger than the
-current value. If not inited, does nothing. */
+Updates the autoinc counter if the value supplied is greater than the
+current value. */
 
 void
-dict_table_autoinc_update(
-/*======================*/
+dict_table_autoinc_update_if_greater(
+/*=================================*/
 
 	dict_table_t*	table,	/* in: table */
 	ib_ulonglong	value);	/* in: value which was assigned to a row */

storage/innobase/include/dict0mem.h

@@ -405,10 +405,6 @@ struct dict_table_struct{
 	mutex_t		autoinc_mutex;
 				/* mutex protecting the autoincrement
 				counter */
-	ibool		autoinc_inited;
-				/* TRUE if the autoinc counter has been
-				inited; MySQL gets the init value by executing
-				SELECT MAX(auto inc column) */
 	ib_ulonglong	autoinc;/* autoinc counter value to give to the
 				next inserted row */
 	ulong		n_waiting_or_granted_auto_inc_locks;

storage/innobase/row/row0mysql.c

@@ -2910,7 +2910,7 @@ next_rec:
 	/* MySQL calls ha_innobase::reset_auto_increment() which does
 	the same thing. */
 	dict_table_autoinc_lock(table);
-	dict_table_autoinc_initialize(table, 0);
+	dict_table_autoinc_initialize(table, 1);
 	dict_table_autoinc_unlock(table);
 	dict_update_statistics(table);