Re: ExecutorCheckPerms() hook

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: ExecutorCheckPerms() hook
Date: 2010-05-26 00:52:13
Message-ID: 4BFC70BD.50305@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2010/05/25 22:59), Stephen Frost wrote:
> * KaiGai Kohei (kaigai(at)ak(dot)jp(dot)nec(dot)com) wrote:
>> * DoCopy() and RI_Initial_Check() were reworked to call ExecCheckRTEPerms()
>> with locally built RangeTblEntry.
>
> Maybe I missed it somewhere, but we still need to address the case where
> the user doesn't have those SELECT permissions that we're looking for in
> RI_Initial_Check(), right? KaiGai, your patch should be addressing that
> in a similar fashion..

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.

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

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?

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Florian Pflug 2010-05-26 00:58:01 Re: Synchronization levels in SR
Previous Message KaiGai Kohei 2010-05-26 00:34:38 Re: ExecutorCheckPerms() hook