Re: ExecutorCheckPerms() hook

KaiGai,

* KaiGai Kohei (kaigai(at)ak(dot)jp(dot)nec(dot)com) wrote:
> Yes, it is entirely separate issue. I don't intend to argue whether
> we can assume the default PG permission allows owner to SELECT on
> the table, or not.

This actually isn't a separate issue. It's the whole crux of it, as a
matter of fact. So, wrt the standard, you do NOT need SELECT rights on
a table to create an FK against it. You only need references. PG
handles this correctly.

This error:

> >> postgres=> ALTER TABLE fk_tbl ADD FOREIGN KEY (x) REFERENCES pk_tbl (a);
> >> ERROR: permission denied for relation pk_tbl
> >> CONTEXT: SQL statement "SELECT 1 FROM ONLY "public"."pk_tbl" x WHERE "a" OPERATOR(pg_catalog.=) $1 FOR SHARE OF x"

Is due to the *owner* of pk_tbl not having SELECT rights- a situation
we're not generally expecting to see. What's happening is that prior to
the above being run, we've switched user over to the owner of the table
to perform this check.

This script illustrates what I'm talking about:

CREATE USER pk_owner;
CREATE USER fk_owner;
GRANT pk_owner TO sfrost;
GRANT fk_owner TO sfrost;

SET ROLE pk_owner;
CREATE TABLE pk_tbl (a int primary key, b text);
INSERT INTO pk_tbl VALUES (1,'aaa'), (2,'bbb'), (3,'ccc');
GRANT REFERENCES ON pk_tbl TO fk_owner;

SET ROLE fk_owner;
CREATE TABLE fk_tbl (x int, y text);
INSERT INTO fk_tbl VALUES (1,'xxx'), (2,'yyy'), (3,'zzz');

ALTER TABLE fk_tbl ADD FOREIGN KEY (x) REFERENCES pk_tbl (a);

ALTER TABLE fk_tbl DROP CONSTRAINT fk_tbl_x_fkey;

SET ROLE pk_owner;
REVOKE ALL ON pk_tbl FROM pk_owner;

SET ROLE fk_owner;
ALTER TABLE fk_tbl ADD FOREIGN KEY (x) REFERENCES pk_tbl (a);

ERROR: permission denied for relation pk_tbl
CONTEXT: SQL statement "SELECT 1 FROM ONLY "public"."pk_tbl" x WHERE
"a" OPERATOR(pg_catalog.=) $1 FOR SHARE OF x"

This does make me think (as I've thought in the past..) that we really
should say *who* doesn't have that permission. We run into the same
problem with views- they're run as the owner of the view, so you can get
a permission denied error trying to select from the view when you
clearly have select rights on the view itself.

As it turns out, the check in RI_Initial_Check() to provide the speed-up
is if the current role can just SELECT against the PK table- in which
case, you can run the check as the FK user and not have to change user.
We can't just switch to the PK user and run the same query though,
because that user might not have any rights on the FK table. So, we end
up taking the slow path, which fires off the FK trigger that's been set
up on the fk_tbl but which runs as the owner of the pk_tbl.

So, long-and-short, I don't see that we have a bug in any of this. I do
think we should allow RI_Initial_Check() to run if it has the necessary
column-level permissions, and we should remove the duplicate
permissions-checking code in favor of using the same code the executor
will, which happens to also be where the new hook we're talking about
is.

Thanks,

Stephen