MDEV-34108 Inappropriate semi-consistent read in RC if innodb_snapshot_isolation=ON

The fixes in b8a67198 have not disabled
semi-consistent read for innodb_snapshot_isolation=ON mode, they just allowed
read uncommitted version of a record, that's why the test for MDEV-26643 worked
well.

The semi-consistent read should be disabled on upper level in
row_search_mvcc() for READ COMMITTED isolation level.

Reviewed by Marko Mäkelä.

mysql-test/suite/innodb/r/lock_isolation.result

@@ -103,6 +103,27 @@ SELECT * FROM t;
 a	b
 10	1
 10	20
+TRUNCATE TABLE t;
+#
+# MDEV-34108 Inappropriate semi-consistent read in snapshot isolation
+#
+INSERT INTO t VALUES(NULL, 1), (1, 1);
+BEGIN;
+UPDATE t SET b = 3;
+connection consistent;
+SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
+BEGIN;
+UPDATE t SET b = 2 WHERE a;
+connection default;
+UPDATE t SET a = 1;
+COMMIT;
+connection consistent;
+COMMIT;
+connection default;
+SELECT * FROM t;
+a	b
+1	2
+1	2
 DROP TABLE t;
 #
 # MDEV-33802 Weird read view after ROLLBACK of other transactions

mysql-test/suite/innodb/t/lock_isolation.test

@@ -103,6 +103,40 @@ COMMIT;
 --reap
 COMMIT;
 
+--connection default
+SELECT * FROM t;
+TRUNCATE TABLE t;
+
+--echo #
+--echo # MDEV-34108 Inappropriate semi-consistent read in snapshot isolation
+--echo #
+INSERT INTO t VALUES(NULL, 1), (1, 1);
+BEGIN;
+UPDATE t SET b = 3;
+
+--connection consistent
+SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
+BEGIN;
+# As semi-consistent read is disabled for innodb_snapshot_isolation=ON, the
+# following UPDATE must be blocked on the first record.
+--send UPDATE t SET b = 2 WHERE a
+
+--connection default
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = 'Updating' and info = 'UPDATE t SET b = 2 WHERE a';
+--source include/wait_condition.inc
+
+UPDATE t SET a = 1;
+COMMIT;
+--connection consistent
+# If the bug wouldn't be fixed, the result would be (1,3),(1,2), because
+# "UPDATE t SET b = 2 WHERE a" would be blocked on the second (1,3) record,
+# as semi-consistent read would filter out the first (null,3) record without
+# blocking.
+--reap
+COMMIT;
+
 --connection default
 SELECT * FROM t;
 DROP TABLE t;

storage/innobase/row/row0sel.cc

@@ -866,6 +866,8 @@ row_sel_build_committed_vers_for_mysql(
 	mtr_t*		mtr)		/*!< in: mtr */
 {
 	if (prebuilt->trx->snapshot_isolation) {
+		ut_ad(prebuilt->trx->isolation_level
+		    == TRX_ISO_READ_UNCOMMITTED);
 		*old_vers = rec;
 		return;
 	}
@@ -5261,7 +5263,20 @@ row_search_mvcc(
 			if (UNIV_LIKELY(prebuilt->row_read_type
 					!= ROW_READ_TRY_SEMI_CONSISTENT)
 			    || unique_search
-			    || index != clust_index) {
+			    || index != clust_index
+			    /* If read view was opened, sel_set_rec_lock()
+			    would return DB_RECORD_CHANGED, and we would not be
+			    here. As read view wasn't opened, do locking read
+			    instead of semi-consistent one for READ COMMITTED.
+			    For READ UNCOMMITTED
+			    row_sel_build_committed_vers_for_mysql() must read
+			    uncommitted version of the record. For REPEATABLE
+			    READ and SERIALIZABLE prebuilt->row_read_type
+			    must be not equal to ROW_READ_TRY_SEMI_CONSISTENT,
+			    so there will be locking read for those isolation
+			    levels. */
+			    || (trx->snapshot_isolation && trx->isolation_level
+			      == TRX_ISO_READ_COMMITTED )) {
 				if (!prebuilt->skip_locked) {
 					goto lock_wait_or_error;
 				}