[BUG] Column-level privileges on inherited tables

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@ak.jp.nec.com>

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@ak.jp.nec.com>

KaiGai,

* KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:

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

Thanks! It looks good to me, but we'll need Tom or some other
committer to review it and commit it, of course.

Thanks again,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

KaiGai,
* KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:

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

Thanks! It looks good to me, but we'll need Tom or some other
committer to review it and commit it, of course.

It's duplicating (rather badly) the work done by
make_inh_translation_list. I'll see about refactoring it to
make use of that translation data.

regards, tom lane