On Mon, Feb 6, 2017 at 1:52 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Sun, Feb 5, 2017 at 5:04 AM, Fujii Masao <masao.fujii@gmail.com> 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