Re: Proof of concept: auto updatable views [Review of Patch]

Continuing to work on this ... I found multiple things I didn't like
about the permission-field update code. Attached are some heavily
commented extracts from the code as I've changed it. Does anybody
object to either the code or the objectives given in the comments?

regards, tom lane

/*
* adjust_view_column_set - map a set of column numbers according to targetlist
*
* This is used with simply-updatable views to map column-permissions sets for
* the view columns onto the matching columns in the underlying base relation.
* The targetlist is expected to be a list of plain Vars of the underlying
* relation (as per the checks above in view_is_auto_updatable).
*/
static Bitmapset *
adjust_view_column_set(Bitmapset *cols, List *targetlist)
{
Bitmapset *result = NULL;
Bitmapset *tmpcols;
AttrNumber col;

tmpcols = bms_copy(cols);
while ((col = bms_first_member(tmpcols)) >= 0)
{
/* bit numbers are offset by FirstLowInvalidHeapAttributeNumber */
AttrNumber attno = col + FirstLowInvalidHeapAttributeNumber;

if (attno == InvalidAttrNumber)
{
/*
* There's a whole-row reference to the view. For permissions
* purposes, treat it as a reference to each column available from
* the view. (We should *not* convert this to a whole-row
* reference to the base relation, since the view may not touch
* all columns of the base relation.)
*/
ListCell *lc;

foreach(lc, targetlist)
{
TargetEntry *tle = (TargetEntry *) lfirst(lc);
Var *var;

if (tle->resjunk)
continue;
var = (Var *) tle->expr;
Assert(IsA(var, Var));
result = bms_add_member(result,
var->varattno - FirstLowInvalidHeapAttributeNumber);
}
}
else
{
/*
* Views do not have system columns, so we do not expect to see
* any other system attnos here. If we do find one, the error
* case will apply.
*/
TargetEntry *tle = get_tle_by_resno(targetlist, attno);

if (tle != NULL && IsA(tle->expr, Var))
{
Var *var = (Var *) tle->expr;

result = bms_add_member(result,
var->varattno - FirstLowInvalidHeapAttributeNumber);
}
else
elog(ERROR, "attribute number %d not found in view targetlist",
attno);
}
}
bms_free(tmpcols);

return result;
}

/*
* Mark the new target RTE for the permissions checks that we want to
* enforce against the view owner, as distinct from the query caller. At
* the relation level, require the same INSERT/UPDATE/DELETE permissions
* that the query caller needs against the view. We drop the ACL_SELECT
* bit that is presumably in new_rte->requiredPerms initially.
*
* Note: the original view RTE remains in the query's rangetable list.
* Although it will be unused in the query plan, we need it there so that
* the executor still performs appropriate permissions checks for the
* query caller's use of the view.
*/
new_rte->checkAsUser = view->rd_rel->relowner;
new_rte->requiredPerms = view_rte->requiredPerms;

/*
* Now for the per-column permissions bits.
*
* Initially, new_rte contains selectedCols permission check bits for all
* base-rel columns referenced by the view, but since the view is a SELECT
* query its modifiedCols is empty. We set modifiedCols to include all
* the columns the outer query is trying to modify, adjusting the column
* numbers as needed. But we leave selectedCols as-is, so the view owner
* must have read permission for all columns used in the view definition,
* even if some of them are not read by the upper query. We could try to
* limit selectedCols to only columns used in the transformed query, but
* that does not correspond to what happens in ordinary SELECT usage of a
* view: all referenced columns must have read permission, even if
* optimization finds that some of them can be discarded during query
* transformation. The flattening we're doing here is an optional
* optimization, too. (If you are unpersuaded and want to change this,
* note that applying adjust_view_column_set to view_rte->selectedCols is
* clearly *not* the right answer, since that neglects base-rel columns
* used in the view's WHERE quals.)
*/
Assert(bms_is_empty(new_rte->modifiedCols));
new_rte->modifiedCols = adjust_view_column_set(view_rte->modifiedCols,
view_targetlist);