Many files:

  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc


innobase/row/row0ins.c:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc
innobase/row/row0sel.c:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc
innobase/row/row0upd.c:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc
innobase/row/row0vers.c:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc
innobase/btr/btr0cur.c:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc
innobase/include/btr0cur.h:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc
innobase/include/row0upd.h:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc
innobase/include/sync0sync.ic:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc
innobase/lock/lock0lock.c:
  Fix assertion failure on line 713 of row0upd.c if there is a column prefix index and the last characters in the prefix are spaces: do not assume that the length of alphabetically equal strings is the same; fix a buglet which could cause InnoDB to think that a secondary index record was not locked though it had been updated in a way which did not alpahabetically change its value, e.g., abc -> aBc

4 files changed, 121 insertions, 19 deletions

diff --git a/innobase/row/row0ins.c b/innobase/row/row0ins.c
index e02859bc851..45c5897eee7 100644
--- a/innobase/row/row0ins.c
+++ b/innobase/row/row0ins.c
@@ -225,11 +225,15 @@ ulint
 row_ins_sec_index_entry_by_modify(
 /*==============================*/
 				/* out: DB_SUCCESS or error code */
+	ulint		mode,	/* in: BTR_MODIFY_LEAF or BTR_MODIFY_TREE,
+				depending on whether mtr holds just a leaf
+				latch or also a tree latch */
 	btr_cur_t*	cursor,	/* in: B-tree cursor */
 	dtuple_t*	entry,	/* in: index entry to insert */
 	que_thr_t*	thr,	/* in: query thread */
 	mtr_t*		mtr)	/* in: mtr */
 {
+	big_rec_t*	dummy_big_rec;
 	mem_heap_t*	heap;
 	upd_t*		update;
 	rec_t*		rec;
@@ -241,16 +245,28 @@ row_ins_sec_index_entry_by_modify(
 	ut_ad(rec_get_deleted_flag(rec));
 	
 	/* We know that in the alphabetical ordering, entry and rec are
-	identical. But in their binary form there may be differences if
+	identified. But in their binary form there may be differences if
 	there are char fields in them. Therefore we have to calculate the
-	difference and do an update-in-place if necessary. */
+	difference. */
 	
 	heap = mem_heap_create(1024);
 	
 	update = row_upd_build_sec_rec_difference_binary(cursor->index,
 							entry, rec, heap); 
+	if (mode == BTR_MODIFY_LEAF) {
+		/* Try an optimistic updating of the record, keeping changes
+		within the page */
 
-	err = btr_cur_update_sec_rec_in_place(cursor, update, thr, mtr);
+		err = btr_cur_optimistic_update(BTR_KEEP_SYS_FLAG, cursor,
+						update, 0, thr, mtr);
+		if (err == DB_OVERFLOW || err == DB_UNDERFLOW) {
+			err = DB_FAIL;
+		}
+	} else  {
+		ut_a(mode == BTR_MODIFY_TREE);
+		err = btr_cur_pessimistic_update(BTR_KEEP_SYS_FLAG, cursor,
+					&dummy_big_rec, update, 0, thr, mtr);
+	}
 
 	mem_heap_free(heap);
 
@@ -1829,7 +1845,8 @@ row_ins_index_entry_low(
 							ext_vec, n_ext_vec,
 							thr, &mtr);
 		} else {
-			err = row_ins_sec_index_entry_by_modify(&cursor, entry,
+			err = row_ins_sec_index_entry_by_modify(mode, &cursor,
+								entry,
 								thr, &mtr);
 		}
 		
diff --git a/innobase/row/row0sel.c b/innobase/row/row0sel.c
index f02fbeeb8fd..27d27b75a1c 100644
--- a/innobase/row/row0sel.c
+++ b/innobase/row/row0sel.c
@@ -51,7 +51,8 @@ to que_run_threads: this is to allow canceling runaway queries */
 
 /************************************************************************
 Returns TRUE if the user-defined column values in a secondary index record
-are the same as the corresponding columns in the clustered index record.
+are alphabetically the same as the corresponding columns in the clustered
+index record.
 NOTE: the comparison is NOT done as a binary comparison, but character
 fields are compared with collation! */
 static
@@ -96,11 +97,6 @@ row_sel_sec_rec_is_for_clust_rec(
 		       clust_len = ifield->prefix_len;
 		}
 
-                if (sec_len != clust_len) {
-
-                        return(FALSE);
-                }
-
                 if (0 != cmp_data_data(dict_col_get_type(col),
                                         clust_field, clust_len,
                                         sec_field, sec_len)) {
diff --git a/innobase/row/row0upd.c b/innobase/row/row0upd.c
index db68479509d..3de0a4019d1 100644
--- a/innobase/row/row0upd.c
+++ b/innobase/row/row0upd.c
@@ -698,7 +698,7 @@ row_upd_build_sec_rec_difference_binary(
 	ulint		i;
 
 	/* This function is used only for a secondary index */
-	ut_ad(0 == (index->type & DICT_CLUSTERED));
+	ut_a(0 == (index->type & DICT_CLUSTERED));
 
 	update = upd_create(dtuple_get_n_fields(entry), heap);
 
@@ -710,7 +710,13 @@ row_upd_build_sec_rec_difference_binary(
 
 		dfield = dtuple_get_nth_field(entry, i);
 
-		ut_a(len == dfield_get_len(dfield));
+		/* NOTE that it may be that len != dfield_get_len(dfield) if we
+		are updating in a character set and collation where strings of
+		different length can be equal in an alphabetical comparison,
+		and also in the case where we have a column prefix index
+		and the last characters in the index field are spaces; the
+		latter case probably caused the assertion failures reported at
+		row0upd.c line 713 in versions 4.0.14 - 4.0.16. */
 
 		/* NOTE: we compare the fields as binary strings!
 		(No collation) */
@@ -820,12 +826,76 @@ Replaces the new column values stored in the update vector to the index entry
 given. */
 
 void
+row_upd_index_replace_new_col_vals_index_pos(
+/*=========================================*/
+	dtuple_t*	entry,	/* in/out: index entry where replaced */
+	dict_index_t*	index,	/* in: index; NOTE that this may also be a
+				non-clustered index */
+	upd_t*		update,	/* in: an update vector built for the index so
+				that the field number in an upd_field is the
+				index position */
+	mem_heap_t*	heap)	/* in: memory heap to which we allocate and
+				copy the new values, set this as NULL if you
+				do not want allocation */
+{
+	dict_field_t*	field;
+	upd_field_t*	upd_field;
+	dfield_t*	dfield;
+	dfield_t*	new_val;
+	ulint		j;
+	ulint		i;
+
+	ut_ad(index);
+
+	dtuple_set_info_bits(entry, update->info_bits);
+
+	for (j = 0; j < dict_index_get_n_fields(index); j++) {
+
+	        field = dict_index_get_nth_field(index, j);
+
+		for (i = 0; i < upd_get_n_fields(update); i++) {
+
+		        upd_field = upd_get_nth_field(update, i);
+
+			if (upd_field->field_no == j) {
+
+			        dfield = dtuple_get_nth_field(entry, j);
+
+				new_val = &(upd_field->new_val);
+
+				dfield_set_data(dfield, new_val->data,
+								new_val->len);
+				if (heap && new_val->len != UNIV_SQL_NULL) {
+				        dfield->data = mem_heap_alloc(heap,
+								new_val->len);
+					ut_memcpy(dfield->data, new_val->data,
+								new_val->len);
+				}
+
+				if (field->prefix_len > 0
+			            && new_val->len != UNIV_SQL_NULL
+			            && new_val->len > field->prefix_len) {
+
+				        dfield->len = field->prefix_len;
+				}
+			}
+		}
+	}
+}
+
+/***************************************************************
+Replaces the new column values stored in the update vector to the index entry
+given. */
+
+void
 row_upd_index_replace_new_col_vals(
 /*===============================*/
 	dtuple_t*	entry,	/* in/out: index entry where replaced */
 	dict_index_t*	index,	/* in: index; NOTE that this may also be a
 				non-clustered index */
-	upd_t*		update,	/* in: update vector */
+	upd_t*		update,	/* in: an update vector built for the
+				CLUSTERED index so that the field number in
+				an upd_field is the clustered index position */
 	mem_heap_t*	heap)	/* in: memory heap to which we allocate and
 				copy the new values, set this as NULL if you
 				do not want allocation */
@@ -893,7 +963,9 @@ row_upd_changes_ord_field_binary(
 				known when this function is called, e.g., at
 				compile time */
 	dict_index_t*	index,	/* in: index of the record */
-	upd_t*		update)	/* in: update vector for the row */
+	upd_t*		update)	/* in: update vector for the row; NOTE: the
+				field numbers in this MUST be clustered index
+				positions! */
 {
 	upd_field_t*	upd_field;
 	dict_field_t*	ind_field;
diff --git a/innobase/row/row0vers.c b/innobase/row/row0vers.c
index d4a463d8a96..fca56389e45 100644
--- a/innobase/row/row0vers.c
+++ b/innobase/row/row0vers.c
@@ -117,9 +117,10 @@ row_vers_impl_x_locked_off_kernel(
 		return(NULL);
 	}
 
-	/* We look up if some earlier version of the clustered index record
-	would require rec to be in a different state (delete marked or
-	unmarked, or not existing). If there is such a version, then rec was
+	/* We look up if some earlier version, which was modified by the trx_id
+	transaction, of the clustered index record would require rec to be in
+	a different state (delete marked or unmarked, or have different field
+	values, or not existing). If there is such a version, then rec was
 	modified by the trx_id transaction, and it has an implicit x-lock on
 	rec. Note that if clust_rec itself would require rec to be in a
 	different state, then the trx_id transaction has not yet had time to
@@ -188,6 +189,8 @@ row_vers_impl_x_locked_off_kernel(
 
 		vers_del = rec_get_deleted_flag(prev_version);
 
+		/* We check if entry and rec are identified in the alphabetical
+		ordering */
 		if (0 == cmp_dtuple_rec(entry, rec)) {
 			/* The delete marks of rec and prev_version should be
 			equal for rec to be in the state required by
@@ -198,6 +201,20 @@ row_vers_impl_x_locked_off_kernel(
 
 				break;
 			}
+
+			/* It is possible that the row was updated so that the
+			secondary index record remained the same in
+			alphabetical ordering, but the field values changed
+			still. For example, 'abc' -> 'ABC'. Check also that. */
+
+			dtuple_set_types_binary(entry,
+						dtuple_get_n_fields(entry));
+			if (0 != cmp_dtuple_rec(entry, rec)) {
+
+				trx = trx_get_on_id(trx_id);
+
+				break;
+			}
 		} else if (!rec_del) {
 			/* The delete mark should be set in rec for it to be
 			in the state required by prev_version */
@@ -256,8 +273,8 @@ row_vers_must_preserve_del_marked(
 Finds out if a version of the record, where the version >= the current
 purge view, should have ientry as its secondary index entry. We check
 if there is any not delete marked version of the record where the trx
-id >= purge view, and the secondary index entry == ientry; exactly in
-this case we return TRUE. */
+id >= purge view, and the secondary index entry and ientry are identified in
+the alphabetical ordering; exactly in this case we return TRUE. */
 
 ibool
 row_vers_old_has_index_entry(