pgsql: Add COMMENT and SECURITY LABEL support for publications and subs

Started by Peter Eisentrautalmost 9 years ago5 messages
#1Peter Eisentraut
peter_e@gmx.net

Add COMMENT and SECURITY LABEL support for publications and subscriptions

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/87dee41f3ed6d6c2a93e7ff359776cfe24f145e0

Modified Files
--------------
doc/src/sgml/ref/comment.sgml | 2 ++
doc/src/sgml/ref/security_label.sgml | 2 ++
src/backend/catalog/system_views.sql | 22 +++++++++++++++
src/backend/parser/gram.y | 4 +++
.../dummy_seclabel/expected/dummy_seclabel.out | 32 ++++++++++++++--------
.../modules/dummy_seclabel/sql/dummy_seclabel.sql | 7 +++++
src/test/regress/expected/publication.out | 7 +++++
src/test/regress/expected/rules.out | 23 ++++++++++++++++
src/test/regress/expected/subscription.out | 7 +++++
src/test/regress/sql/publication.sql | 3 ++
src/test/regress/sql/subscription.sql | 3 ++
11 files changed, 100 insertions(+), 12 deletions(-)

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

#2Stephen Frost
sfrost@snowman.net
In reply to: Peter Eisentraut (#1)
Re: pgsql: Add COMMENT and SECURITY LABEL support for publications and subs

Peter,

* Peter Eisentraut (peter_e@gmx.net) wrote:

Add COMMENT and SECURITY LABEL support for publications and subscriptions

Isn't this missing psql tab completion, and pg_dump support? And
regression tests for the latter?

Thanks!

Stephen

#3Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Stephen Frost (#2)
1 attachment(s)
Re: [COMMITTERS] pgsql: Add COMMENT and SECURITY LABEL support for publications and subs

On 3/25/17 12:24, Stephen Frost wrote:

Add COMMENT and SECURITY LABEL support for publications and subscriptions

Isn't this missing psql tab completion, and pg_dump support? And
regression tests for the latter?

I have added the tab completion support.

Attached is a patch that adds the pg_dump support, but I'm struggling to
make the tests work. Could you take a look? Problem one I'm seeing is
that the tests assert that there are no comments in the post-data
section, which is no longer the case here. Problem two is that
subscriptions are not dumped by default, so those new tests fail, but I
don't understand why the existing tests about subscriptions work so far.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-pg_dump-Dump-comments-and-security-labels-for-public.patchinvalid/octet-stream; name=0001-pg_dump-Dump-comments-and-security-labels-for-public.patchDownload
From ef4d8c64a48253da5c0c227dcf3a9f659db7ee62 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 4 Apr 2017 09:25:02 -0400
Subject: [PATCH] pg_dump: Dump comments and security labels for publication
 and subscriptions

---
 src/bin/pg_dump/pg_dump.c        | 28 +++++++++++++++++
 src/bin/pg_dump/t/002_pg_dump.pl | 68 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 262f5539bc..18ffd4ca80 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3457,12 +3457,14 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
 {
 	PQExpBuffer delq;
 	PQExpBuffer query;
+	PQExpBuffer labelq;
 
 	if (!(pubinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
 		return;
 
 	delq = createPQExpBuffer();
 	query = createPQExpBuffer();
+	labelq = createPQExpBuffer();
 
 	appendPQExpBuffer(delq, "DROP PUBLICATION %s;\n",
 					  fmtId(pubinfo->dobj.name));
@@ -3470,6 +3472,8 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
 	appendPQExpBuffer(query, "CREATE PUBLICATION %s",
 					  fmtId(pubinfo->dobj.name));
 
+	appendPQExpBuffer(labelq, "PUBLICATION %s", fmtId(pubinfo->dobj.name));
+
 	if (pubinfo->puballtables)
 		appendPQExpBufferStr(query, " FOR ALL TABLES");
 
@@ -3501,6 +3505,16 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
 				 NULL, 0,
 				 NULL, NULL);
 
+	if (pubinfo->dobj.dump & DUMP_COMPONENT_COMMENT)
+		dumpComment(fout, labelq->data,
+					NULL, pubinfo->rolname,
+					pubinfo->dobj.catId, 0, pubinfo->dobj.dumpId);
+
+	if (pubinfo->dobj.dump & DUMP_COMPONENT_SECLABEL)
+		dumpSecLabel(fout, labelq->data,
+					 NULL, pubinfo->rolname,
+					 pubinfo->dobj.catId, 0, pubinfo->dobj.dumpId);
+
 	destroyPQExpBuffer(delq);
 	destroyPQExpBuffer(query);
 }
@@ -3726,6 +3740,7 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
 	DumpOptions *dopt = fout->dopt;
 	PQExpBuffer delq;
 	PQExpBuffer query;
+	PQExpBuffer labelq;
 	PQExpBuffer publications;
 	char	  **pubnames = NULL;
 	int			npubnames = 0;
@@ -3736,6 +3751,7 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
 
 	delq = createPQExpBuffer();
 	query = createPQExpBuffer();
+	labelq = createPQExpBuffer();
 
 	appendPQExpBuffer(delq, "DROP SUBSCRIPTION %s;\n",
 					  fmtId(subinfo->dobj.name));
@@ -3779,6 +3795,8 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
 
 	appendPQExpBufferStr(query, ");\n");
 
+	appendPQExpBuffer(labelq, "SUBSCRIPTION %s", fmtId(subinfo->dobj.name));
+
 	ArchiveEntry(fout, subinfo->dobj.catId, subinfo->dobj.dumpId,
 				 subinfo->dobj.name,
 				 NULL,
@@ -3789,6 +3807,16 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
 				 NULL, 0,
 				 NULL, NULL);
 
+	if (subinfo->dobj.dump & DUMP_COMPONENT_COMMENT)
+		dumpComment(fout, labelq->data,
+					NULL, subinfo->rolname,
+					subinfo->dobj.catId, 0, subinfo->dobj.dumpId);
+
+	if (subinfo->dobj.dump & DUMP_COMPONENT_SECLABEL)
+		dumpSecLabel(fout, labelq->data,
+					 NULL, subinfo->rolname,
+					 subinfo->dobj.catId, 0, subinfo->dobj.dumpId);
+
 	destroyPQExpBuffer(publications);
 	if (pubnames)
 		free(pubnames);
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 366737440c..b5a2da130a 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -1774,6 +1774,74 @@
 			section_data             => 1,
 			section_post_data        => 1, }, },
 
+	'COMMENT ON PUBLICATION pub1' => {
+		all_runs     => 1,
+		create_order => 84,
+		create_sql   => 'COMMENT ON PUBLICATION pub1
+					   IS \'comment on publication\';',
+		regexp => qr/^COMMENT ON PUBLICATION pub1 IS 'comment on publication';/m,
+		like   => {
+			binary_upgrade           => 1,
+			clean                    => 1,
+			clean_if_exists          => 1,
+			createdb                 => 1,
+			defaults                 => 1,
+			exclude_dump_test_schema => 1,
+			exclude_test_table       => 1,
+			exclude_test_table_data  => 1,
+			no_blobs                 => 1,
+			no_privs                 => 1,
+			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
+			schema_only              => 1,
+			section_post_data        => 1,
+			with_oids                => 1, },
+		unlike => {
+			column_inserts           => 1,
+			data_only                => 1,
+			only_dump_test_table     => 1,
+			only_dump_test_schema    => 1,
+			pg_dumpall_globals       => 1,
+			pg_dumpall_globals_clean => 1,
+			role                     => 1,
+			section_data             => 1,
+			section_pre_data         => 1,
+			test_schema_plus_blobs   => 1, }, },
+
+	'COMMENT ON SUBSCRIPTION sub1' => {
+		all_runs     => 1,
+		create_order => 84,
+		create_sql   => 'COMMENT ON SUBSCRIPTION sub1
+					   IS \'comment on subscription\';',
+		regexp => qr/^COMMENT ON SUBSCRIPTION sub1 IS 'comment on subscription';/m,
+		like   => {
+			binary_upgrade           => 1,
+			clean                    => 1,
+			clean_if_exists          => 1,
+			createdb                 => 1,
+			defaults                 => 1,
+			exclude_dump_test_schema => 1,
+			exclude_test_table       => 1,
+			exclude_test_table_data  => 1,
+			no_blobs                 => 1,
+			no_privs                 => 1,
+			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
+			schema_only              => 1,
+			section_post_data        => 1,
+			with_oids                => 1, },
+		unlike => {
+			column_inserts           => 1,
+			data_only                => 1,
+			only_dump_test_table     => 1,
+			only_dump_test_schema    => 1,
+			pg_dumpall_globals       => 1,
+			pg_dumpall_globals_clean => 1,
+			role                     => 1,
+			section_data             => 1,
+			section_pre_data         => 1,
+			test_schema_plus_blobs   => 1, }, },
+
 	'COMMENT ON TEXT SEARCH CONFIGURATION dump_test.alt_ts_conf1' => {
 		all_runs     => 1,
 		catch_all    => 'COMMENT commands',
-- 
2.12.2

#4Stephen Frost
sfrost@snowman.net
In reply to: Peter Eisentraut (#3)
Re: [COMMITTERS] pgsql: Add COMMENT and SECURITY LABEL support for publications and subs

Peter,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 3/25/17 12:24, Stephen Frost wrote:

Add COMMENT and SECURITY LABEL support for publications and subscriptions

Isn't this missing psql tab completion, and pg_dump support? And
regression tests for the latter?

I have added the tab completion support.

Thanks!

Attached is a patch that adds the pg_dump support, but I'm struggling to
make the tests work. Could you take a look? Problem one I'm seeing is
that the tests assert that there are no comments in the post-data
section, which is no longer the case here.

If that's the case (and is intended), then you'll need to remove the
'section_post_data' entry from the COMMENTS catch-all 'unlike' and move
that into the 'unlike' for each of the COMMENT tests which were
depending on the catch-all to handle that.

The other approach is to create a new catch-all which catches COMMENTs
that are not pub/sub and have the other COMMENT tests use that but have
a different catch-all for the pub/sub entries or just have all of the
tests covered by them. There are a few examples of this approach
already (off-hand, I think there's one related to blobs).

Problem two is that
subscriptions are not dumped by default, so those new tests fail, but I
don't understand why the existing tests about subscriptions work so far.

There are (or were?) some tests which explicitly use
'--include-subscription'. You can see how each test's pg_dump is run in
the hash at the top.

I thought we were changing that anyway though? To dump subscriptions by
default but to have them dumped in a 'not enabled' fashion? In fact, I
thought that had already happened, but I might be thinking of something
else.

Thanks!

Stephen

#5Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Stephen Frost (#4)
Re: Re: [COMMITTERS] pgsql: Add COMMENT and SECURITY LABEL support for publications and subs

On 4/4/17 09:59, Stephen Frost wrote:

Attached is a patch that adds the pg_dump support, but I'm struggling to
make the tests work. Could you take a look? Problem one I'm seeing is
that the tests assert that there are no comments in the post-data
section, which is no longer the case here.

If that's the case (and is intended), then you'll need to remove the
'section_post_data' entry from the COMMENTS catch-all 'unlike' and move
that into the 'unlike' for each of the COMMENT tests which were
depending on the catch-all to handle that.

done

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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