On Thu, Feb 22, 2018 at 9:09 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:

> Andres Freund <and...@anarazel.de> writes:
> > The more important part I think is that we solve it via a GUC that can
> > be used outside of bgworkers.
>
> Are you proposing an "ignore_datallowconn" GUC?  That's a remarkably
> bad idea.  We don't have infrastructure that would allow it to be set
> at an appropriate scope.  I can't imagine any good use-case for allowing
> it to be on globally; you'd want it to be per-session (or per-bgworker).
> But I don't think there's any way to change the system default setting
> in time for the setting to take effect during connection startup or
> bgworker startup.
>

I hacked up an attempt to do this. It does seem to work in the very simple
case, but it does requiring changing the order in InitPostgres() to load
the startup packet before validating those.

PFA WIP, which also shows how to do it in the background worker.

This one also lets me do

PGOPTIONS="-c ignore_connection_restrictionon" psql template0

to bypass the restriction, which is what pg_upgrade would basically do.


Magnus' most recent patch in this thread seems like a fine answer for
> bgworkers.  You've moved the goalposts into the next county, and the
> design you're proposing to satisfy that goal is lousy.  It will end
> up being a large amount of additional work with no benefit except
> being able to use an arguably less ugly (but almost certainly no
> shorter) datallowconn override method in pg_upgrade.
>

*If* the moving of the startup packet processing to one step earlier in
InitPostgres is safe, then it is actually less code this way right now.
>From a quick look I think it's safe to move it, but I haven't looked in
detail.

I also haven't checked if this actually helps in the pg_upgrade case, but
if bypassing datallowconn is what's needed there then it shuld.


-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
diff --git a/src/backend/postmaster/checksumhelper.c b/src/backend/postmaster/checksumhelper.c
index 44535f9976..997bffa416 100644
--- a/src/backend/postmaster/checksumhelper.c
+++ b/src/backend/postmaster/checksumhelper.c
@@ -586,6 +588,8 @@ ChecksumHelperWorkerMain(Datum arg)
 	ereport(DEBUG1,
 			(errmsg("Checksum worker starting for database oid %d", dboid)));
 
+	SetConfigOption("ignore_connection_restriction", "true", PGC_SU_BACKEND, PGC_S_OVERRIDE);
+
 	BackgroundWorkerInitializeConnectionByOid(dboid, InvalidOid);
 
 	/*
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 484628987f..7034697e1b 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -326,7 +326,7 @@ CheckMyDatabase(const char *name, bool am_superuser)
 		/*
 		 * Check that the database is currently allowing connections.
 		 */
-		if (!dbform->datallowconn)
+		if (!dbform->datallowconn && !IgnoreDatAllowConn)
 			ereport(FATAL,
 					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 					 errmsg("database \"%s\" is not currently accepting connections",
@@ -999,14 +999,6 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
 	/* set up ACL framework (so CheckMyDatabase can check permissions) */
 	initialize_acl();
 
-	/*
-	 * Re-read the pg_database row for our database, check permissions and set
-	 * up database-specific GUC settings.  We can't do this until all the
-	 * database-access infrastructure is up.  (Also, it wants to know if the
-	 * user is a superuser, so the above stuff has to happen first.)
-	 */
-	if (!bootstrap)
-		CheckMyDatabase(dbname, am_superuser);
 
 	/*
 	 * Now process any command-line switches and any additional GUC variable
@@ -1016,6 +1008,15 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
 	if (MyProcPort != NULL)
 		process_startup_options(MyProcPort, am_superuser);
 
+	/*
+	 * Re-read the pg_database row for our database, check permissions and set
+	 * up database-specific GUC settings.  We can't do this until all the
+	 * database-access infrastructure is up.  (Also, it wants to know if the
+	 * user is a superuser, so the above stuff has to happen first.)
+	 */
+	if (!bootstrap)
+		CheckMyDatabase(dbname, am_superuser);
+
 	/* Process pg_db_role_setting options */
 	process_settings(MyDatabaseId, GetSessionUserId());
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 039b63bb05..8ac6c7eefe 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -461,6 +461,7 @@ bool		row_security;
 bool		check_function_bodies = true;
 bool		default_with_oids = false;
 bool		session_auth_is_superuser;
+bool		IgnoreDatAllowConn;
 
 int			log_min_error_statement = ERROR;
 int			log_min_messages = WARNING;
@@ -1648,6 +1649,18 @@ static struct config_bool ConfigureNamesBool[] =
 	},
 
 	{
+		{"ignore_connection_restriction", PGC_SU_BACKEND, DEVELOPER_OPTIONS,
+			gettext_noop("Ignores the datallowconn restriction on database."),
+			NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&IgnoreDatAllowConn,
+		false,
+		NULL, NULL, NULL
+	},
+
+
+	{
 		{"lo_compat_privileges", PGC_SUSET, COMPAT_OPTIONS_PREVIOUS,
 			gettext_noop("Enables backward compatibility mode for privilege checks on large objects."),
 			gettext_noop("Skips privilege checks when reading or modifying large objects, "
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index a4574cd533..666508cfaf 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -424,6 +424,7 @@ extern void BaseInit(void);
 
 /* in utils/init/miscinit.c */
 extern bool IgnoreSystemIndexes;
+extern bool IgnoreDatAllowConn;
 extern PGDLLIMPORT bool process_shared_preload_libraries_in_progress;
 extern char *session_preload_libraries_string;
 extern char *shared_preload_libraries_string;

Reply via email to