MDEV-33802 Weird read view after ROLLBACK of other transactions.

In the case if some unique key fields are nullable, there can be
several records with the same key fields in unique index with at least
one key field equal to NULL, as NULL != NULL.

When transaction is resumed after waiting on the record with at least one
key field equal to NULL, and stored in persistent cursor record is
deleted, persistent cursor can be restored to the record with all key
fields equal to the stored ones, but with at least one field equal to
NULL. And such record is wrongly treated as a record with the same unique
key as stored in persistent cursor record one, what is wrong as
NULL != NULL.

The fix is to check if at least one unique field is NULL in restored
persistent cursor position, and, if so, then don't treat the record as
one with the same unique key as in the stored record key.

dict_index_t::nulls_equal was removed, as it was initially developed for
never existed in MariaDB "intrinsic tables", and there is no code, which
would set it to "true".

Reviewed by Marko Mäkelä.

mysql-test/suite/innodb/r/cursor-restore-unique-null.result

+CREATE TABLE t(a INT PRIMARY KEY, b INT, c INT, UNIQUE KEY `b_c` (`b`,`c`))
+ENGINE=InnoDB, STATS_PERSISTENT=0;
+INSERT INTO t SET a = 1, c = 2;
+connect con1,localhost,root;
+BEGIN;
+INSERT INTO t SET a=2, c=2;
+connection default;
+BEGIN;
+SET DEBUG_SYNC="lock_wait_suspend_thread_enter SIGNAL select_locked";
+SELECT * FROM t FORCE INDEX(b) FOR UPDATE;
+connection con1;
+SET DEBUG_SYNC="now WAIT_FOR select_locked";
+ROLLBACK;
+connection default;
+# If the bug is not fixed, and the both unique index key fields are
+# NULL, there will be two (1, NULL, 2) rows in the result,
+# because cursor will be restored to (NULL, 2, 1) position for
+# secondary key instead of "supremum".
+a	b	c
+1	NULL	2
+COMMIT;
+SET DEBUG_SYNC="RESET";
+disconnect con1;
+DROP TABLE t;

mysql-test/suite/innodb/t/cursor-restore-unique-null.test

+--source include/have_innodb.inc
+--source include/have_debug.inc
+--source include/have_debug_sync.inc
+--source include/count_sessions.inc
+
+
+CREATE TABLE t(a INT PRIMARY KEY, b INT, c INT, UNIQUE KEY `b_c` (`b`,`c`))
+  ENGINE=InnoDB, STATS_PERSISTENT=0;
+INSERT INTO t SET a = 1, c = 2;
+
+--connect con1,localhost,root
+BEGIN;
+  INSERT INTO t SET a=2, c=2;
+
+--connection default
+BEGIN;
+SET DEBUG_SYNC="lock_wait_suspend_thread_enter SIGNAL select_locked";
+--send SELECT * FROM t FORCE INDEX(b) FOR UPDATE
+
+--connection con1
+SET DEBUG_SYNC="now WAIT_FOR select_locked";
+ROLLBACK;
+
+--connection default
+--echo # If the bug is not fixed, and the both unique index key fields are
+--echo # NULL, there will be two (1, NULL, 2) rows in the result,
+--echo # because cursor will be restored to (NULL, 2, 1) position for
+--echo # secondary key instead of "supremum".
+--reap
+COMMIT;
+
+SET DEBUG_SYNC="RESET";
+
+--disconnect con1
+DROP TABLE t;
+--source include/wait_until_count_sessions.inc

storage/innobase/btr/btr0pcur.cc

@@ -453,8 +453,15 @@ btr_pcur_t::restore_position(ulint restore_latch_mode, const char *file,
 
 			return SAME_ALL;
 		}
-		if (n_matched_fields >= index->n_uniq)
-			ret_val= SAME_UNIQ;
+		if (n_matched_fields >= index->n_uniq
+		    /* Unique indexes can contain "NULL" keys, and if all
+		    unique fields are NULL and not all tuple
+		    fields match to record fields, then treat it as if
+		    restored cursor position points to the record with
+		    not the same unique key. */
+		    && !(index->n_nullable
+			    && dtuple_contains_null(tuple, index->n_uniq)))
+			  ret_val= SAME_UNIQ;
 	}
 
 	mem_heap_free(heap);

storage/innobase/dict/dict0dict.cc

@@ -1849,7 +1849,6 @@ dict_index_add_to_cache(
 	new_index->n_fields = new_index->n_def;
 	new_index->trx_id = index->trx_id;
 	new_index->set_committed(index->is_committed());
-	new_index->nulls_equal = index->nulls_equal;
 #ifdef MYSQL_INDEX_DISABLE_AHI
 	new_index->disable_ahi = index->disable_ahi;
 #endif

storage/innobase/include/data0data.h

@@ -349,15 +349,12 @@ dtuple_set_types_binary(
 	dtuple_t*	tuple,	/*!< in: data tuple */
 	ulint		n)	/*!< in: number of fields to set */
 	MY_ATTRIBUTE((nonnull));
-/**********************************************************************//**
-Checks if a dtuple contains an SQL null value.
-@return TRUE if some field is SQL null */
+/** Checks if a dtuple contains an SQL null value.
+@param tuple tuple
+@param fields_number number of fields in the tuple to check
+@return true if some field is SQL null */
 UNIV_INLINE
-ibool
-dtuple_contains_null(
-/*=================*/
-	const dtuple_t*	tuple)	/*!< in: dtuple */
-	MY_ATTRIBUTE((nonnull, warn_unused_result));
+bool dtuple_contains_null(const dtuple_t *tuple, ulint fields_number = 0);
 /**********************************************************//**
 Checks that a data field is typed. Asserts an error if not.
 @return TRUE if ok */

storage/innobase/include/data0data.inl

@@ -599,28 +599,18 @@ data_write_sql_null(
 	memset(data, 0, len);
 }
 
-/**********************************************************************//**
-Checks if a dtuple contains an SQL null value.
-@return TRUE if some field is SQL null */
+/** Checks if a dtuple contains an SQL null value.
+@param tuple tuple
+@param fields_number number of fields in the tuple to check
+@return true if some field is SQL null */
 UNIV_INLINE
-ibool
-dtuple_contains_null(
-/*=================*/
-	const dtuple_t*	tuple)	/*!< in: dtuple */
+bool dtuple_contains_null(const dtuple_t *tuple, ulint fields_number)
 {
-	ulint	n;
-	ulint	i;
-
-	n = dtuple_get_n_fields(tuple);
-
-	for (i = 0; i < n; i++) {
-		if (dfield_is_null(dtuple_get_nth_field(tuple, i))) {
-
-			return(TRUE);
-		}
-	}
-
-	return(FALSE);
+  ulint n= fields_number ? fields_number : dtuple_get_n_fields(tuple);
+  for (ulint i= 0; i < n; i++)
+    if (dfield_is_null(dtuple_get_nth_field(tuple, i)))
+      return true;
+  return false;
 }
 
 /**************************************************************//**

storage/innobase/include/dict0mem.h

@@ -1011,8 +1011,6 @@ struct dict_index_t {
 				/*!< number of columns the user defined to
 				be in the index: in the internal
 				representation we add more columns */
-	unsigned	nulls_equal:1;
-				/*!< if true, SQL NULL == SQL NULL */
 #ifdef BTR_CUR_HASH_ADAPT
 #ifdef MYSQL_INDEX_DISABLE_AHI
  	unsigned	disable_ahi:1;

storage/innobase/include/dict0mem.inl

@@ -63,7 +63,6 @@ dict_mem_fill_index_struct(
 	index->n_core_fields = (unsigned int) n_fields;
 	/* The '1 +' above prevents allocation
 	of an empty mem block */
-	index->nulls_equal = false;
 #ifdef BTR_CUR_HASH_ADAPT
 #ifdef MYSQL_INDEX_DISABLE_AHI
 	index->disable_ahi = false;

storage/innobase/row/row0ins.cc

@@ -2038,7 +2038,7 @@ row_ins_dupl_error_with_rec(
 	/* In a unique secondary index we allow equal key values if they
 	contain SQL NULLs */
 
-	if (!dict_index_is_clust(index) && !index->nulls_equal) {
+	if (!dict_index_is_clust(index)) {
 
 		for (i = 0; i < n_unique; i++) {
 			if (dfield_is_null(dtuple_get_nth_field(entry, i))) {
@@ -2143,16 +2143,8 @@ row_ins_scan_sec_index_for_duplicate(
 	/* If the secondary index is unique, but one of the fields in the
 	n_unique first fields is NULL, a unique key violation cannot occur,
 	since we define NULL != NULL in this case */
-
-	if (!index->nulls_equal) {
-		for (ulint i = 0; i < n_unique; i++) {
-			if (UNIV_SQL_NULL == dfield_get_len(
-					dtuple_get_nth_field(entry, i))) {
-
-				DBUG_RETURN(DB_SUCCESS);
-			}
-		}
-	}
+	if (index->n_nullable && dtuple_contains_null(entry, n_unique))
+		DBUG_RETURN(DB_SUCCESS);
 
 	/* Store old value on n_fields_cmp */