pg_dump not dumping the run_as_owner setting from version 16?

Started by Philip Warnerabout 2 years ago6 messages
#1Philip Warner
pjw@rhyme.com.au

Hi,

I as far as I can tell, pg_dump does not dup the ‘run_as_owner` setting for a subscription.

Should it? Should I submit a patch? It seems pretty trivial to fix if anyone else is working on it.

Sent from Mail for Windows

#2Philip Warner
pjw@rhyme.com.au
In reply to: Philip Warner (#1)
RE: pg_dump not dumping the run_as_owner setting from version 16?

Further to this: it seems that `Alter Subscription X Set(Run_As_Owner=True);` has no influence on the `subrunasowner` column of pg_subscriptions.

Sent from Mail for Windows

From: Philip Warner
Sent: Friday, 27 October 2023 3:26 PM
To: pgsql-hackers@lists.postgresql.org
Subject: pg_dump not dumping the run_as_owner setting from version 16?

Hi,

I as far as I can tell, pg_dump does not dup the ‘run_as_owner` setting for a subscription.

Should it? Should I submit a patch? It seems pretty trivial to fix if anyone else is working on it.

Sent from Mail for Windows

#3Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Philip Warner (#2)
Re: pg_dump not dumping the run_as_owner setting from version 16?

On Fri, 2023-10-27 at 18:05 +1100, Philip Warner wrote:

I as far as I can tell, pg_dump does not dup the ‘run_as_owner` setting for a subscription.

Should it? Should I submit a patch? It seems pretty trivial to fix if anyone else is working on it.

Yes, it certainly should. That is an omission in 482675987b.
Go ahead and write a fix!

Further to this: it seems that `Alter Subscription X Set(Run_As_Owner=True);`
has no influence on the `subrunasowner` column of pg_subscriptions.

This seems to have been fixed in f062cddafe.

Yours,
Laurenz Albe

#4Philip Warner
pjw@rhyme.com.au
In reply to: Laurenz Albe (#3)
RE: pg_dump not dumping the run_as_owner setting from version 16?

I as far as I can tell, pg_dump does not dup the ‘run_as_owner` setting for a subscription.

Should it? Should I submit a patch? It seems pretty trivial to fix if anyone else is working on it.

Yes, it certainly should. That is an omission in 482675987b.
Go ahead and write a fix!

Please find attached a patch for pg_dump to honour the setting of `run_as_owner`; I believe that effective pre-16 behavious was to run as owner, so I have set the flag to ‘t’ for pre-16 versions. Please let me know if you would prefer the opposite.

Further to this: it seems that `Alter Subscription X Set(Run_As_Owner=True);`
has no influence on the `subrunasowner` column of pg_subscriptions.

This seems to have been fixed in f062cddafe.

Yes, I can confirm that in the current head `pg_subscriptions` reflects the setting correctly.

#5Philip Warner
pjw@rhyme.com.au
In reply to: Laurenz Albe (#3)
1 attachment(s)
RE: pg_dump not dumping the run_as_owner setting from version 16?

...patch actually attached this time...

I as far as I can tell, pg_dump does not dup the ‘run_as_owner` setting for a subscription.

Should it? Should I submit a patch? It seems pretty trivial to fix if anyone else is working on it.

Yes, it certainly should.  That is an omission in 482675987b.
Go ahead and write a fix!

Please find attached a patch for pg_dump to honour the setting of `run_as_owner`; I believe that effective pre-16 behavious was to run as owner, so I have set the flag to ‘t’ for pre-16 versions. Please let me know if you would prefer the opposite.

Further to this: it seems that `Alter Subscription X Set(Run_As_Owner=True);`
has no influence on the `subrunasowner` column of pg_subscriptions.

This seems to have been fixed in f062cddafe.

Yes, I can confirm that in the current head `pg_subscriptions` reflects the setting correctly.

Attachments:

pg_dump-run_as_owner.patchapplication/octet-stream; name=pg_dump-run_as_owner.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7afdbf4d9d..ab657e5254 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4600,6 +4600,7 @@ getSubscriptions(Archive *fout)
 	int			i_subtwophasestate;
 	int			i_subdisableonerr;
 	int			i_suborigin;
+	int			i_subrunasowner;
 	int			i_subconninfo;
 	int			i_subslotname;
 	int			i_subsynccommit;
@@ -4660,10 +4661,12 @@ getSubscriptions(Archive *fout)
 	if (fout->remoteVersion >= 160000)
 		appendPQExpBufferStr(query,
 							 " s.suborigin,\n"
+							 " s.subrunasowner,\n"
 							 " s.subpasswordrequired\n");
 	else
 		appendPQExpBuffer(query,
 						  " '%s' AS suborigin,\n"
+						  " 't' AS subrunasowner,\n"
 						  " 't' AS subpasswordrequired\n",
 						  LOGICALREP_ORIGIN_ANY);
 
@@ -4685,6 +4688,7 @@ getSubscriptions(Archive *fout)
 	i_subname = PQfnumber(res, "subname");
 	i_subowner = PQfnumber(res, "subowner");
 	i_subconninfo = PQfnumber(res, "subconninfo");
+	i_subrunasowner = PQfnumber(res, "subrunasowner");
 	i_subslotname = PQfnumber(res, "subslotname");
 	i_subsynccommit = PQfnumber(res, "subsynccommit");
 	i_subpublications = PQfnumber(res, "subpublications");
@@ -4707,6 +4711,7 @@ getSubscriptions(Archive *fout)
 		subinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_subname));
 		subinfo[i].rolname = getRoleName(PQgetvalue(res, i, i_subowner));
 		subinfo[i].subconninfo = pg_strdup(PQgetvalue(res, i, i_subconninfo));
+		subinfo[i].subrunasowner = pg_strdup(PQgetvalue(res, i, i_subrunasowner));
 		if (PQgetisnull(res, i, i_subslotname))
 			subinfo[i].subslotname = NULL;
 		else
@@ -4810,6 +4815,9 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
 	if (strcmp(subinfo->subpasswordrequired, "t") != 0)
 		appendPQExpBuffer(query, ", password_required = false");
 
+	if (strcmp(subinfo->subrunasowner, "t") == 0)
+		appendPQExpBufferStr(query, ", run_as_owner = true");
+
 	appendPQExpBufferStr(query, ");\n");
 
 	if (subinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index d8f27f187c..bcba4a1ef0 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -661,6 +661,7 @@ typedef struct _SubscriptionInfo
 	DumpableObject dobj;
 	const char *rolname;
 	char	   *subconninfo;
+	char	   *subrunasowner;
 	char	   *subslotname;
 	char	   *subbinary;
 	char	   *substream;
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Philip Warner (#5)
Re: pg_dump not dumping the run_as_owner setting from version 16?

Philip Warner <pjw@rhyme.com.au> writes:

Please find attached a patch for pg_dump to honour the setting of `run_as_owner`; I believe that effective pre-16 behavious was to run as owner, so I have set the flag to ‘t’ for pre-16 versions. Please let me know if you would prefer the opposite.

I think that's the correct choice. Fix pushed, thanks.

regards, tom lane