Custom reloptions and lock modes

From c0c2f75ae0b4fa3e6959a1157d400e316f40ada0 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 23 Sep 2019 15:20:37 +0900
Subject: [PATCH v2 1/3] Fix failure with lock mode used for custom relation
 options

Relation options can use a custom lock mode since 47167b7, which has
lowered the lock available for some autovacuum parameters, however it
forgot to consider custom relation options.  This causes failures with
ALTER TABLE SET when changing a custom relation option, as its lock is
not defined.  The existing APIs to define a custom reloption does not
allow to define a custom lock mode, so enforce its initialization to
AccessExclusiveMode which is safe enough in all cases.  An upcoming
patch will extend the existing APIs to allow a custom lock mode to be
defined.

The problem can be reproduced with bloom indexes, so add a test there.

Reported-by: Nikolay Sharplov
Analyzed-by: Thomas Munro, Michael Paquier
Author: Michael Paquier
Reviewed-by: Kuntal Ghosh
Discussion: https://postgr.es/m/20190920013831.GD1844@paquier.xyz
Backpatch-through: 9.6
---
 contrib/bloom/expected/bloom.out       | 1 +
 contrib/bloom/sql/bloom.sql            | 1 +
 src/backend/access/common/reloptions.c | 7 +++++++
 3 files changed, 9 insertions(+)

diff --git a/contrib/bloom/expected/bloom.out b/contrib/bloom/expected/bloom.out
index 5ab9e34f82..dae12a7d3e 100644
--- a/contrib/bloom/expected/bloom.out
+++ b/contrib/bloom/expected/bloom.out
@@ -5,6 +5,7 @@ CREATE TABLE tst (
 );
 INSERT INTO tst SELECT i%10, substr(md5(i::text), 1, 1) FROM generate_series(1,2000) i;
 CREATE INDEX bloomidx ON tst USING bloom (i, t) WITH (col1 = 3);
+ALTER INDEX bloomidx SET (length=80);
 SET enable_seqscan=on;
 SET enable_bitmapscan=off;
 SET enable_indexscan=off;
diff --git a/contrib/bloom/sql/bloom.sql b/contrib/bloom/sql/bloom.sql
index 32755f2b1a..4733e1e705 100644
--- a/contrib/bloom/sql/bloom.sql
+++ b/contrib/bloom/sql/bloom.sql
@@ -7,6 +7,7 @@ CREATE TABLE tst (
 
 INSERT INTO tst SELECT i%10, substr(md5(i::text), 1, 1) FROM generate_series(1,2000) i;
 CREATE INDEX bloomidx ON tst USING bloom (i, t) WITH (col1 = 3);
+ALTER INDEX bloomidx SET (length=80);
 
 SET enable_seqscan=on;
 SET enable_bitmapscan=off;
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 20f4ed3c38..b59e606771 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -659,6 +659,13 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc)
 	newoption->namelen = strlen(name);
 	newoption->type = type;
 
+	/*
+	 * Set the default lock mode for this option.  There is no actual way
+	 * for a module to enforce it when declaring a custom relation option,
+	 * so just use the highest level, which is safe for all cases.
+	 */
+	newoption->lockmode = AccessExclusiveLock;
+
 	MemoryContextSwitchTo(oldcxt);
 
 	return newoption;
-- 
2.23.0

From c495f03450b6c4727b3d2f44b3509916f0ee73ae Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 23 Sep 2019 15:28:02 +0900
Subject: [PATCH v2 2/3] Allow definition of lock mode for custom reloptions

Relation options can define a lock mode other than AccessExclusiveMode
since 47167b7, but modules defining custom relation options did not
really have a way to enforce that.  Correct that by extending the
current API set so as modules can define a custom lock mode.

Author: Michael Paquier
Reviewed-by: Kuntal Ghosh
Discussion: https://postgr.es/m/20190920013831.GD1844@paquier.xyz
---
 contrib/bloom/blutils.c                |  6 ++++--
 src/backend/access/common/reloptions.c | 28 +++++++++++---------------
 src/include/access/reloptions.h        | 11 ++++++----
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index cc1670934f..dbb24cb5b2 100644
--- a/contrib/bloom/blutils.c
+++ b/contrib/bloom/blutils.c
@@ -60,7 +60,8 @@ _PG_init(void)
 	/* Option for length of signature */
 	add_int_reloption(bl_relopt_kind, "length",
 					  "Length of signature in bits",
-					  DEFAULT_BLOOM_LENGTH, 1, MAX_BLOOM_LENGTH);
+					  DEFAULT_BLOOM_LENGTH, 1, MAX_BLOOM_LENGTH,
+					  AccessExclusiveLock);
 	bl_relopt_tab[0].optname = "length";
 	bl_relopt_tab[0].opttype = RELOPT_TYPE_INT;
 	bl_relopt_tab[0].offset = offsetof(BloomOptions, bloomLength);
@@ -71,7 +72,8 @@ _PG_init(void)
 		snprintf(buf, sizeof(buf), "col%d", i + 1);
 		add_int_reloption(bl_relopt_kind, buf,
 						  "Number of bits generated for each index column",
-						  DEFAULT_BLOOM_BITS, 1, MAX_BLOOM_BITS);
+						  DEFAULT_BLOOM_BITS, 1, MAX_BLOOM_BITS,
+						  AccessExclusiveLock);
 		bl_relopt_tab[i + 1].optname = MemoryContextStrdup(TopMemoryContext,
 														   buf);
 		bl_relopt_tab[i + 1].opttype = RELOPT_TYPE_INT;
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index b59e606771..3b8517efea 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -621,7 +621,8 @@ add_reloption(relopt_gen *newoption)
  *		(for types other than string)
  */
 static relopt_gen *
-allocate_reloption(bits32 kinds, int type, const char *name, const char *desc)
+allocate_reloption(bits32 kinds, int type, const char *name, const char *desc,
+				   LOCKMODE lockmode)
 {
 	MemoryContext oldcxt;
 	size_t		size;
@@ -658,13 +659,7 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc)
 	newoption->kinds = kinds;
 	newoption->namelen = strlen(name);
 	newoption->type = type;
-
-	/*
-	 * Set the default lock mode for this option.  There is no actual way
-	 * for a module to enforce it when declaring a custom relation option,
-	 * so just use the highest level, which is safe for all cases.
-	 */
-	newoption->lockmode = AccessExclusiveLock;
+	newoption->lockmode = lockmode;
 
 	MemoryContextSwitchTo(oldcxt);
 
@@ -676,12 +671,13 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc)
  *		Add a new boolean reloption
  */
 void
-add_bool_reloption(bits32 kinds, const char *name, const char *desc, bool default_val)
+add_bool_reloption(bits32 kinds, const char *name, const char *desc,
+				   bool default_val, LOCKMODE lockmode)
 {
 	relopt_bool *newoption;
 
 	newoption = (relopt_bool *) allocate_reloption(kinds, RELOPT_TYPE_BOOL,
-												   name, desc);
+												   name, desc, lockmode);
 	newoption->default_val = default_val;
 
 	add_reloption((relopt_gen *) newoption);
@@ -693,12 +689,12 @@ add_bool_reloption(bits32 kinds, const char *name, const char *desc, bool defaul
  */
 void
 add_int_reloption(bits32 kinds, const char *name, const char *desc, int default_val,
-				  int min_val, int max_val)
+				  int min_val, int max_val, LOCKMODE lockmode)
 {
 	relopt_int *newoption;
 
 	newoption = (relopt_int *) allocate_reloption(kinds, RELOPT_TYPE_INT,
-												  name, desc);
+												  name, desc, lockmode);
 	newoption->default_val = default_val;
 	newoption->min = min_val;
 	newoption->max = max_val;
@@ -712,12 +708,12 @@ add_int_reloption(bits32 kinds, const char *name, const char *desc, int default_
  */
 void
 add_real_reloption(bits32 kinds, const char *name, const char *desc, double default_val,
-				   double min_val, double max_val)
+				   double min_val, double max_val, LOCKMODE lockmode)
 {
 	relopt_real *newoption;
 
 	newoption = (relopt_real *) allocate_reloption(kinds, RELOPT_TYPE_REAL,
-												   name, desc);
+												   name, desc, lockmode);
 	newoption->default_val = default_val;
 	newoption->min = min_val;
 	newoption->max = max_val;
@@ -736,7 +732,7 @@ add_real_reloption(bits32 kinds, const char *name, const char *desc, double defa
  */
 void
 add_string_reloption(bits32 kinds, const char *name, const char *desc, const char *default_val,
-					 validate_string_relopt validator)
+					 validate_string_relopt validator, LOCKMODE lockmode)
 {
 	relopt_string *newoption;
 
@@ -745,7 +741,7 @@ add_string_reloption(bits32 kinds, const char *name, const char *desc, const cha
 		(validator) (default_val);
 
 	newoption = (relopt_string *) allocate_reloption(kinds, RELOPT_TYPE_STRING,
-													 name, desc);
+													 name, desc, lockmode);
 	newoption->validate_cb = validator;
 	if (default_val)
 	{
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 6d392e4d5a..4b82c6370a 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -247,13 +247,16 @@ typedef struct
 
 extern relopt_kind add_reloption_kind(void);
 extern void add_bool_reloption(bits32 kinds, const char *name, const char *desc,
-							   bool default_val);
+							   bool default_val, LOCKMODE lockmode);
 extern void add_int_reloption(bits32 kinds, const char *name, const char *desc,
-							  int default_val, int min_val, int max_val);
+							  int default_val, int min_val, int max_val,
+							  LOCKMODE lockmode);
 extern void add_real_reloption(bits32 kinds, const char *name, const char *desc,
-							   double default_val, double min_val, double max_val);
+							   double default_val, double min_val, double max_val,
+							   LOCKMODE lockmode);
 extern void add_string_reloption(bits32 kinds, const char *name, const char *desc,
-								 const char *default_val, validate_string_relopt validator);
+								 const char *default_val, validate_string_relopt validator,
+								 LOCKMODE lockmode);
 
 extern Datum transformRelOptions(Datum oldOptions, List *defList,
 								 const char *namspace, char *validnsps[],
-- 
2.23.0