MDEV-28478: INSERT into SPATIAL INDEX in TEMPORARY table writes log

dr-m · dr-m · commit 26d46234e831 · 2022-05-06T10:12:31.000+03:00
This is based on commit 20ae481 with some adjustments for MDEV-12353. row_ins_sec_index_entry_low(): If a separate mini-transaction is needed to adjust the minimum bounding rectangle (MBR) in the parent page, we must disable redo logging if the table is a temporary table. For temporary tables, no log is supposed to be written, because the temporary tablespace will be reinitialized on server restart. rtr_update_mbr_field(), rtr_merge_and_update_mbr(): Changed the return type to void and removed unreachable code. In older versions, these used to return a different value for temporary tables. page_id_t: Add constexpr to most member functions. mtr_t::log_write(): Catch log writes to invalid tablespaces so that the test case would crash without the fix to row_ins_sec_index_entry_low().
diff --git a/mysql-test/suite/innodb_gis/r/rtree_split.result b/mysql-test/suite/innodb_gis/r/rtree_split.result
@@ -61,10 +61,3 @@ select count(*) from t1 where MBRWithin(t1.c2, @g1);
 count(*)
 57344
 drop table t1;
-#
-# MDEV-27417 Spatial index tries to update
-#	change buffer bookkeeping page
-#
-CREATE TEMPORARY TABLE t1 (c POINT NOT NULL, SPATIAL(c)) ENGINE=InnoDB;
-INSERT INTO t1 SELECT PointFromText('POINT(0 0)') FROM seq_1_to_366;
-DROP TABLE t1;
diff --git a/mysql-test/suite/innodb_gis/r/rtree_temporary.result b/mysql-test/suite/innodb_gis/r/rtree_temporary.result
@@ -0,0 +1,14 @@
+#
+# MDEV-27417 Spatial index tries to update
+#	change buffer bookkeeping page
+#
+CREATE TEMPORARY TABLE t1 (c POINT NOT NULL, SPATIAL(c)) ENGINE=InnoDB;
+INSERT INTO t1 SELECT PointFromText('POINT(0 0)') FROM seq_1_to_366;
+DROP TABLE t1;
+#
+# MDEV-28478 Assertion mtr->get_log_mode() == MTR_LOG_NO_REDO
+#
+CREATE TEMPORARY TABLE t1 (c POINT NOT NULL,SPATIAL (c)) ENGINE=InnoDB;
+INSERT INTO t1 SELECT POINT(0,0) FROM seq_1_to_366;
+INSERT INTO t1 VALUES (POINT(1e-270,1e-130));
+DROP TABLE t1;
diff --git a/mysql-test/suite/innodb_gis/t/rtree_split.test b/mysql-test/suite/innodb_gis/t/rtree_split.test
@@ -73,11 +73,3 @@ select count(*) from t1 where MBRWithin(t1.c2, @g1);
 
 # Clean up.
 drop table t1;
-
---echo #
---echo # MDEV-27417 Spatial index tries to update
---echo #	change buffer bookkeeping page
---echo #
-CREATE TEMPORARY TABLE t1 (c POINT NOT NULL, SPATIAL(c)) ENGINE=InnoDB;
-INSERT INTO t1 SELECT PointFromText('POINT(0 0)') FROM seq_1_to_366;
-DROP TABLE t1;
diff --git a/mysql-test/suite/innodb_gis/t/rtree_temporary.test b/mysql-test/suite/innodb_gis/t/rtree_temporary.test
@@ -0,0 +1,18 @@
+--source include/have_innodb.inc
+--source include/have_sequence.inc
+
+--echo #
+--echo # MDEV-27417 Spatial index tries to update
+--echo #	change buffer bookkeeping page
+--echo #
+CREATE TEMPORARY TABLE t1 (c POINT NOT NULL, SPATIAL(c)) ENGINE=InnoDB;
+INSERT INTO t1 SELECT PointFromText('POINT(0 0)') FROM seq_1_to_366;
+DROP TABLE t1;
+
+--echo #
+--echo # MDEV-28478 Assertion mtr->get_log_mode() == MTR_LOG_NO_REDO
+--echo #
+CREATE TEMPORARY TABLE t1 (c POINT NOT NULL,SPATIAL (c)) ENGINE=InnoDB;
+INSERT INTO t1 SELECT POINT(0,0) FROM seq_1_to_366;
+INSERT INTO t1 VALUES (POINT(1e-270,1e-130));
+DROP TABLE t1;
diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/btr0btr.cc
@@ -3588,17 +3588,9 @@ btr_compress(
 			}
 
 			if (mbr_changed) {
-#ifdef UNIV_DEBUG
-				bool	success = rtr_update_mbr_field(
-					&cursor2, offsets2, &father_cursor,
-					merge_page, &new_mbr, NULL, mtr);
-
-				ut_ad(success);
-#else
 				rtr_update_mbr_field(
 					&cursor2, offsets2, &father_cursor,
 					merge_page, &new_mbr, NULL, mtr);
-#endif
 			} else {
 				rtr_node_ptr_delete(&father_cursor, mtr);
 			}
diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc
@@ -5834,7 +5834,6 @@ btr_cur_pessimistic_delete(
 			rec_t*		father_rec;
 			btr_cur_t	father_cursor;
 			rec_offs*	offsets;
-			bool		upd_ret;
 			ulint		len;
 
 			rtr_page_get_father_block(NULL, heap, index,
@@ -5848,17 +5847,8 @@ btr_cur_pessimistic_delete(
 			rtr_read_mbr(rec_get_nth_field(
 				father_rec, offsets, 0, &len), &father_mbr);
 
-			upd_ret = rtr_update_mbr_field(&father_cursor, offsets,
-						       NULL, page, &father_mbr,
-						       next_rec, mtr);
-
-			if (!upd_ret) {
-				*err = DB_ERROR;
-
-				mem_heap_free(heap);
-				return(FALSE);
-			}
-
+			rtr_update_mbr_field(&father_cursor, offsets, NULL,
+					     page, &father_mbr, next_rec, mtr);
 			ut_d(parent_latched = true);
 		} else {
 			/* Otherwise, if we delete the leftmost node pointer
diff --git a/storage/innobase/gis/gis0rtree.cc b/storage/innobase/gis/gis0rtree.cc
@@ -1,7 +1,13 @@
 /*****************************************************************************
 
 Copyright (c) 2016, Oracle and/or its affiliates. All Rights Reserved.
+<<<<<<< HEAD
 Copyright (c) 2018, 2021, MariaDB Corporation.
+||||||| parent of 20ae4816bba (MDEV-28478: INSERT into SPATIAL INDEX in TEMPORARY table writes log)
+Copyright (c) 2019, 2020, MariaDB Corporation.
+=======
+Copyright (c) 2019, 2022, MariaDB Corporation.
+>>>>>>> 20ae4816bba (MDEV-28478: INSERT into SPATIAL INDEX in TEMPORARY table writes log)
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -185,9 +191,8 @@ rtr_index_build_node_ptr(
 }
 
 /**************************************************************//**
-Update the mbr field of a spatial index row.
-@return true if update is successful */
-bool
+Update the mbr field of a spatial index row. */
+void
 rtr_update_mbr_field(
 /*=================*/
 	btr_cur_t*	cursor,		/*!< in/out: cursor pointed to rec.*/
@@ -535,8 +540,6 @@ rtr_update_mbr_field(
 			  page_is_comp(page))));
 
 	mem_heap_free(heap);
-
-	return(true);
 }
 
 /**************************************************************//**
@@ -1264,12 +1267,8 @@ rtr_ins_enlarge_mbr(
 		page = buf_block_get_frame(block);
 
 		/* Update the mbr field of the rec. */
-		if (!rtr_update_mbr_field(&cursor, offsets, NULL, page,
-					  &new_mbr, NULL, mtr)) {
-			err = DB_ERROR;
-			break;
-		}
-
+		rtr_update_mbr_field(&cursor, offsets, NULL, page,
+				     &new_mbr, NULL, mtr);
 		page_cursor = btr_cur_get_page_cur(&cursor);
 		block = page_cur_get_block(page_cursor);
 	}
@@ -1579,7 +1578,7 @@ rtr_merge_mbr_changed(
 
 /****************************************************************//**
 Merge 2 mbrs and update the the mbr that cursor is on. */
-dberr_t
+void
 rtr_merge_and_update_mbr(
 /*=====================*/
 	btr_cur_t*		cursor,		/*!< in/out: cursor */
@@ -1589,27 +1588,15 @@ rtr_merge_and_update_mbr(
 	page_t*			child_page,	/*!< in: the page. */
 	mtr_t*			mtr)		/*!< in: mtr */
 {
-	dberr_t			err = DB_SUCCESS;
 	rtr_mbr_t		new_mbr;
-	bool			changed = false;
-
-	ut_ad(dict_index_is_spatial(cursor->index));
 
-	changed = rtr_merge_mbr_changed(cursor, cursor2, offsets, offsets2,
-					&new_mbr);
-
-	/* Update the mbr field of the rec. And will delete the record
-	pointed by cursor2 */
-	if (changed) {
-		if (!rtr_update_mbr_field(cursor, offsets, cursor2, child_page,
-					  &new_mbr, NULL, mtr)) {
-			err = DB_ERROR;
-		}
+	if (rtr_merge_mbr_changed(cursor, cursor2, offsets, offsets2,
+                                  &new_mbr)) {
+		rtr_update_mbr_field(cursor, offsets, cursor2, child_page,
+				     &new_mbr, NULL, mtr);
 	} else {
 		rtr_node_ptr_delete(cursor2, mtr);
 	}
-
-	return(err);
 }
 
 /*************************************************************//**
diff --git a/storage/innobase/include/buf0types.h b/storage/innobase/include/buf0types.h
@@ -24,9 +24,7 @@ The database buffer pool global types for the directory
 Created 11/17/1995 Heikki Tuuri
 *******************************************************/
 
-#ifndef buf0types_h
-#define buf0types_h
-
+#pragma once
 #include "univ.i"
 
 /** Buffer page (uncompressed or compressed) */
@@ -122,18 +120,22 @@ class page_id_t
   /** Constructor from (space, page_no).
   @param[in]	space	tablespace id
   @param[in]	page_no	page number */
-  page_id_t(ulint space, uint32_t page_no) : m_id(uint64_t{space} << 32 | page_no)
-  {
-    ut_ad(space <= 0xFFFFFFFFU);
-  }
-
-  page_id_t(uint64_t id) : m_id(id) {}
-  bool operator==(const page_id_t& rhs) const { return m_id == rhs.m_id; }
-  bool operator!=(const page_id_t& rhs) const { return m_id != rhs.m_id; }
-  bool operator<(const page_id_t& rhs) const { return m_id < rhs.m_id; }
-  bool operator>(const page_id_t& rhs) const { return m_id > rhs.m_id; }
-  bool operator<=(const page_id_t& rhs) const { return m_id <= rhs.m_id; }
-  bool operator>=(const page_id_t& rhs) const { return m_id >= rhs.m_id; }
+  constexpr page_id_t(ulint space, uint32_t page_no) :
+    m_id(uint64_t{space} << 32 | page_no) {}
+
+  constexpr page_id_t(uint64_t id) : m_id(id) {}
+  constexpr bool operator==(const page_id_t& rhs) const
+  { return m_id == rhs.m_id; }
+  constexpr bool operator!=(const page_id_t& rhs) const
+  { return m_id != rhs.m_id; }
+  constexpr bool operator<(const page_id_t& rhs) const
+  { return m_id < rhs.m_id; }
+  constexpr bool operator>(const page_id_t& rhs) const
+  { return m_id > rhs.m_id; }
+  constexpr bool operator<=(const page_id_t& rhs) const
+  { return m_id <= rhs.m_id; }
+  constexpr bool operator>=(const page_id_t& rhs) const
+  { return m_id >= rhs.m_id; }
   page_id_t &operator--() { ut_ad(page_no()); m_id--; return *this; }
   page_id_t &operator++()
   {
@@ -154,15 +156,16 @@ class page_id_t
 
   /** Retrieve the tablespace id.
   @return tablespace id */
-  uint32_t space() const { return static_cast<uint32_t>(m_id >> 32); }
+  constexpr uint32_t space() const { return static_cast<uint32_t>(m_id >> 32); }
 
   /** Retrieve the page number.
   @return page number */
-  uint32_t page_no() const { return static_cast<uint32_t>(m_id); }
+  constexpr uint32_t page_no() const { return static_cast<uint32_t>(m_id); }
 
   /** Retrieve the fold value.
   @return fold value */
-  ulint fold() const { return (ulint{space()} << 20) + space() + page_no(); }
+  constexpr ulint fold() const
+  { return (ulint{space()} << 20) + space() + page_no(); }
 
   /** Reset the page number only.
   @param[in]	page_no	page number */
@@ -171,7 +174,8 @@ class page_id_t
     m_id= (m_id & ~uint64_t{0} << 32) | page_no;
   }
 
-  ulonglong raw() { return m_id; }
+  constexpr ulonglong raw() { return m_id; }
+
 private:
   /** The page identifier */
   uint64_t m_id;
@@ -221,5 +225,3 @@ class page_hash_latch : public rw_lock
 };
 
 #endif /* !UNIV_INNOCHECKSUM */
-
-#endif /* buf0types.h */
diff --git a/storage/innobase/include/gis0rtree.h b/storage/innobase/include/gis0rtree.h
@@ -379,7 +379,7 @@ rtr_page_copy_rec_list_start_no_locks(
 
 /****************************************************************//**
 Merge 2 mbrs and update the the mbr that cursor is on. */
-dberr_t
+void
 rtr_merge_and_update_mbr(
 /*=====================*/
 	btr_cur_t*		cursor,		/*!< in/out: cursor */
@@ -411,9 +411,8 @@ rtr_merge_mbr_changed(
 
 
 /**************************************************************//**
-Update the mbr field of a spatial index row.
-@return true if successful */
-bool
+Update the mbr field of a spatial index row. */
+void
 rtr_update_mbr_field(
 /*=================*/
 	btr_cur_t*	cursor,		/*!< in: cursor pointed to rec.*/
diff --git a/storage/innobase/include/mtr0log.h b/storage/innobase/include/mtr0log.h
@@ -1,6 +1,6 @@
 /*****************************************************************************
 
-Copyright (c) 2019, 2020, MariaDB Corporation.
+Copyright (c) 2019, 2022, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -24,6 +24,9 @@ Mini-transaction log record encoding and decoding
 #pragma once
 #include "mtr0mtr.h"
 
+/** The smallest invalid page identifier for persistent tablespaces */
+constexpr page_id_t end_page_id{SRV_SPACE_ID_UPPER_BOUND, 0};
+
 /** The minimum 2-byte integer (0b10xxxxxx xxxxxxxx) */
 constexpr uint32_t MIN_2BYTE= 1 << 7;
 /** The minimum 3-byte integer (0b110xxxxx xxxxxxxx xxxxxxxx) */
@@ -388,6 +391,7 @@ inline byte *mtr_t::log_write(const page_id_t id, const buf_page_t *bpage,
                 type <= FILE_CHECKPOINT, "invalid type");
   ut_ad(type >= FILE_CREATE || is_named_space(id.space()));
   ut_ad(!bpage || bpage->id() == id);
+  ut_ad(id < end_page_id);
   constexpr bool have_len= type != INIT_PAGE && type != FREE_PAGE;
   constexpr bool have_offset= type == WRITE || type == MEMSET ||
     type == MEMMOVE;
diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc
@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2016, 2021, MariaDB Corporation.
+Copyright (c) 2016, 2022, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -2952,8 +2952,12 @@ row_ins_sec_index_entry_low(
 			rtr_init_rtr_info(&rtr_info, false, &cursor,
 					  index, false);
 			rtr_info_update_btr(&cursor, &rtr_info);
-			mtr_start(&mtr);
-			index->set_modified(mtr);
+			mtr.start();
+			if (index->table->is_temporary()) {
+				mtr.set_log_mode(MTR_LOG_NO_REDO);
+			} else {
+				index->set_modified(mtr);
+			}
 			search_mode &= ulint(~BTR_MODIFY_LEAF);
 			search_mode |= BTR_MODIFY_TREE;
 			err = btr_cur_search_to_nth_level(
