On Tue, Feb 21, 2017 at 12:48 AM, Stephen Frost <sfr...@snowman.net> wrote:
> * Fujii Masao (masao.fu...@gmail.com) wrote:
>> On Fri, Feb 17, 2017 at 11:17 PM, Peter Eisentraut
>> 1) Expose all the columns except subconninfo in pg_subscription to
>>     non-superusers. In this idea, the tab-completion and psql meta-command
>>     for subscription still sees pg_subscription. One good point of
>>     idea is that even non-superusers can see all the useful information
>>     about subscriptions other than sensitive information like conninfo.
>>
>> 2) Change pg_stat_subscription so that it also shows all the columns except
>>     subconninfo in pg_subscription. Also change the tab-completion and
>>     psql meta-command for subscription so that they see pg_stat_subscription
>>     instead of pg_subscription. One good point is that pg_stat_subscription
>>     exposes all the useful information about subscription to even
>>     non-superusers. OTOH, pg_subscription and pg_stat_subscription have
>>     to have several same columns. This would be redundant and a bit 
>> confusing.
>>
>> 3) Expose subdbid in pg_stat_subscription. Also change the tab-completion
>>     and psql meta-command for subscription so that they see
>>     pg_stat_subscription. This change is very simple. But non-superusers 
>> cannot
>>     see useful information like subslotname because of privilege problem.
>>
>> I like #1, but any better approach?
>
> #1 seems alright to me, at least.  We could start using column-level
> privs in other places also, as I mentioned up-thread.

Among the three, this looks like a good one.

> I don't particularly like the suggestions to have psql use pg_stat_X
> views or to put things into pg_stat_X views which are object definitions
> for non-superusers.  If we really don't want to use column-level
> privileges then we should have an appropriate view create instead which
> provides non-superusers with the non-sensitive object information.

Neither do I, those are by definition tables for statistics. Having
tab completion depend on them is not right.

So what do you think about the patch attached? This does the following:
- complete subscription list for CREATE/ALTER SUBSCRIPTION
- complete publication list for CREATE/ALTER PUBLICATION
- complete to nothing for PUBLICATION in CREATE/ALTER SUBSCRIPTION as
this refers to remote objects defined in subconninfo.
- authorize read access to public for all columns of pg_subscription
except subconninfo
Thoughts?
-- 
Michael
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 38be9cf1a0..3e88a2a595 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -900,7 +900,10 @@ CREATE VIEW pg_replication_origin_status AS
 
 REVOKE ALL ON pg_replication_origin_status FROM public;
 
+-- All columns of pg_subscription, except subconninfo, are readable.
 REVOKE ALL ON pg_subscription FROM public;
+GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+    ON pg_subscription TO public;
 
 --
 -- We have a few function definitions in here, too.
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 94814c20d0..fa0ce5c184 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -830,6 +830,18 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   FROM pg_catalog.pg_am "\
 "  WHERE substring(pg_catalog.quote_ident(amname),1,%d)='%s'"
 
+#define Query_for_list_of_subscriptions \
+" SELECT pg_catalog.quote_ident(c1.subname) "\
+"   FROM pg_catalog.pg_subscription c1, pg_catalog.pg_database c2"\
+"  WHERE substring(pg_catalog.quote_ident(c1.subname),1,%d)='%s'"\
+"    AND c2.datname = pg_catalog.current_database()"\
+"    AND c1.subdbid = c2.oid"
+
+#define Query_for_list_of_publications \
+" SELECT pg_catalog.quote_ident(pubname) "\
+"   FROM pg_catalog.pg_publication "\
+"  WHERE substring(pg_catalog.quote_ident(pubname),1,%d)='%s'"
+
 /* the silly-looking length condition is just to eat up the current word */
 #define Query_for_list_of_arguments \
 "SELECT pg_catalog.oidvectortypes(proargtypes)||')' "\
@@ -960,13 +972,13 @@ static const pgsql_thing_t words_after_create[] = {
 	{"OWNED", NULL, NULL, THING_NO_CREATE},		/* for DROP OWNED BY ... */
 	{"PARSER", Query_for_list_of_ts_parsers, NULL, THING_NO_SHOW},
 	{"POLICY", NULL, NULL},
-	{"PUBLICATION", NULL, NULL},
+	{"PUBLICATION", Query_for_list_of_publications},
 	{"ROLE", Query_for_list_of_roles},
 	{"RULE", "SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"},
 	{"SCHEMA", Query_for_list_of_schemas},
 	{"SEQUENCE", NULL, &Query_for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
-	{"SUBSCRIPTION", NULL, NULL},
+	{"SUBSCRIPTION", Query_for_list_of_subscriptions},
 	{"TABLE", NULL, &Query_for_list_of_tables},
 	{"TABLESPACE", Query_for_list_of_tablespaces},
 	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */
@@ -2317,8 +2329,13 @@ psql_completion(const char *text, int start, int end)
 /* CREATE SUBSCRIPTION */
 	else if (Matches3("CREATE", "SUBSCRIPTION", MatchAny))
 		COMPLETE_WITH_CONST("CONNECTION");
-	else if (Matches5("CREATE", "SUBSCRIPTION", MatchAny, "CONNECTION",MatchAny))
+	else if (Matches5("CREATE", "SUBSCRIPTION", MatchAny, "CONNECTION", MatchAny))
 		COMPLETE_WITH_CONST("PUBLICATION");
+	else if (Matches6("CREATE", "SUBSCRIPTION", MatchAny, "CONNECTION",
+					  MatchAny, "PUBLICATION"))
+	{
+		/* complete with nothing here as this refers to remote publications */
+	}
 	/* Complete "CREATE SUBSCRIPTION <name> ...  WITH ( <opt>" */
 	else if (HeadMatches2("CREATE", "SUBSCRIPTION") && TailMatches2("WITH", "("))
 		COMPLETE_WITH_LIST5("ENABLED", "DISABLED", "CREATE SLOT",
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to