Re: [HACKERS] varattno remapping

[Craig Ringer]

On 12/24/2013 11:17 PM, Dean Rasheed wrote:
> I don't think this bit is quite right.
> 
> It's not correct to assume that all the view columns are simple
> references to columns of the base relation --- auto-updatable views
> may now contain a mix of updatable and non-updatable columns, so some
> of the view columns may be arbitrary expressions.

Ah - it looks like I'd checked against 9.3 and missed the relaxation of
those requirements.

> There is already code in rewriteTargetView() that does something very
> similar (to the whole parsetree, rather than just the returning list)
> with 2 function calls:

Copying the view tlist and then adjusting it is a much smarter way to do
it. I should've seen that the pull-up code could be adapted to deal with
the RETURNING list, so thankyou.

It's a cleaner way to do it.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] varattno remapping

[Dean Rasheed]

On 24 December 2013 12:12, Craig Ringer  wrote:
> On 12/24/2013 03:21 PM, Abbas Butt wrote:
>
>> Could you please explain a little bit more how would you solve the posed
>> problem using map_variable_attnos?
>
> It actually turns out to be even simpler, and easy to do in one pass,
> when using ReplaceVarsFromTargetList .
>
> You just generate a temporary new list of TargetEntry, with resnos
> matching the attribute numbers of the view. Each contained Var points to
> the remapped varno and varattno.
>
> Then you let ReplaceVarsFromTargetList substitute Vars recursively
> through the expression tree with ones in the replacement tlist you supply.
>
> The more I've thought about it, the shorter this code has got. Currently:
>
>
>
>
> /*
>  * Scan the passed view target list, whose members must consist solely
>  * of Var nodes with a varno equal to the passed targetvarno, and
>  * produce a targetlist of Var nodes with the corresponding varno and
>  * varattno of the base relation 'targetvarno'.
>  *
>  * This tlist is used when remapping Vars that point to a view so they
>  * point to the base relation of the view instead. It is entirely
>  * newly allocated. The result tlist is not in resno order.
>  *
>  * Must not be called with a targetlist containing non-Var entries.
>  */
> static List *
> gen_view_base_attr_map(List *viewtlist, int targetvarno, int newvarno)
> {
> ListCell*lc;
> TargetEntry *te, *newte;
> Var *tev, *newvar;
> int l_viewtlist = list_length(viewtlist);
> List*newtlist = NIL;
>
> foreach(lc, viewtlist)
> {
> te = (TargetEntry*) lfirst(lc);
> /* Could relax this in future and map only the var entries,
>  * ignoring everything else, but currently pointless since we
>  * are only interested in simple views. */
> Assert(IsA(te->expr, Var));
> tev = (Var*) te->expr;
> Assert(tev->varno == targetvarno);
> Assert(te->resno - 1 < l_viewtlist);
>
> /* Construct the new Var with the remapped attno and varno */
> newvar = (Var*) palloc(sizeof(Var));
> *newvar = *tev;
> newvar->varno = newvarno;
> newvar->varattno = tev->varattno;
>
> /* and wrap it in a new tle to cons to the list */
> newte = flatCopyTargetEntry(te);
> newte->expr = (Expr*) newvar;
> newtlist = lcons(newte, newtlist);
> }
>
> return newtlist;
> }
>
>

I don't think this bit is quite right.

It's not correct to assume that all the view columns are simple
references to columns of the base relation --- auto-updatable views
may now contain a mix of updatable and non-updatable columns, so some
of the view columns may be arbitrary expressions.

There is already code in rewriteTargetView() that does something very
similar (to the whole parsetree, rather than just the returning list)
with 2 function calls:

/*
 * Make a copy of the view's targetlist, adjusting its Vars to reference
 * the new target RTE, ie make their varnos be new_rt_index instead of
 * base_rt_index.  There can be no Vars for other rels in the tlist, so
 * this is sufficient to pull up the tlist expressions for use in the
 * outer query.  The tlist will provide the replacement expressions used
 * by ReplaceVarsFromTargetList below.
 */
view_targetlist = copyObject(viewquery->targetList);

ChangeVarNodes((Node *) view_targetlist,
   base_rt_index,
   new_rt_index,
   0);

and then later:

/*
 * Now update all Vars in the outer query that reference the view to
 * reference the appropriate column of the base relation instead.
 */
parsetree = (Query *)
ReplaceVarsFromTargetList((Node *) parsetree,
  parsetree->resultRelation,
  0,
  view_rte,
  view_targetlist,
  REPLACEVARS_REPORT_ERROR,
  0,
  &parsetree->hasSubLinks);


Regards,
Dean


>
>
> and invocation:
>
>
> /*
>  * We need to adjust any RETURNING clause entries to point to the new
>  * target RTE instead of the old one so that we see the effects of
>  * BEFORE triggers. Varattnos must be remapped so that the new Var
>  * points to the correct col of the base rel, since the view will
>  * usually have a different set of columns / column order.
>  *
>  * As part of this pass any whole-row references to the view are
>  * expanded into ROW(...) expressions to ensure we don't expose
>  * columns that are not visible through the view, and to make sure
>  * the client gets the result type it expected.
>  */
>
> List * remap_tlist = gen_view_base_attr_map(
> viewquery->targetList, rtr->rtindex, new_result_rt_index);
>
> parsetree->r

Re: [HACKERS] varattno remapping

[Craig Ringer]

On 12/24/2013 03:21 PM, Abbas Butt wrote:

> Could you please explain a little bit more how would you solve the posed
> problem using map_variable_attnos?

It actually turns out to be even simpler, and easy to do in one pass,
when using ReplaceVarsFromTargetList .

You just generate a temporary new list of TargetEntry, with resnos
matching the attribute numbers of the view. Each contained Var points to
the remapped varno and varattno.

Then you let ReplaceVarsFromTargetList substitute Vars recursively
through the expression tree with ones in the replacement tlist you supply.

The more I've thought about it, the shorter this code has got. Currently:




/*
 * Scan the passed view target list, whose members must consist solely
 * of Var nodes with a varno equal to the passed targetvarno, and
 * produce a targetlist of Var nodes with the corresponding varno and
 * varattno of the base relation 'targetvarno'.
 *
 * This tlist is used when remapping Vars that point to a view so they
 * point to the base relation of the view instead. It is entirely
 * newly allocated. The result tlist is not in resno order.
 *
 * Must not be called with a targetlist containing non-Var entries.
 */
static List *
gen_view_base_attr_map(List *viewtlist, int targetvarno, int newvarno)
{
ListCell*lc;
TargetEntry *te, *newte;
Var *tev, *newvar;
int l_viewtlist = list_length(viewtlist);
List*newtlist = NIL;

foreach(lc, viewtlist)
{
te = (TargetEntry*) lfirst(lc);
/* Could relax this in future and map only the var entries,
 * ignoring everything else, but currently pointless since we
 * are only interested in simple views. */
Assert(IsA(te->expr, Var));
tev = (Var*) te->expr;
Assert(tev->varno == targetvarno);
Assert(te->resno - 1 < l_viewtlist);

/* Construct the new Var with the remapped attno and varno */
newvar = (Var*) palloc(sizeof(Var));
*newvar = *tev;
newvar->varno = newvarno;
newvar->varattno = tev->varattno;

/* and wrap it in a new tle to cons to the list */
newte = flatCopyTargetEntry(te);
newte->expr = (Expr*) newvar;
newtlist = lcons(newte, newtlist);
}

return newtlist;
}




and invocation:


/*
 * We need to adjust any RETURNING clause entries to point to the new
 * target RTE instead of the old one so that we see the effects of
 * BEFORE triggers. Varattnos must be remapped so that the new Var
 * points to the correct col of the base rel, since the view will
 * usually have a different set of columns / column order.
 *
 * As part of this pass any whole-row references to the view are
 * expanded into ROW(...) expressions to ensure we don't expose
 * columns that are not visible through the view, and to make sure
 * the client gets the result type it expected.
 */

List * remap_tlist = gen_view_base_attr_map(
viewquery->targetList, rtr->rtindex, new_result_rt_index);

parsetree->returningList = ReplaceVarsFromTargetList(
(Node*) parsetree->returningList,
old_result_rt_index, 0 /*sublevels_up*/,
view_rte,
remap_tlist,
REPLACEVARS_REPORT_ERROR, 0 /*nomatch_varno, unused */,
NULL /* outer_hasSubLinks, unused */
);





-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] varattno remapping

[Craig Ringer]

On 12/24/2013 03:21 PM, Abbas Butt wrote:

> Could you please explain a little bit more how would you solve the posed
> problem using map_variable_attnos?
> 
> I was recently working on a similar problem and used the following algo
> to solve it.
> 
> I had to find to which column of the base table does a column in
> the select statement of the view query belong.
> To relate a target list entry in the select query of the view to
> an actual column in base table.

Sounds similar. My problem is simplified by the constraint that the view
must be a simple view (only one base relation) and must only contain
simple column-references to single the base relation. No  expressions,
no joins. (These are the rules for simply updatable views).

I'm new to the planner and rewriter, so don't take what I do as any kind
of example beyond "this seems to work".

I generate a varattno mapping as follows:

/*
 * Scan the passed view target list, whose members must consist solely
 * of Var nodes with a varno equal to the passed targetvarno.
 *
 * A mapping is built from the resno (i.e. tlist index) of the view
 * tlist to the corresponding attribute number of the base relation
 * the varattno points to.
 *
 * Must not be called with a targetlist containing non-Var entries.
 */
static void
gen_view_base_attr_map(List *viewtlist, AttrNumber * attnomap, int
targetvarno)
{
ListCell*lc;
TargetEntry *te;
Var *tev;
int l_viewtlist = list_length(viewtlist);

foreach(lc, viewtlist)
{
te = (TargetEntry*) lfirst(lc);
/* Could relax this in future and map only the var entries,
 * ignoring everything else, but currently pointless since we
 * are only interested in simple views. */
Assert(IsA(te->expr, Var));
tev = (Var*) te->expr;
Assert(tev->varno == targetvarno);
Assert(te->resno - 1 < l_viewtlist);
attnomap[te->resno - 1] = tev->varattno;
}
}


producing a forward mapping of view attno to base relation attno.



I then apply the varattno remapping, in this case to the returning list
of a DML query acting on a view, with something like:

varattno_map = palloc( list_length(viewquery->targetList) *
   sizeof(AttrNumber) );

gen_view_base_attr_map(viewquery->targetList,
   varattno_map, rtr->rtindex);

parsetree->returningList = map_variable_attnos(
(Node*) parsetree->returningList,
old_result_rt_index, 0,
varattno_map, list_length(viewquery->targetList),
&found_whole_row_var
);

if (found_whole_row_var)
{
/* TODO: Extend map_variable_attnos API to
   pass a mutator to handle whole-row vars. */
elog(ERROR, "RETURNING list contains a whole-row variable, "
"which is not currently supported for updatable "
"views");
}

ChangeVarNodes((Node*) parsetree->returningList,
   old_result_rt_index,
   new_result_rt_index,
   0);


I'd prefer to be doing the map_variable_attnos and ChangeVarNodes work
in a single pass, but it looks like a clumsy and verbose process to
write a new walker. So I'm going to leave it as an "opportunity for
future optimisation" for now ;-)




(As it happens that "found_whole_row_var" is a real pain; I'm going to
have to deal with a TODO item in map_variable_attnos to provide a
callback that replaces a whole-row Var with an expansion of it into a
row-expression).


-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] varattno remapping

[Abbas Butt]

On Tue, Dec 24, 2013 at 11:47 AM, Craig Ringer wrote:

> On 12/24/2013 02:35 PM, Craig Ringer wrote:
>
> > So the short version: Given the RTE for a simple view with only one base
> > rel and an attribute number for a col in that view, and assuming that
> > the view col is a simple reference to a col in the underlying base rel,
> > is there any sane way to get the attribute number for the corresponding
> > col on the base rel?
>
> So, of course, I find it as soon as I post.
>
> map_variable_attnos(...), also in src/backend/rewrite/rewriteManip.c .
>
> Just generate the mapping table and go.
>

Could you please explain a little bit more how would you solve the posed
problem using map_variable_attnos?

I was recently working on a similar problem and used the following algo to
solve it.

I had to find to which column of the base table does a column in the select
statement of the view query belong.
To relate a target list entry in the select query of the view to an actual
column in base table here is what I did

First determine whether the var's varno refer to an RTE which is a view?
If yes then get the view query (RangeTblEntry::subquery) and see which
element in the view query's target list does this var's varattno point to.
Get the varno of that target list entry. Look for that RTE in the view's
query and see if that RTE is a real table then use that var making sure its
varno now points to the actual table.

Thanks in advance.



>
> Sorry for the noise folks.
>
> --
>  Craig Ringer   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>



-- 
-- 
*Abbas*
 Architect

Ph: 92.334.5100153
 Skype ID: gabbasb
www.enterprisedb.co
m

*Follow us on Twitter*
@EnterpriseDB

Visit EnterpriseDB for tutorials, webinars,
whitepapersand
more

Re: [HACKERS] varattno remapping

[Craig Ringer]

On 12/24/2013 02:35 PM, Craig Ringer wrote:

> So the short version: Given the RTE for a simple view with only one base
> rel and an attribute number for a col in that view, and assuming that
> the view col is a simple reference to a col in the underlying base rel,
> is there any sane way to get the attribute number for the corresponding
> col on the base rel?

So, of course, I find it as soon as I post.

map_variable_attnos(...), also in src/backend/rewrite/rewriteManip.c .

Just generate the mapping table and go.

Sorry for the noise folks.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] varattno remapping

[Craig Ringer]

Hi all

I'm finding it necessary to remap the varno and varattno of Var elements
in RETURNING lists as part of updatable s.b. view processing. A
reference to a view col in RETURNING must be rewritten to point to the
new resultRelation at the top level, so that the effects of BEFORE
triggers are visible in the returned result.

Usually the view will have a different structure to the base table, and
in this case it's necessary to change the varattno of the Var to point
to the corresponding col on the base rel.

So the short version: Given the RTE for a simple view with only one base
rel and an attribute number for a col in that view, and assuming that
the view col is a simple reference to a col in the underlying base rel,
is there any sane way to get the attribute number for the corresponding
col on the base rel?



For example, given:

CREATE TABLE t (a integer, b text, c numeric);
CREATE VIEW v1 AS SELECT c, a FROM t;
UPDATE v1 SET c = NUMERIC '42' RETURNING a, c;

... the Vars for a and c in the RETURNING clause need to be remapped so
their varno is the rti for t once the RTE has been injected, and the
varattnos need changing to the corresponding attribute numbers on the
base table. So a Var for v1(c), say (1,1), must be remapped to (2,3)
i.e. varno 2 (t), varattno 3 (c).

I'm aware that this is somewhat like the var/attr twiddling being
complained about in the RLS patch. I don't see a way around it, though.
I can't push the RETURNING tlist entries down into the expanded view's
subquery and add an outer Var reference - they must point directly to
the resultRelation so that we see the effects of any BEFORE triggers.

I'm looking for advice on how to determine, given an RTI and an
attribute number for a simple view, what the attribute number of the col
in the view's base relation is. It can be assumed for this purpose that
the view attribute is a simple Var (otherwise we'll ERROR out, since we
can't update an expression).

I'm a bit stumped at this point.

I could adapt the ChangeVarNodes walker to handle the actual recursive
rewriting and Var changing. It assumes that the varattno is the same on
both the old and new varno, as it's intended for when you have an RT
index change, but could be taught to optionally remap varattnos. To do
that, though, I need a way to actually do that varattno remapping cleanly.

Anyone got any ideas?
--
Craig Ringer


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers