Re: New patch for Column-level privileges - Mailing list pgsql-hackers

Tom Lane wrote:
> I'm thinking make_var is not the place to do this.  The places that are
> supposed to be taking care of permissions are the ones that do this:
> 
>         /* Require read access --- see comments in setTargetTable() */
>         rte->requiredPerms |= ACL_SELECT;

Indeed, it seems to me appropriate policy, because CLP feature is
a part of SQL standard access control model.


The rte->requiredPerms is set on the following places:

(1) transformLockingClause() and markQueryForLocking()
It adds ACL_SELECT_FOR_UPDATE for listed relations.
In this case, column-level priv feature checks ACL_UPDATE for each
columns on rte->cols_sel, doesn't it?
So, I think it is unnecessary to care about anything here.

(2) setTargetTable()
It is invoked from transformInsertStmt(), transformUpdateStmt() and
transformDeleteStmt(). I thnk it is a proper place to set rte->cols_mod,
but the caller does not initialize Query->targetList yet, when it is
invoked.
So, I think we need put a function call to set cols_mod on caller side
based on Query->resultRelation and Query->targetList.

(3) scanRTEForColumn()
Stephen's original one patched here, but it does not handle RTE_JOIN
well, so it made a matter on table-joins.
Please revert a code to mark rte->cols_sel, with proper handling for
RTE_JOIN cases.

(4) addRangeTableEntry()
Purpose of the function is to translate RangeVar node to RangeTblEntry
node listed on FROM clause and so on.
I think we don't need to add any column references here.
When the translated relation used for column-references, it is a case
that rte->cols_sel is empty.

(5) addRangeTableEntryForRelation()
It is similar to addRangeTableEntry().
I think we don't need to add something here.

(6) ExpandColumnRefStar() and ExpandAllTables()
They invoke expandRelAttrs() to handle "SELECT * FROM t;" case.
I think here is a proper point to mark refered columns.
Please note that here is no guarantee that given RangeTblEntry has
RTE_RELATION.

Thus, we need the following reworking in my opinion.
(a) Implement a function to set rte->cols_sel and rte->cols_mod correctly,   even if the given rte has RTE_JOIN.
(b) Put a function to set rte->cols_mod on the caller of setTargetTable().
(c) Put a function to set rte->cols_sel on scanRTEForColumn(), expandRelAttrs()   and transformWholeRowRef().

> It's possible that we've missed some --- in particular, right at the
> moment I am not sure that whole-row Vars are handled properly.

Is transformWholeRowRef() a proper place to handle it?
If given RangeTblEntry has RTE_JOIN, it has to resolve it and set
its source's cols_sel.

> And maybe we could refactor a little bit to save some code.
> But those are basically the same places that ought to be adding
> bits to the column bitmaps.

-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>