Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-03-03 Thread Peter Eisentraut
On 2/21/17 01:50, Michael Paquier wrote:
> 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

committed

-- 
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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-21 Thread Petr Jelinek
On 21/02/17 07:50, Michael Paquier wrote:
> On Tue, Feb 21, 2017 at 12:48 AM, Stephen Frost  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?
> 

Maybe it would make sense to also have pg_subscriptions view which
selects everything but subconninfo (or does some removal of sensitive
info there, if we have function for that)? The reason why I am thinking
about this is that, it's much more convenient to do SELECT * than
specifying all columns you have access to.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-20 Thread Michael Paquier
On Tue, Feb 21, 2017 at 12:48 AM, Stephen Frost  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, _for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
-	{"SUBSCRIPTION", NULL, NULL},
+	{"SUBSCRIPTION", Query_for_list_of_subscriptions},
 	{"TABLE", NULL, _for_list_of_tables},
 	{"TABLESPACE", Query_for_list_of_tablespaces},
 	{"TEMP", NULL, NULL, 

Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-20 Thread Stephen Frost
Greetings,

* Fujii Masao (masao.fu...@gmail.com) wrote:
> On Fri, Feb 17, 2017 at 11:17 PM, Peter Eisentraut
>  wrote:
> > On 2/13/17 12:07, Fujii Masao wrote:
> >> Anyway IMO that we can expose all the
> >> columns except the sensitive information (i.e., subconninfo field)
> >> in pg_subscription to even non-superusers.
> >
> > You mean with column privileges?
> 
> Yes.
> 
> So there are several approaches...
> 
> 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.

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.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-20 Thread Fujii Masao
On Fri, Feb 17, 2017 at 11:17 PM, Peter Eisentraut
 wrote:
> On 2/13/17 12:07, Fujii Masao wrote:
>> Anyway IMO that we can expose all the
>> columns except the sensitive information (i.e., subconninfo field)
>> in pg_subscription to even non-superusers.
>
> You mean with column privileges?

Yes.

So there are several approaches...

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?

Regards,

-- 
Fujii Masao


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-19 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
> On Sun, Feb 19, 2017 at 6:13 PM, Michael Paquier
>  wrote:
> > On Sun, Feb 19, 2017 at 8:03 PM, Magnus Hagander  
> > wrote:
> >> If password auth is used, we have to store the password in plaintext
> >> equivalent somewhere. Meaning it's by definition going to be exposed to
> >> superusers and replication downstreams.
> >
> > Another possibility is to mention the use of the new passfile
> > parameter for connection strings in the docs... This removes the need
> > to have plain passwords directly stored in the database. Not sure if
> > that's better though because that still mean that the password is
> > present in plain format somewhere.
> 
> The real solution to "the password is present in plain form somewhere"
> is probably "don't use passwords for authentication".  Because,
> ultimately, a password by its nature has to exist in plain form
> somewhere, at least in someone's brain, and very likely in their
> password manager or the post-it stuck to their desk or the Notes app
> on their iPhone or similar.  If the password is simple enough that the
> DBA can be certain of remembering it without any sort of memory aid,
> it's probably dumb simple.  If the DBA has few enough distinct
> passwords that he doesn't need a memory aid just on the basis of sheer
> volume of passwords needing to be remembered, that's probably not good
> either.

I agree that the use of plaintext passwords should be discouraged, but
we don't actually provide any way to address these kinds of proxy
authentication issues today except to store *some* secret somewhere on
the system which, if compromised, would allow the attacker to gain
access to the downstream system.

There are solutions to authentication proxying and it would be great if
we would implement them (Kerberos Delegation, for example, and there's a
mechanism to do something similar in TLS also, iirc, and we should be
able to come up with something for same-system cross-database or
cross-cluster unix-socket based auth, really...), but I don't think
anyone is currently working on implementing them, which is unfortunate.

I tend to agree with Michael that we should make it very clear in
our documentation that the password/private key/whatever has to be
stored unencrypted on the filesystem somewhere today.  Perhaps it
doesn't need to be a big "WARNING WARNING" type of message, but it's
important information which we should make sure the user is aware of.

If we do implement proper proxy authentication, we will want to include
documentation as to just what that means regarding attacks too (Kerberos
Delegation, for example, usually means that a ticket for the remote
service is stored somewhere and is valid for a period of time, meaning
that a privileged attacker who has access to the system at the same time
the user is using the system would be able to gain access to the remote
system by stealing the delegated ticket).

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-19 Thread Robert Haas
On Sun, Feb 19, 2017 at 6:13 PM, Michael Paquier
 wrote:
> On Sun, Feb 19, 2017 at 8:03 PM, Magnus Hagander  wrote:
>> If password auth is used, we have to store the password in plaintext
>> equivalent somewhere. Meaning it's by definition going to be exposed to
>> superusers and replication downstreams.
>
> Another possibility is to mention the use of the new passfile
> parameter for connection strings in the docs... This removes the need
> to have plain passwords directly stored in the database. Not sure if
> that's better though because that still mean that the password is
> present in plain format somewhere.

The real solution to "the password is present in plain form somewhere"
is probably "don't use passwords for authentication".  Because,
ultimately, a password by its nature has to exist in plain form
somewhere, at least in someone's brain, and very likely in their
password manager or the post-it stuck to their desk or the Notes app
on their iPhone or similar.  If the password is simple enough that the
DBA can be certain of remembering it without any sort of memory aid,
it's probably dumb simple.  If the DBA has few enough distinct
passwords that he doesn't need a memory aid just on the basis of sheer
volume of passwords needing to be remembered, that's probably not good
either.

-- 
Robert Haas
EnterpriseDB: http://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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-19 Thread Magnus Hagander
On Sun, Feb 19, 2017 at 1:43 PM, Michael Paquier 
wrote:

> On Sun, Feb 19, 2017 at 8:03 PM, Magnus Hagander 
> wrote:
> > If password auth is used, we have to store the password in plaintext
> > equivalent somewhere. Meaning it's by definition going to be exposed to
> > superusers and replication downstreams.
>
> Another possibility is to mention the use of the new passfile
> parameter for connection strings in the docs... This removes the need
> to have plain passwords directly stored in the database. Not sure if
> that's better though because that still mean that the password is
> present in plain format somewhere.
>

That might definitely be a help, because it would be stored out of band.
But it also comes with a caveat in that when it's stored outside, it's not
included in replication (physical HA replication) so you need to maintain
it across all nodes or Bad Things can happen.

And yes, whatever you do, if you want to the system to be able to
automatically start/restart, you have to keep the password in cleartext
equivalent *somewhere*. There's no way around that.



> > Or are you suggesting a scheme
> > whereby you have to enter all your subscription passwords in a prompt of
> > some kind when starting the postmaster, to avoid it?
>
> Thinking about that now we could have a connection string parameter
> called for example password_command, that could be used for example
> with gpg2 to get back a decrypted password value.
>
>
We could, but we don't really have a good way to interface with that. When
the server is started with says systemd, how and where are you going to
prompt the user for the gpg passphrase? It's not like there is a console
available to pop the question up on.

It's the same basic issue as with password protected SSL certificates -
which is why people end up deploying their servers with an unprotected key.
For all services.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-19 Thread Michael Paquier
On Sun, Feb 19, 2017 at 8:03 PM, Magnus Hagander  wrote:
> If password auth is used, we have to store the password in plaintext
> equivalent somewhere. Meaning it's by definition going to be exposed to
> superusers and replication downstreams.

Another possibility is to mention the use of the new passfile
parameter for connection strings in the docs... This removes the need
to have plain passwords directly stored in the database. Not sure if
that's better though because that still mean that the password is
present in plain format somewhere.

> Or are you suggesting a scheme
> whereby you have to enter all your subscription passwords in a prompt of
> some kind when starting the postmaster, to avoid it?

Thinking about that now we could have a connection string parameter
called for example password_command, that could be used for example
with gpg2 to get back a decrypted password value.
-- 
Michael


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-19 Thread Magnus Hagander
On Sun, Feb 19, 2017 at 1:03 PM, Petr Jelinek 
wrote:

> On 19/02/17 12:03, Magnus Hagander wrote:
> >
> >
> > On Sun, Feb 19, 2017 at 2:01 AM, Michael Paquier
> > > wrote:
> >
> > On Sun, Feb 19, 2017 at 9:50 AM, Michael Paquier
> > >
> wrote:
> > > I have been poking at it, and yeah... I missed the fact that
> > > pg_subcription is not a view. I thought that check_conninfo was
> being
> > > called in this context only..
> >
> > Still, storing plain passwords in system catalogs is a practice that
> > should be discouraged as base backup data can go over a network as
> > well... At least adding a note or a warning in the documentation
> would
> > be nice about the fact that any kind of security-sensitive data
> should
> > be avoided here.
> >
> >
> > Isn't that moving the goalposts quite a bit? We already allow passwords
> > in CREATE USER MAPPING without any warnings against it (in fact, we
> > suggest that's what you should do), which is a similar situation. Same
> > goes for dblink.
> >
> > If password auth is used, we have to store the password in plaintext
> > equivalent somewhere. Meaning it's by definition going to be exposed to
> > superusers and replication downstreams. Or are you suggesting a scheme
> > whereby you have to enter all your subscription passwords in a prompt of
> > some kind when starting the postmaster, to avoid it?
> >
>
> The subscriptions will happily use .pgpass for example so it's not like
> users are forced to put password to catalog (well barring some DBaaS
> solutions). But I guess it would not hurt to give extra notice in docs
> about dangers of the various catalogs storing passwords.
>
>
I certainly wouldn't object to doing that, but if we do we should
consistently do it in the other places that have work the same way (like
user mappings).

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-19 Thread Petr Jelinek
On 19/02/17 12:03, Magnus Hagander wrote:
> 
> 
> On Sun, Feb 19, 2017 at 2:01 AM, Michael Paquier
> > wrote:
> 
> On Sun, Feb 19, 2017 at 9:50 AM, Michael Paquier
> > wrote:
> > I have been poking at it, and yeah... I missed the fact that
> > pg_subcription is not a view. I thought that check_conninfo was being
> > called in this context only..
> 
> Still, storing plain passwords in system catalogs is a practice that
> should be discouraged as base backup data can go over a network as
> well... At least adding a note or a warning in the documentation would
> be nice about the fact that any kind of security-sensitive data should
> be avoided here.
> 
> 
> Isn't that moving the goalposts quite a bit? We already allow passwords
> in CREATE USER MAPPING without any warnings against it (in fact, we
> suggest that's what you should do), which is a similar situation. Same
> goes for dblink.
> 
> If password auth is used, we have to store the password in plaintext
> equivalent somewhere. Meaning it's by definition going to be exposed to
> superusers and replication downstreams. Or are you suggesting a scheme
> whereby you have to enter all your subscription passwords in a prompt of
> some kind when starting the postmaster, to avoid it?
> 

The subscriptions will happily use .pgpass for example so it's not like
users are forced to put password to catalog (well barring some DBaaS
solutions). But I guess it would not hurt to give extra notice in docs
about dangers of the various catalogs storing passwords.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-19 Thread Magnus Hagander
On Sun, Feb 19, 2017 at 2:01 AM, Michael Paquier 
wrote:

> On Sun, Feb 19, 2017 at 9:50 AM, Michael Paquier
>  wrote:
> > I have been poking at it, and yeah... I missed the fact that
> > pg_subcription is not a view. I thought that check_conninfo was being
> > called in this context only..
>
> Still, storing plain passwords in system catalogs is a practice that
> should be discouraged as base backup data can go over a network as
> well... At least adding a note or a warning in the documentation would
> be nice about the fact that any kind of security-sensitive data should
> be avoided here.
>
>
Isn't that moving the goalposts quite a bit? We already allow passwords in
CREATE USER MAPPING without any warnings against it (in fact, we suggest
that's what you should do), which is a similar situation. Same goes for
dblink.

If password auth is used, we have to store the password in plaintext
equivalent somewhere. Meaning it's by definition going to be exposed to
superusers and replication downstreams. Or are you suggesting a scheme
whereby you have to enter all your subscription passwords in a prompt of
some kind when starting the postmaster, to avoid it?


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-18 Thread Michael Paquier
On Sun, Feb 19, 2017 at 9:50 AM, Michael Paquier
 wrote:
> I have been poking at it, and yeah... I missed the fact that
> pg_subcription is not a view. I thought that check_conninfo was being
> called in this context only..

Still, storing plain passwords in system catalogs is a practice that
should be discouraged as base backup data can go over a network as
well... At least adding a note or a warning in the documentation would
be nice about the fact that any kind of security-sensitive data should
be avoided here.
-- 
Michael


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-18 Thread Michael Paquier
On Sun, Feb 19, 2017 at 8:05 AM, Petr Jelinek
 wrote:
> It's not a view it's system catalog which actually stores the data, how
> would it hide anything?

I have been poking at it, and yeah... I missed the fact that
pg_subcription is not a view. I thought that check_conninfo was being
called in this context only..
-- 
Michael


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-18 Thread Petr Jelinek
On 19/02/17 00:02, Michael Paquier wrote:
> On Sat, Feb 18, 2017 at 11:57 PM, Petr Jelinek
>  wrote:
>> On 15/02/17 05:56, Michael Paquier wrote:
>>> I thought that this was correctly clobbered... But... No that's not
>>> the case by looking at the code. And honestly I think that it is
>>> unacceptable to show potentially security-sensitive information in
>>> system catalogs via a connection string. We are really careful about
>>> not showing anything bad in pg_stat_wal_receiver, which also sets to
>>> NULL fields for non-superusers and even clobbered values in the
>>> printed connection string for superusers, but pg_subscription fails on
>>> those points.
>>>
>>
>> I am not following here, pg_subscription is currently superuser only
>> catalog, similarly to pg_user_mapping, there is no leaking.
> 
> Even if it is a superuser-only view, pg_subscription does not hide
> sensitive values in connection strings while it should. See similar

It's not a view it's system catalog which actually stores the data, how
would it hide anything?

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-18 Thread Michael Paquier
On Sat, Feb 18, 2017 at 11:57 PM, Petr Jelinek
 wrote:
> On 15/02/17 05:56, Michael Paquier wrote:
>> I thought that this was correctly clobbered... But... No that's not
>> the case by looking at the code. And honestly I think that it is
>> unacceptable to show potentially security-sensitive information in
>> system catalogs via a connection string. We are really careful about
>> not showing anything bad in pg_stat_wal_receiver, which also sets to
>> NULL fields for non-superusers and even clobbered values in the
>> printed connection string for superusers, but pg_subscription fails on
>> those points.
>>
>
> I am not following here, pg_subscription is currently superuser only
> catalog, similarly to pg_user_mapping, there is no leaking.

Even if it is a superuser-only view, pg_subscription does not hide
sensitive values in connection strings while it should. See similar
discussion for pg_stat_wal_receiver here which is also superuser-only
(it does display null values for non-superusers):
https://www.postgresql.org/message-id/562f6c7f-6a47-0a8a-e189-2de9ea896...@2ndquadrant.com
Something needs to be done at least for that, independently on the
psql completion.
-- 
Michael


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-18 Thread Petr Jelinek
On 15/02/17 05:56, Michael Paquier wrote:
> On Wed, Feb 15, 2017 at 3:18 AM, Jim Nasby  wrote:
>> Why not do what we do for pg_stat_activity.current_query and leave it NULL 
>> for non-SU?
> 
> If subcriptions are designed to be superuser-only, it seems fair to me
> to do so. Some other system SRFs do that already.
> 
>> Even better would be if we could simply strip out password info. Presumably
>> we already know how to parse the contents, so I'd think that shouldn't be
>> that difficult.
> 
> I thought that this was correctly clobbered... But... No that's not
> the case by looking at the code. And honestly I think that it is
> unacceptable to show potentially security-sensitive information in
> system catalogs via a connection string. We are really careful about
> not showing anything bad in pg_stat_wal_receiver, which also sets to
> NULL fields for non-superusers and even clobbered values in the
> printed connection string for superusers, but pg_subscription fails on
> those points.
> 

I am not following here, pg_subscription is currently superuser only
catalog, similarly to pg_user_mapping, there is no leaking.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-17 Thread Stephen Frost
* Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> On 2/13/17 12:07, Fujii Masao wrote:
> > Anyway IMO that we can expose all the
> > columns except the sensitive information (i.e., subconninfo field)
> > in pg_subscription to even non-superusers.
> 
> You mean with column privileges?
> 
> We could probably do that.  I don't know if we have done that before on
> system catalogs.

I don't believe we have, though I'm not against doing so.  I can't think
of any reason off-hand why it wouldn't work.

If we did that then perhaps we could remove some of the other views that
we have which are just to provide a subset of the columns of some other
table (eg: pg_roles)...

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-17 Thread Peter Eisentraut
On 2/13/17 12:07, Fujii Masao wrote:
> Anyway IMO that we can expose all the
> columns except the sensitive information (i.e., subconninfo field)
> in pg_subscription to even non-superusers.

You mean with column privileges?

We could probably do that.  I don't know if we have done that before on
system catalogs.

-- 
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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-14 Thread Michael Paquier
On Wed, Feb 15, 2017 at 3:18 AM, Jim Nasby  wrote:
> Why not do what we do for pg_stat_activity.current_query and leave it NULL 
> for non-SU?

If subcriptions are designed to be superuser-only, it seems fair to me
to do so. Some other system SRFs do that already.

> Even better would be if we could simply strip out password info. Presumably
> we already know how to parse the contents, so I'd think that shouldn't be
> that difficult.

I thought that this was correctly clobbered... But... No that's not
the case by looking at the code. And honestly I think that it is
unacceptable to show potentially security-sensitive information in
system catalogs via a connection string. We are really careful about
not showing anything bad in pg_stat_wal_receiver, which also sets to
NULL fields for non-superusers and even clobbered values in the
printed connection string for superusers, but pg_subscription fails on
those points.

I am adding an open item on the wiki regarding that. FWIW, a patch
needs to refactor libpqrcv_check_conninfo and libpqrcv_get_conninfo so
as the connection string build of PQconninfoOption data goes through
the same process. If everybody agrees on those lines, I have no
problems in producing a patch.
-- 
Michael


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-14 Thread Jim Nasby

On 2/13/17 9:36 PM, Michael Paquier wrote:

Probably I failed to get Peter's point... Anyway IMO that we can expose all the
columns except the sensitive information (i.e., subconninfo field)
in pg_subscription to even non-superusers. Then we can use pg_subscription
for the tab-completion for ALTER/DROP SUBSCRIPTION.

To be honest, I find subconninfo quite useful to check where a
subscription is getting its changes from, so I'd rather not touch it.
It looks as well a bit overkill to just create a new view on an object
type non-superusers cannot even use... There are already 1 view and 1
system catalog related to it, so I'd be of the opinion to let it fail
silently with a permission error and keep it as an empty list for
them.


Why not do what we do for pg_stat_activity.current_query and leave it 
NULL for non-SU?


Even better would be if we could simply strip out password info. 
Presumably we already know how to parse the contents, so I'd think that 
shouldn't be that difficult.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-13 Thread Michael Paquier
On Tue, Feb 14, 2017 at 2:07 AM, Fujii Masao  wrote:
> No, the tab-completions for ALTER/DROP PUBLICATION should show the local
> publications because those commands drop and alter the local ones. OTOH,
> CREATE/ALTER SUBSCRIPTOIN ... PUBLICATION should show nothing because
> the remote publications in the publisher side should be specified there.

Doh, yes. You are right about that.

>> - Addition of a view pg_subscriptions with all the non-sensitive data.
>> (- Or really extend pg_stat_subscriptions with the database ID and use
>> it for tab completion?)
>
> Probably I failed to get Peter's point... Anyway IMO that we can expose all 
> the
> columns except the sensitive information (i.e., subconninfo field)
> in pg_subscription to even non-superusers. Then we can use pg_subscription
> for the tab-completion for ALTER/DROP SUBSCRIPTION.

To be honest, I find subconninfo quite useful to check where a
subscription is getting its changes from, so I'd rather not touch it.
It looks as well a bit overkill to just create a new view on an object
type non-superusers cannot even use... There are already 1 view and 1
system catalog related to it, so I'd be of the opinion to let it fail
silently with a permission error and keep it as an empty list for
them.
-- 
Michael


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-13 Thread Fujii Masao
On Fri, Feb 10, 2017 at 1:52 PM, Michael Paquier
 wrote:
> On Tue, Feb 7, 2017 at 6:35 AM, Peter Eisentraut
>  wrote:
>> On 2/6/17 10:54 AM, Fujii Masao wrote:
>>> Yes, that's an option. And, if we add dbid to pg_stat_subscription,
>>> I'm tempted to add all the pg_subscription's columns except subconninfo
>>> into pg_stat_subscription. Since subconninfo may contain security-sensitive
>>> information, it should be excluded. But it seems useful to expose other
>>> columns. Then, if we expose all the columns except subconninfo, maybe
>>> it's better to just revoke subconninfo column on pg_subscription instead of
>>> all columns. Thought?
>>
>> I think previous practice is to provide a view such as pg_subscriptions
>> that contains all the nonprivileged information.
>
> OK, I think that I see the point you are coming at:
> pg_stat_get_subscription (or stat tables) should not be used for
> psql's tab completion. So gathering all things discussed, we have:
> - No tab completion for publications.

No, the tab-completions for ALTER/DROP PUBLICATION should show the local
publications because those commands drop and alter the local ones. OTOH,
CREATE/ALTER SUBSCRIPTOIN ... PUBLICATION should show nothing because
the remote publications in the publisher side should be specified there.

> - Fix the meta-commands.

Yes.

> - Addition of a view pg_subscriptions with all the non-sensitive data.
> (- Or really extend pg_stat_subscriptions with the database ID and use
> it for tab completion?)

Probably I failed to get Peter's point... Anyway IMO that we can expose all the
columns except the sensitive information (i.e., subconninfo field)
in pg_subscription to even non-superusers. Then we can use pg_subscription
for the tab-completion for ALTER/DROP SUBSCRIPTION.

Regards,

-- 
Fujii Masao


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-09 Thread Michael Paquier
On Tue, Feb 7, 2017 at 6:35 AM, Peter Eisentraut
 wrote:
> On 2/6/17 10:54 AM, Fujii Masao wrote:
>> Yes, that's an option. And, if we add dbid to pg_stat_subscription,
>> I'm tempted to add all the pg_subscription's columns except subconninfo
>> into pg_stat_subscription. Since subconninfo may contain security-sensitive
>> information, it should be excluded. But it seems useful to expose other
>> columns. Then, if we expose all the columns except subconninfo, maybe
>> it's better to just revoke subconninfo column on pg_subscription instead of
>> all columns. Thought?
>
> I think previous practice is to provide a view such as pg_subscriptions
> that contains all the nonprivileged information.

OK, I think that I see the point you are coming at:
pg_stat_get_subscription (or stat tables) should not be used for
psql's tab completion. So gathering all things discussed, we have:
- No tab completion for publications.
- Fix the meta-commands.
- Addition of a view pg_subscriptions with all the non-sensitive data.
(- Or really extend pg_stat_subscriptions with the database ID and use
it for tab completion?)
Am I missing something?
-- 
Michael


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-06 Thread Peter Eisentraut
On 2/6/17 10:54 AM, Fujii Masao wrote:
> Yes, that's an option. And, if we add dbid to pg_stat_subscription,
> I'm tempted to add all the pg_subscription's columns except subconninfo
> into pg_stat_subscription. Since subconninfo may contain security-sensitive
> information, it should be excluded. But it seems useful to expose other
> columns. Then, if we expose all the columns except subconninfo, maybe
> it's better to just revoke subconninfo column on pg_subscription instead of
> all columns. Thought?

I think previous practice is to provide a view such as pg_subscriptions
that contains all the nonprivileged information.

-- 
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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-06 Thread Peter Eisentraut
On 2/5/17 11:52 PM, Michael Paquier wrote:
>> In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
>> PUBLICATION" cases, the publication defined in the publisher side should be
>> specified. But, with this patch, the tab-completion for PUBLICATION shows
>> the publications defined in local server (ie, subscriber side in those 
>> cases).
>> This might be confusing.
> Which would mean that psql tab completion should try to connect the
> remote server using the connection string defined with the
> subscription to get this information, which looks unsafe to me. To be
> honest, I'd rather see a list of local objects defined than nothing..

But it's the wrong list.

-- 
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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-06 Thread Fujii Masao
On Mon, Feb 6, 2017 at 1:52 PM, Michael Paquier
 wrote:
> On Sun, Feb 5, 2017 at 5:04 AM, Fujii Masao  wrote:
>> With this patch, when normal users type TAB after SUBSCRIPTION,
>> they got "permission denied" error. I don't think that's acceptable.
>
> Right, I can see that now. pg_stat_get_subscription() does not offer
> as well a way to filter by databases... Perhaps we could add it? it is
> stored as LogicalRepWorker->dbid so making it visible is sort of easy.

Yes, that's an option. And, if we add dbid to pg_stat_subscription,
I'm tempted to add all the pg_subscription's columns except subconninfo
into pg_stat_subscription. Since subconninfo may contain security-sensitive
information, it should be excluded. But it seems useful to expose other
columns. Then, if we expose all the columns except subconninfo, maybe
it's better to just revoke subconninfo column on pg_subscription instead of
all columns. Thought?

BTW, I found that psql's \dRs command has the same problem;
the permission denied error occurs when normal user runs \dRs.
This issue should be fixed in the same way as tab-completion one.

Also I found that tab-completion for psql's meta-commands doesn't
show \dRp and \dRs.

>> In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
>> PUBLICATION" cases, the publication defined in the publisher side should be
>> specified. But, with this patch, the tab-completion for PUBLICATION shows
>> the publications defined in local server (ie, subscriber side in those 
>> cases).
>> This might be confusing.
>
> Which would mean that psql tab completion should try to connect the
> remote server using the connection string defined with the
> subscription to get this information, which looks unsafe to me. To be
> honest, I'd rather see a list of local objects defined than nothing..

IMO showing nothing is better. But many people think that even local
objects should be showed in that case, I can live with that.

Regards,

-- 
Fujii Masao


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-05 Thread Michael Paquier
On Sun, Feb 5, 2017 at 5:04 AM, Fujii Masao  wrote:
> With this patch, when normal users type TAB after SUBSCRIPTION,
> they got "permission denied" error. I don't think that's acceptable.

Right, I can see that now. pg_stat_get_subscription() does not offer
as well a way to filter by databases... Perhaps we could add it? it is
stored as LogicalRepWorker->dbid so making it visible is sort of easy.

> In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
> PUBLICATION" cases, the publication defined in the publisher side should be
> specified. But, with this patch, the tab-completion for PUBLICATION shows
> the publications defined in local server (ie, subscriber side in those cases).
> This might be confusing.

Which would mean that psql tab completion should try to connect the
remote server using the connection string defined with the
subscription to get this information, which looks unsafe to me. To be
honest, I'd rather see a list of local objects defined than nothing..
-- 
Michael


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-04 Thread Fujii Masao
On Sat, Feb 4, 2017 at 9:01 PM, Michael Paquier
 wrote:
> On Fri, Feb 3, 2017 at 3:56 AM, Peter Eisentraut
>  wrote:
>> On 2/2/17 12:48 AM, Michael Paquier wrote:
>>> +#define Query_for_list_of_subscriptions \
>>> +" SELECT pg_catalog.quote_ident(subname) "\
>>> +"   FROM pg_catalog.pg_subscription "\
>>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>>
>> This query should also be qualified by current database.
>
> Indeed. Here is an updated patch.

With this patch, when normal users type TAB after SUBSCRIPTION,
they got "permission denied" error. I don't think that's acceptable.

In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
PUBLICATION" cases, the publication defined in the publisher side should be
specified. But, with this patch, the tab-completion for PUBLICATION shows
the publications defined in local server (ie, subscriber side in those cases).
This might be confusing.

Regards,

-- 
Fujii Masao


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-04 Thread Fujii Masao
On Fri, Feb 3, 2017 at 3:55 AM, Peter Eisentraut
 wrote:
> On 2/2/17 12:48 PM, Fujii Masao wrote:
>> +#define Query_for_list_of_subscriptions \
>> +" SELECT pg_catalog.quote_ident(subname) "\
>> +"   FROM pg_catalog.pg_subscription "\
>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>>
>> Since non-superuser is not allowed to access to pg_subscription,
>> pg_stat_subscription should be accessed here, instead. Thought?
>
> Arguably, you could leave it like that, assuming it fails cleanly for
> nonsuperusers.  Nonsuperusers are not going to be able to run any
> commands on subscriptions anyway, so there is little use for it.

No. You can get rid of superuser privilege from the owner of the subscription
and that nonsuperuser owner can run some commands on the subscriptin.
It's a bit artificial, but you can. I'm not sure if we should add the check to
prevent the owner from becoming nonsuperuser. But if the owner always must
have a superuser privilege per the specification of logical replication,
I think that such check would be necessary.

Also I prefer to tab-complete the subscriptions even for nonsuperusers.
There are some objects that only superuser or owner can manage, but their
names are currently tab-completed even when current user is "normal" one.
So I'm afraid that handling only subscriptions differently might be more
confusing.

Regards,

-- 
Fujii Masao


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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-04 Thread Michael Paquier
On Fri, Feb 3, 2017 at 3:56 AM, Peter Eisentraut
 wrote:
> On 2/2/17 12:48 AM, Michael Paquier wrote:
>> +#define Query_for_list_of_subscriptions \
>> +" SELECT pg_catalog.quote_ident(subname) "\
>> +"   FROM pg_catalog.pg_subscription "\
>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>
> This query should also be qualified by current database.

Indeed. Here is an updated patch.
-- 
Michael
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index d6fffcf42f..0835a71bab 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, _for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
-	{"SUBSCRIPTION", NULL, NULL},
+	{"SUBSCRIPTION", Query_for_list_of_subscriptions},
 	{"TABLE", NULL, _for_list_of_tables},
 	{"TABLESPACE", Query_for_list_of_tablespaces},
 	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */

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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-02 Thread Peter Eisentraut
On 2/2/17 12:48 AM, Michael Paquier wrote:
> +#define Query_for_list_of_subscriptions \
> +" SELECT pg_catalog.quote_ident(subname) "\
> +"   FROM pg_catalog.pg_subscription "\
> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"

This query should also be qualified by current database.

-- 
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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-02 Thread Peter Eisentraut
On 2/2/17 12:48 PM, Fujii Masao wrote:
> +#define Query_for_list_of_subscriptions \
> +" SELECT pg_catalog.quote_ident(subname) "\
> +"   FROM pg_catalog.pg_subscription "\
> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
> 
> Since non-superuser is not allowed to access to pg_subscription,
> pg_stat_subscription should be accessed here, instead. Thought?

Arguably, you could leave it like that, assuming it fails cleanly for
nonsuperusers.  Nonsuperusers are not going to be able to run any
commands on subscriptions anyway, so there is little use for it.

-- 
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


Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

2017-02-02 Thread Fujii Masao
On Thu, Feb 2, 2017 at 2:48 PM, Michael Paquier
 wrote:
> Hi all,
>
> While testing a bit the logical replication facility, I have bumped on
> the fast that psql's completion does not show the list of things
> already created. Attached is a patch.

+#define Query_for_list_of_subscriptions \
+" SELECT pg_catalog.quote_ident(subname) "\
+"   FROM pg_catalog.pg_subscription "\
+"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"

Since non-superuser is not allowed to access to pg_subscription,
pg_stat_subscription should be accessed here, instead. Thought?

Regards,

-- 
Fujii Masao


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