Re: ExecutorCheckPerms() hook

* KaiGai Kohei (kaigai(at)ak(dot)jp(dot)nec(dot)com) wrote:
> The reason why user must have SELECT privileges on the PK/FK tables is
> the validateForeignKeyConstraint() entirely calls SPI_execute() to verify
> FK constraints can be established between two tables (even if fallback path).
>
> And, the reason why RI_Initial_Check() now calls pg_class_aclcheck() is
> to try to avoid unexpected access violation error because of SPI_execute().
> However, the fallback path also calls SPI_execute() entirely, so I concluded
> the permission checks in RI_Initial_Check() is nonsense.

That may be the case. I'm certainly more concerned with a bug in the
existing code than any new code that we're working on. The question is-
is this actually a user-visible bug? Or do we require that a user
creating an FK needs SELECT rights on the primary table? If so, it's
still a bug, but at that point it's a bug in our documentation where we
don't mention that SELECT rights are also needed.

Anyone know what the SQL spec says about this (if anything...)?

> However, it is an independent issue right now, so I kept it as is.

Uh, I don't really see it as independent.. If we have a bug there that
we need to fix, and it's because we have two different bits of code
trying to do the same checking, we should fix it be eliminating the
duplicate checking, imv.

> The origin of the matter is that we applies unnecessary permission checks,
> although it is purely internal use and user was already checked to execute
> whole of ALTER TABLE statement. Right?

That's certainly a nice thought, but given the complexity in ALTER
TABLE, in particular with regard to permissions checking, I have no idea
if what it's doing is intentional or wrong.

Thanks,

Stephen