Re: [BUG] Column-level privileges on inherited tables

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [BUG] Column-level privileges on inherited tables
Date: 2009-03-05 07:32:28
Message-ID: 49AF800C.4040408@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Stephen,

The attached patch fixes the matter.
It fixes up attribute number of child relation when it is extracted.

(*) Injected elog()s are removed.

postgres=# select * from t1;
NOTICE: markRTEForSelectPriv: ACL_SELECT on t1.a
NOTICE: markRTEForSelectPriv: ACL_SELECT on t1.c
NOTICE: ExecCheckRTEPerms: ACL_SELECT on t1 perms = 0000 inh = 1
NOTICE: ExecCheckRTEPerms: selectedCols: t1.a
NOTICE: ExecCheckRTEPerms: selectedCols: t1.c
NOTICE: ExecCheckRTEPerms: ACL_SELECT on t1 perms = 0002 inh = 0
NOTICE: ExecCheckRTEPerms: selectedCols: t1.a
NOTICE: ExecCheckRTEPerms: selectedCols: t1.c
NOTICE: ExecCheckRTEPerms: ACL_SELECT on t2 perms = 0002 inh = 0
NOTICE: ExecCheckRTEPerms: selectedCols: t2.a
NOTICE: ExecCheckRTEPerms: selectedCols: t2.c
a | c
---+---
(0 rows)

postgres=# select t1 from t1;
NOTICE: markRTEForSelectPriv: ACL_SELECT on t1.t1
NOTICE: ExecCheckRTEPerms: ACL_SELECT on t1 perms = 0000 inh = 1
NOTICE: ExecCheckRTEPerms: selectedCols: t1.t1
NOTICE: ExecCheckRTEPerms: ACL_SELECT on t1 perms = 0002 inh = 0
NOTICE: ExecCheckRTEPerms: selectedCols: t1.t1
NOTICE: ExecCheckRTEPerms: ACL_SELECT on t2 perms = 0002 inh = 0
NOTICE: ExecCheckRTEPerms: selectedCols: t2.a
NOTICE: ExecCheckRTEPerms: selectedCols: t2.c
t1
----
(0 rows)

KaiGai Kohei wrote:
> KaiGai Kohei wrote:
>> I've observed the behavior of column-level privileges and
>> required permissions with a few elog()s injected.
>>
>> I noticed rte->selectedCols is incorrect when we make a query
>> on inherited tables.
>>
>> See below:
>> -------------------------------------------------
>> postgres=# CREATE TABLE t1 (a int, b int, c int);
>> CREATE TABLE
>> postgres=# ALTER TABLE t1 DROP COLUMN b;
>> ALTER TABLE
>> postgres=# CREATE TABLE t2 (d int) inherits (t1);
>> CREATE TABLE
>> postgres=# SELECT * FROM t1;
>> NOTICE: markRTEForSelectPriv: ACL_SELECT on t1.a
>> NOTICE: markRTEForSelectPriv: ACL_SELECT on t1.c
>> NOTICE: ExecCheckRTEPerms: ACL_SELECT on t1 perms = 0000 inh = 1
>> NOTICE: ExecCheckRTEPerms: selectedCols: t1.a
>> NOTICE: ExecCheckRTEPerms: selectedCols: t1.c
>> NOTICE: ExecCheckRTEPerms: ACL_SELECT on t1 perms = 0002 inh = 0
>> NOTICE: ExecCheckRTEPerms: selectedCols: t1.a
>> NOTICE: ExecCheckRTEPerms: selectedCols: t1.c
>> NOTICE: ExecCheckRTEPerms: ACL_SELECT on t2 perms = 0002 inh = 0
>> NOTICE: ExecCheckRTEPerms: selectedCols: t2.a
>> NOTICE: ExecCheckRTEPerms: selectedCols: t2.d <--- (*)
>> a | c
>> ---+---
>> (0 rows)
>> -------------------------------------------------
>>
>> I injected elog() at the head of ExecCheckRTEPerms() to print requiredPerms
>> and all the columns on selectedCols/modifiedCols.
>>
>> It seems to me the current implementation assumes the parant table and
>> child table have same set of attribute name/number pair, but incorrect.
>> It is necessary to lookup attribute names of "t2" when we extract
>> inherited tables.
>
> In addition, the whole-row-reference can be problematic.
>
> When we run "SELECT t1 FROM t1", the column level privilege tries to check
> all the columns within t1 and t2. But, I think it should not check on "t2.d"
> in this case, because the column is not a target of this query.
>
> In the whole-row-reference case, attno==0 on the parent table should be
> extracted into correct set of columns on the children.
>
> Thanks,

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

Attachment Content-Type Size
pgsql-v8.4-colpriv-fixup-inherited-tables.patch text/x-patch 3.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sophie Yang 2009-03-05 07:36:01 Use array in a dynamic statement
Previous Message Heikki Linnakangas 2009-03-05 07:31:17 Re: Patch for the MUST time zone (Mauritius Summer Time)