v2-0001-Reserve-the-prefix-for-each-extension.patch

text/plain

Filename: v2-0001-Reserve-the-prefix-for-each-extension.patch
Type: text/plain
Part: 0
Message: Re: Reserve prefixes for loaded libraries proposal

Patch

Format: format-patch
Series: patch v2-0001
Subject: Reserve the prefix for each extension
File+
src/backend/utils/misc/guc.c 49 0
src/test/regress/expected/guc.out 21 0
src/test/regress/sql/guc.sql 10 0
From 9a50261faad3a7578dab150f1c05b510285e283c Mon Sep 17 00:00:00 2001
From: Florin Irion <florin.irion@enterprisedb.com>
Date: Thu, 7 Oct 2021 09:51:29 +0200
Subject: [PATCH] Reserve the prefix for each extension

Throw a warning when we `SET` a variable that doesn't exist with
a reserved prefix.  This will protect against setting incorrect
configurations for any extension, like we do with the `GUC`s.

Add a regression test that checks the patch using the "plpgsql"
registered prefix.
---
 src/backend/utils/misc/guc.c      | 49 +++++++++++++++++++++++++++++++
 src/test/regress/expected/guc.out | 21 +++++++++++++
 src/test/regress/sql/guc.sql      | 10 +++++++
 3 files changed, 80 insertions(+)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6652a60ec3..f19310df44 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -234,6 +234,8 @@ static bool check_recovery_target_lsn(char **newval, void **extra, GucSource sou
 static void assign_recovery_target_lsn(const char *newval, void *extra);
 static bool check_primary_slot_name(char **newval, void **extra, GucSource source);
 static bool check_default_with_oids(bool *newval, void **extra, GucSource source);
+static void EmitWarningsOnSetPlaceholders(const char *varName);
+static List *reserved_class_prefix = NIL;
 
 /* Private functions in guc-file.l that need to be called from guc.c */
 static ConfigVariable *ProcessConfigFileInternal(GucContext context,
@@ -8703,6 +8705,8 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 									 (superuser() ? PGC_SUSET : PGC_USERSET),
 									 PGC_S_SESSION,
 									 action, true, 0, false);
+
+			EmitWarningsOnSetPlaceholders(stmt->name);
 			break;
 		case VAR_SET_MULTI:
 
@@ -8789,6 +8793,8 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 									 (superuser() ? PGC_SUSET : PGC_USERSET),
 									 PGC_S_SESSION,
 									 action, true, 0, false);
+
+			EmitWarningsOnSetPlaceholders(stmt->name);
 			break;
 		case VAR_RESET_ALL:
 			ResetAllOptions();
@@ -9275,6 +9281,7 @@ EmitWarningsOnPlaceholders(const char *className)
 {
 	int			classLen = strlen(className);
 	int			i;
+	MemoryContext	oldcontext;
 
 	for (i = 0; i < num_guc_variables; i++)
 	{
@@ -9290,8 +9297,50 @@ EmitWarningsOnPlaceholders(const char *className)
 							var->name)));
 		}
 	}
+	oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+	reserved_class_prefix = lappend(reserved_class_prefix, pstrdup(className));
+	MemoryContextSwitchTo(oldcontext);
 }
 
+/*
+ * Throw a warning if a user creates a placeholder with
+ * the "SET" command and we don't recognize it.
+ */
+static void
+EmitWarningsOnSetPlaceholders(const char *varName)
+{
+	char 	*gucQualifierSeparator = strchr(varName, GUC_QUALIFIER_SEPARATOR);
+
+	if (gucQualifierSeparator)
+	{
+		unsigned int	classLen = gucQualifierSeparator - varName;
+		unsigned int	varLen = strlen(varName);
+		ListCell	   *lc;
+
+		foreach(lc, reserved_class_prefix)
+		{
+			char	*rcprefix = (char *) lfirst(lc);
+
+			if (strncmp(varName, rcprefix, classLen) == 0)
+			{
+				for (int i = 0; i < num_guc_variables; i++)
+				{
+					struct config_generic *var = guc_variables[i];
+
+					if ((var->flags & GUC_CUSTOM_PLACEHOLDER) != 0 &&
+						strncmp(varName, var->name, varLen)==0)
+					{
+						ereport(WARNING,
+								(errcode(ERRCODE_UNDEFINED_OBJECT),
+								 errmsg("unrecognized configuration parameter \"%s\"", var->name),
+								 errdetail("\"%.*s\" is a reserved prefix.", classLen, var->name),
+								 errhint("If you need to create a custom placeholder use a different prefix.")));
+					}
+				}
+			}
+		}
+	}
+}
 
 /*
  * SHOW command
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 59da91ff04..e5049bed19 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -813,3 +813,24 @@ set default_with_oids to f;
 -- Should not allow to set it to true.
 set default_with_oids to t;
 ERROR:  tables declared WITH OIDS are not supported
+-- test SET unrecongnized parameter
+SET foo = FALSE; -- no such setting
+ERROR:  unrecognized configuration parameter "foo"
+-- test setting a parameter with a registered prefix (plpgsql)
+SET plpgsql.extra_foo_warnings = FALSE; -- no such setting
+WARNING:  unrecognized configuration parameter "plpgsql.extra_foo_warnings"
+DETAIL:  "plpgsql" is a reserved prefix.
+HINT:  If you need to create a custom placeholder use a different prefix.
+SHOW plpgsql.extra_foo_warnings; -- the parameter is however set
+ plpgsql.extra_foo_warnings 
+----------------------------
+ false
+(1 row)
+
+-- cleanup
+RESET foo;
+ERROR:  unrecognized configuration parameter "foo"
+RESET plpgsql.extra_foo_warnings;
+WARNING:  unrecognized configuration parameter "plpgsql.extra_foo_warnings"
+DETAIL:  "plpgsql" is a reserved prefix.
+HINT:  If you need to create a custom placeholder use a different prefix.
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index c39c11388d..d9669d6e5a 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -311,3 +311,13 @@ reset check_function_bodies;
 set default_with_oids to f;
 -- Should not allow to set it to true.
 set default_with_oids to t;
+
+-- test SET unrecongnized parameter
+SET foo = FALSE; -- no such setting
+
+-- test setting a parameter with a registered prefix (plpgsql)
+SET plpgsql.extra_foo_warnings = FALSE; -- no such setting
+SHOW plpgsql.extra_foo_warnings; -- the parameter is however set
+-- cleanup
+RESET foo;
+RESET plpgsql.extra_foo_warnings;
\ No newline at end of file
-- 
2.31.1