Failed assertion root->hasLateralRTEs on initsplan.c

Emre Hasegeli <emre@hasegeli.com> writes:

I get assertion failure on initsplan.c line 1325 while executing following query
on HEAD (edc43458d797a5956f4bf39af18cf62abb0077db). It works fine
without --enable-cassert.

update subscriber set properties = hstore(a) from (select firstname,
lastname from player where player.id = subscriber.id) as a;

Hm, AFAICS this query should absolutely *not* work; the reference to
subscriber.id inside the sub-select is illegal. It might be legal with
LATERAL, but not otherwise. So I think this is a parser bug, and there's
nothing wrong with the planner's Assert. 9.2 and earlier throw the
error I'd expect, so probably something in the LATERAL patches broke
this case; will look.

The next question is if we should allow it with LATERAL. That would
essentially be treating "subscriber" as having implicitly appeared at the
start of the FROM list, which I guess is all right ... but does anyone
want to argue against it? I seem to recall some old discussions about
allowing the update target to be explicitly shown in FROM, in case you
wanted say to left join it against something else. Allowing this implicit
appearance might limit our options if we ever get around to trying to do
that. On the other hand, those discussions were a long time back, so
maybe it'll never happen anyway.

regards, tom lane

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

On Tue, Jan 7, 2014 at 1:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Emre Hasegeli <emre@hasegeli.com> writes:

I get assertion failure on initsplan.c line 1325 while executing following query
on HEAD (edc43458d797a5956f4bf39af18cf62abb0077db). It works fine
without --enable-cassert.

update subscriber set properties = hstore(a) from (select firstname,
lastname from player where player.id = subscriber.id) as a;

Hm, AFAICS this query should absolutely *not* work; the reference to
subscriber.id inside the sub-select is illegal. It might be legal with
LATERAL, but not otherwise. So I think this is a parser bug, and there's
nothing wrong with the planner's Assert. 9.2 and earlier throw the
error I'd expect, so probably something in the LATERAL patches broke
this case; will look.

The next question is if we should allow it with LATERAL. That would
essentially be treating "subscriber" as having implicitly appeared at the
start of the FROM list, which I guess is all right ... but does anyone
want to argue against it? I seem to recall some old discussions about
allowing the update target to be explicitly shown in FROM, in case you
wanted say to left join it against something else. Allowing this implicit
appearance might limit our options if we ever get around to trying to do
that. On the other hand, those discussions were a long time back, so
maybe it'll never happen anyway.

I still think that would be a good thing to do, but I don't see a
problem. The way I imagine it would work is: if the alias used for
the update target also appears in the FROM clause, then we treat them
as the same thing (after checking that they refer to the same table in
both cases). Otherwise, we add the update target as an additional
from-list item.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Jan 7, 2014 at 1:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The next question is if we should allow it with LATERAL. That would
essentially be treating "subscriber" as having implicitly appeared at the
start of the FROM list, which I guess is all right ... but does anyone
want to argue against it? I seem to recall some old discussions about
allowing the update target to be explicitly shown in FROM, in case you
wanted say to left join it against something else. Allowing this implicit
appearance might limit our options if we ever get around to trying to do
that. On the other hand, those discussions were a long time back, so
maybe it'll never happen anyway.

I still think that would be a good thing to do, but I don't see a
problem. The way I imagine it would work is: if the alias used for
the update target also appears in the FROM clause, then we treat them
as the same thing (after checking that they refer to the same table in
both cases). Otherwise, we add the update target as an additional
from-list item.

Um, well, no; this does make it harder. Consider

update t1 ... from lateral (select...) ss join (t1 left join ...)

You propose that we identify t1 in the sub-JOIN clause with the target
table. What if we have already resolved some outer references in
subselect ss as belonging to t1? Now we have an illegal reference
structure in the FROM clause, which is likely to lead to all sorts
of grief.

I'm sure we could forbid this combination of features, with some klugy
parse-time check or other, but it feels like we started from wrong
premises somewhere.

It might be better if we simply didn't allow lateral references to the
target table for now. We could introduce them in combination with the
other feature, in which case we could say that the lateral reference has
to be to an explicit reference to the target table in FROM, ie, if you
want a lateral reference to t1 in ss you must write

update t1 ... from t1 join lateral (select...) ss;

The fly in the ointment is that we've already shipped a couple of
9.3.x releases that allowed lateral references to the target table.
Even though this wasn't suggested or documented anywhere, somebody
might be relying on it already.

I'm inclined though to pull it back anyway, now that I've thought
about it some more.

regards, tom lane

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

On 9 January 2014 15:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Jan 7, 2014 at 1:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The next question is if we should allow it with LATERAL. That would
essentially be treating "subscriber" as having implicitly appeared at the
start of the FROM list, which I guess is all right ... but does anyone
want to argue against it? I seem to recall some old discussions about
allowing the update target to be explicitly shown in FROM, in case you
wanted say to left join it against something else. Allowing this implicit
appearance might limit our options if we ever get around to trying to do
that. On the other hand, those discussions were a long time back, so
maybe it'll never happen anyway.

I still think that would be a good thing to do, but I don't see a
problem. The way I imagine it would work is: if the alias used for
the update target also appears in the FROM clause, then we treat them
as the same thing (after checking that they refer to the same table in
both cases). Otherwise, we add the update target as an additional
from-list item.

Um, well, no; this does make it harder. Consider

update t1 ... from lateral (select...) ss join (t1 left join ...)

You propose that we identify t1 in the sub-JOIN clause with the target
table. What if we have already resolved some outer references in
subselect ss as belonging to t1? Now we have an illegal reference
structure in the FROM clause, which is likely to lead to all sorts
of grief.

I'm sure we could forbid this combination of features, with some klugy
parse-time check or other, but it feels like we started from wrong
premises somewhere.

It might be better if we simply didn't allow lateral references to the
target table for now. We could introduce them in combination with the
other feature, in which case we could say that the lateral reference has
to be to an explicit reference to the target table in FROM, ie, if you
want a lateral reference to t1 in ss you must write

update t1 ... from t1 join lateral (select...) ss;

The fly in the ointment is that we've already shipped a couple of
9.3.x releases that allowed lateral references to the target table.
Even though this wasn't suggested or documented anywhere, somebody
might be relying on it already.

I'm inclined though to pull it back anyway, now that I've thought
about it some more.

While testing updatable s.b. views, I came up with the following test
case which shows that supporting lateral references to the target
table is more than just a matter of syntax. Consider the following
example:

create table t1(x int);
create table t2() inherits(t1);
create table t3(a int, b int);
update t1 set x=b from lateral (select * from t3 where a=x offset 0) t3;

In 9.3.2 and master, prior to this being disallowed, this raises the
following error:

ERROR: no relation entry for relid 1

because in inheritance_planner(), adjust_appendrel_attrs() uses
QTW_IGNORE_RC_SUBQUERIES and so doesn't process subqueries in the
rangetable, and so the reference to t1.x in the subquery isn't updated
to point to the appropriate append_rel child relation.

Of course, if adjust_appendrel_attrs() were made to process subqueries
in the rangetable, it would then also have to be able to deal with
not-yet-planned SubLinks that might appear there, as the updatable
s.b. views patch does, although maybe there's a different way of
handling this.

Regards,
Dean

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