On Sun, Sep 11, 2016 at 8:07 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:

> I wasn't aware that this patch was doing anything nontrivial ...

Well, it is not doing anything other than showing candidate
templates for tab completion beyond those flagged as template

> After looking at it I think it's basically uninformed about how to test
> for ownership.  An explicit join against pg_roles is almost never the
> right way for SQL queries to do that.  Lose the join and write it more
> like this:
> +"SELECT pg_catalog.quote_ident(d.datname) "\
> +"  FROM pg_catalog.pg_database d "\
> +" WHERE substring(pg_catalog.quote_ident(d.datname),1,%d)='%s' "\
> +"   AND (d.datistemplate OR pg_catalog.pg_has_role(d.datdba, 'USAGE'))"

But that gives incorrect results for the case I asked about earlier
on the thread, while the query I pushed gives correct results:

test=# \du fred
                  List of roles
 Role name |       Attributes        | Member of
 fred      | Create DB, Cannot login | {}

test=# \du bob
           List of roles
 Role name | Attributes | Member of
 bob       |            | {fred}

test=# \l
                                 List of databases
    Name    |  Owner  | Encoding |   Collate   |    Ctype    |  Access
 db1        | fred    | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
 db2        | bob     | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
 postgres   | fred    | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
 regression | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
=Tc/kgrittn        +
            |         |          |             |             |
 template0  | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
=c/kgrittn         +
            |         |          |             |             |
 template1  | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
=c/kgrittn         +
            |         |          |             |             |
 test       | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
(7 rows)

test=# set role bob;
test=> SELECT pg_catalog.quote_ident(d.datname)
  FROM pg_catalog.pg_database d
 WHERE (d.datistemplate OR pg_catalog.pg_has_role(d.datdba, 'USAGE'));
(5 rows)

test=> SELECT pg_catalog.quote_ident(datname)
  FROM pg_catalog.pg_database d
  JOIN pg_catalog.pg_roles r ON rolname = CURRENT_USER
 WHERE (datistemplate OR rolsuper OR datdba = r.oid);
(3 rows)

There is absolutely no question that the query you suggest is
wrong; the only case I had to think about very hard after
establishing that CREATEDB was not inherited was when bob owned a
database but did not have CREATEDB permission.  It seemed to me
least astonishing to show such a database, because that seemed
parallel to, for example, tab completion for table names on CREATE
TABLE AS.  We show a database object in the tab completion when it
would be available except for absence of a permission on the user.
That seems sane to me, because the attempt to actually execute with
that object gives a potentially useful error message.  Anyway, I
tend to like symmetry in these things -- it could also be
considered sane not to show t2 on tab completion fro bob above, but
then we should probably add more security tests to other tab
completion queries, like CREATE TABLE AS ... SELECT ... FROM.

Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

Reply via email to