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

> 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
privileges
------------+---------+----------+-------------+-------------+---------------------
 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        +
            |         |          |             |             |
kgrittn=CTc/kgrittn
 template0  | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
=c/kgrittn         +
            |         |          |             |             |
kgrittn=CTc/kgrittn
 template1  | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
=c/kgrittn         +
            |         |          |             |             |
kgrittn=CTc/kgrittn
 test       | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
(7 rows)

test=# set role bob;
SET
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'));
 quote_ident
-------------
 template1
 template0
 postgres
 db1
 db2
(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);
 quote_ident
-------------
 template1
 template0
 db2
(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:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to