Create publication syntax is not coming properly in pg_dump / pg_dumpall

Started by tusharover 8 years ago4 messages
#1tushar
tushar.ahuja@enterprisedb.com

Hi,

I observed that in pg_dump/pg_dumpall - 'create publication' syntax is
not coming properly if only specified value is mentioned in publish.

Testcase to reproduce -

\\create a publication

postgres=# CREATE PUBLICATION abc for all tables with (publish='insert');
CREATE PUBLICATION

\\take the plain dump

[centos@centos-cpula bin]$ ./pg_dump -FP -p 5000 postgres > /tmp/a.a

\\check the syntax

[centos@centos-cpula bin]$ cat /tmp/a.a |grep 'create publication abc' -i
CREATE PUBLICATION abc FOR ALL TABLES WITH (publish = 'insert, , ');

\\try to execute the same syntax against psql terminal

postgres=# CREATE PUBLICATION abc FOR ALL TABLES WITH (publish =
'insert, , ');
ERROR: invalid publish list

Same is valid for pg_dumpall as well..

--
regards,tushar
EnterpriseDB https://www.enterprisedb.com/
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Masahiko Sawada
sawada.mshk@gmail.com
In reply to: tushar (#1)
1 attachment(s)
Re: Create publication syntax is not coming properly in pg_dump / pg_dumpall

On Mon, May 15, 2017 at 5:04 PM, tushar <tushar.ahuja@enterprisedb.com> wrote:

Hi,

I observed that in pg_dump/pg_dumpall - 'create publication' syntax is not
coming properly if only specified value is mentioned in publish.

Testcase to reproduce -

\\create a publication

postgres=# CREATE PUBLICATION abc for all tables with (publish='insert');
CREATE PUBLICATION

\\take the plain dump

[centos@centos-cpula bin]$ ./pg_dump -FP -p 5000 postgres > /tmp/a.a

\\check the syntax

[centos@centos-cpula bin]$ cat /tmp/a.a |grep 'create publication abc' -i
CREATE PUBLICATION abc FOR ALL TABLES WITH (publish = 'insert, , ');

\\try to execute the same syntax against psql terminal

postgres=# CREATE PUBLICATION abc FOR ALL TABLES WITH (publish = 'insert, ,
');
ERROR: invalid publish list

Same is valid for pg_dumpall as well..

Thank you for reporting.

Hm, It's a bug of pg_dump. Attached patch should fix both pg_dump and
pg_dumpall.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

fix_pg_dump.patchapplication/octet-stream; name=fix_pg_dump.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index b6794d0..f7b2840 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3491,20 +3491,20 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
 		first = false;
 	}
 
-	if (!first)
-		appendPQExpBufferStr(query, ", ");
-
 	if (pubinfo->pubupdate)
 	{
+		if (!first)
+			appendPQExpBufferStr(query, ", ");
+
 		appendPQExpBufferStr(query, "update");
 		first = false;
 	}
 
-	if (!first)
-		appendPQExpBufferStr(query, ", ");
-
 	if (pubinfo->pubdelete)
 	{
+		if (!first)
+			appendPQExpBufferStr(query, ", ");
+
 		appendPQExpBufferStr(query, "delete");
 		first = false;
 	}
#3Kuntal Ghosh
kuntalghosh.2007@gmail.com
In reply to: Masahiko Sawada (#2)
Re: Create publication syntax is not coming properly in pg_dump / pg_dumpall

On Mon, May 15, 2017 at 3:06 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Mon, May 15, 2017 at 5:04 PM, tushar <tushar.ahuja@enterprisedb.com> wrote:

Hi,

I observed that in pg_dump/pg_dumpall - 'create publication' syntax is not
coming properly if only specified value is mentioned in publish.

Testcase to reproduce -

\\create a publication

postgres=# CREATE PUBLICATION abc for all tables with (publish='insert');
CREATE PUBLICATION

\\take the plain dump

[centos@centos-cpula bin]$ ./pg_dump -FP -p 5000 postgres > /tmp/a.a

\\check the syntax

[centos@centos-cpula bin]$ cat /tmp/a.a |grep 'create publication abc' -i
CREATE PUBLICATION abc FOR ALL TABLES WITH (publish = 'insert, , ');

\\try to execute the same syntax against psql terminal

postgres=# CREATE PUBLICATION abc FOR ALL TABLES WITH (publish = 'insert, ,
');
ERROR: invalid publish list

Same is valid for pg_dumpall as well..

Thank you for reporting.

Hm, It's a bug of pg_dump. Attached patch should fix both pg_dump and
pg_dumpall.

I've reproduced the bug and the patch fixes the issue for me. Also,
tested with different combinations of insert, delete and update.

--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Masahiko Sawada (#2)
Re: Create publication syntax is not coming properly in pg_dump / pg_dumpall

Masahiko Sawada <sawada.mshk@gmail.com> writes:

Hm, It's a bug of pg_dump. Attached patch should fix both pg_dump and
pg_dumpall.

Pushed, thanks.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers