Re: UPDATE SET (a,b,c) = (SELECT ...) versus rules

As I mentioned awhile ago, I'm thinking about implementing the
SQL-standard construct

UPDATE foo SET ..., (a,b,...) = (SELECT x,y,...), ...

I've run into a rather nasty problem, which is how does this interact
with expansion of NEW references in ON UPDATE rules? For example,
suppose foo has a rule

ON UPDATE DO ALSO INSERT INTO foolog VALUES (new.a, new.b, ...);

The existing implementation relies on being able to pull expressions
for individual fields' new values out of the UPDATE targetlist; but
there is no independent expression for the new value of "a" here.
Worse yet, the NEW references might be in WHERE quals, or some other
place outside the targetlist of the rule query, which pretty much
breaks the implementation I'd sketched earlier.

The best that I think is reasonable to do in such cases is to pull out
a separate copy of the sub-select for each actual NEW reference in a
rule query. So the example above would give rise to an expanded
rule query along the lines of

INSERT INTO foolog VALUES ( (SELECT x as a, y as b, ...).a,
(SELECT x as a, y as b, ...).b,
... );

which would work, but it would re-evaluate the sub-select more times
than the user might be hoping. (Of course, if there are volatile
functions in the sub-select, he's screwed, but that's not a new
problem with rules.)

Given that ON UPDATE rules are close to being a deprecated feature,
it doesn't seem appropriate to work harder than this; and frankly
I don't see how we could avoid multiple sub-select evaluations anyway,
if the NEW references are in WHERE or other odd places.

Another possible answer is to just throw a "not implemented" error;
but that doesn't seem terribly helpful, and I think it wouldn't save
a lot of code anyway.

Thoughts?

regards, tom lane

Hi,

On 2014-06-14 15:35:33 -0400, Tom Lane wrote:
> Given that ON UPDATE rules are close to being a deprecated feature,
> it doesn't seem appropriate to work harder than this; and frankly
> I don't see how we could avoid multiple sub-select evaluations anyway,
> if the NEW references are in WHERE or other odd places.
>
> Another possible answer is to just throw a "not implemented" error;
> but that doesn't seem terribly helpful, and I think it wouldn't save
> a lot of code anyway.

I vote for throwing an error. This would make the rules about how rules
can be used safely even more confusing. I don't think anybody would be
helped by that. If somebody wrote a halfway sane ON UPDATE rule
(i.e. calling a function to do the dirty work) it wouldn't be sane
anymore if somebody starts to use the new syntax...

Greetings,

Andres Freund

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

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> Hi,
> On 2014-06-14 15:35:33 -0400, Tom Lane wrote:
>> Given that ON UPDATE rules are close to being a deprecated feature,
>> it doesn't seem appropriate to work harder than this; and frankly
>> I don't see how we could avoid multiple sub-select evaluations anyway,
>> if the NEW references are in WHERE or other odd places.
>>
>> Another possible answer is to just throw a "not implemented" error;
>> but that doesn't seem terribly helpful, and I think it wouldn't save
>> a lot of code anyway.

> I vote for throwing an error. This would make the rules about how rules
> can be used safely even more confusing. I don't think anybody would be
> helped by that. If somebody wrote a halfway sane ON UPDATE rule
> (i.e. calling a function to do the dirty work) it wouldn't be sane
> anymore if somebody starts to use the new syntax...

Well, it wouldn't be "unsafe" (barring volatile functions in the UPDATE,
which are unsafe already). It might be slow, but that's probably better
than failing.

regards, tom lane

On 2014-06-14 15:48:52 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > Hi,
> > On 2014-06-14 15:35:33 -0400, Tom Lane wrote:
> >> Given that ON UPDATE rules are close to being a deprecated feature,
> >> it doesn't seem appropriate to work harder than this; and frankly
> >> I don't see how we could avoid multiple sub-select evaluations anyway,
> >> if the NEW references are in WHERE or other odd places.
> >>
> >> Another possible answer is to just throw a "not implemented" error;
> >> but that doesn't seem terribly helpful, and I think it wouldn't save
> >> a lot of code anyway.
>
> > I vote for throwing an error. This would make the rules about how rules
> > can be used safely even more confusing. I don't think anybody would be
> > helped by that. If somebody wrote a halfway sane ON UPDATE rule
> > (i.e. calling a function to do the dirty work) it wouldn't be sane
> > anymore if somebody starts to use the new syntax...
>
> Well, it wouldn't be "unsafe" (barring volatile functions in the UPDATE,
> which are unsafe already). It might be slow, but that's probably better
> than failing.

I forgot the details, but IIRC it's possible to write a ON UPDATE ...
DO INSTEAD rule that's safe wrt multiple evaluations today by calling a
function passing in the old pkey and NEW. At least I believed so at some
point in the past :P

Greetings,

Andres Freund

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

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2014-06-14 15:48:52 -0400, Tom Lane wrote:
>> Well, it wouldn't be "unsafe" (barring volatile functions in the UPDATE,
>> which are unsafe already). It might be slow, but that's probably better
>> than failing.

> I forgot the details, but IIRC it's possible to write a ON UPDATE ...
> DO INSTEAD rule that's safe wrt multiple evaluations today by calling a
> function passing in the old pkey and NEW. At least I believed so at some
> point in the past :P

Hm. But you might as well use a trigger, no? Is anyone likely to
actually be doing such a thing?

It's conceivable that we could optimize the special case of NEW.*,
especially if it appears in the rule query's targetlist. But it's
trouble I don't really care to undertake ...

regards, tom lane

On 2014-06-14 16:44:10 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-06-14 15:48:52 -0400, Tom Lane wrote:
> >> Well, it wouldn't be "unsafe" (barring volatile functions in the UPDATE,
> >> which are unsafe already). It might be slow, but that's probably better
> >> than failing.
>
> > I forgot the details, but IIRC it's possible to write a ON UPDATE ...
> > DO INSTEAD rule that's safe wrt multiple evaluations today by calling a
> > function passing in the old pkey and NEW. At least I believed so at some
> > point in the past :P
>
> Hm. But you might as well use a trigger, no? Is anyone likely to
> actually be doing such a thing?

I don't think anybody is likely to do such a thing on an actual table,
but INSTEAD OF for views is pretty new. For a long time rules were the
the only way to implement updatable views (including any form of row
level security).

> It's conceivable that we could optimize the special case of NEW.*,
> especially if it appears in the rule query's targetlist. But it's
> trouble I don't really care to undertake ...

I think it's fine to just throw an error.

Greetings,

Andres Freund

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

On 6/14/14, 3:51 PM, Andres Freund wrote:
>> Hm. But you might as well use a trigger, no? Is anyone likely to
>> >actually be doing such a thing?
> I don't think anybody is likely to do such a thing on an actual table,
> but INSTEAD OF for views is pretty new. For a long time rules were the
> the only way to implement updatable views (including any form of row
> level security).
>
>> >It's conceivable that we could optimize the special case of NEW.*,
>> >especially if it appears in the rule query's targetlist. But it's
>> >trouble I don't really care to undertake ...
> I think it's fine to just throw an error.

If there was a showstopper to moving forward with rule support I think it'd be OK to throw our hands in the air, but that's not the case here.

I'm in favor of doing the substitution, just like we do today with RULES, warts and all. We already warn people against using rules and that they're very difficult to get correct, so I don't think double eval of an expression should surprise anyone.
--
Jim C. Nasby, Data Architect jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net

On 2014-06-14 19:27:03 -0500, Jim Nasby wrote:
> On 6/14/14, 3:51 PM, Andres Freund wrote:
> >>Hm. But you might as well use a trigger, no? Is anyone likely to
> >>>actually be doing such a thing?
> >I don't think anybody is likely to do such a thing on an actual table,
> >but INSTEAD OF for views is pretty new. For a long time rules were the
> >the only way to implement updatable views (including any form of row
> >level security).
> >
> >>>It's conceivable that we could optimize the special case of NEW.*,
> >>>especially if it appears in the rule query's targetlist. But it's
> >>>trouble I don't really care to undertake ...
> >I think it's fine to just throw an error.
>
> If there was a showstopper to moving forward with rule support I think
> it'd be OK to throw our hands in the air, but that's not the case
> here.
>
> I'm in favor of doing the substitution, just like we do today with
> RULES, warts and all. We already warn people against using rules and
> that they're very difficult to get correct, so I don't think double
> eval of an expression should surprise anyone.

It makes a formerly correct/safe rule unsafe. That's a showstopper from
my POV.

There's *STILL* no proper warning against rules in the manual, btw.

Greetings,

Andres Freund

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

On Sat, Jun 14, 2014 at 03:35:33PM -0400, Tom Lane wrote:
> The best that I think is reasonable to do in such cases is to pull out
> a separate copy of the sub-select for each actual NEW reference in a
> rule query. So the example above would give rise to an expanded
> rule query along the lines of
>
> INSERT INTO foolog VALUES ( (SELECT x as a, y as b, ...).a,
> (SELECT x as a, y as b, ...).b,
> ... );

Would it not be possible to use WITH here, like:

WITH bar AS ( ... subselect ... )
INSERT INTO foolog VALUES (bar.a, bar.b, ...)

Or am I missing something?

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> He who writes carelessly confesses thereby at the very outset that he does
> not attach much importance to his own thoughts.
-- Arthur Schopenhauer

On 06/14/2014 09:35 PM, Tom Lane wrote:
> As I mentioned awhile ago, I'm thinking about implementing the
> SQL-standard construct
>
> UPDATE foo SET ..., (a,b,...) = (SELECT x,y,...), ...
>
> I've run into a rather nasty problem, which is how does this interact
> with expansion of NEW references in ON UPDATE rules?
Was'nt there a plan (consensus?) about deprecating rules altogether ?

Cheers
Hannu
> For example,
> suppose foo has a rule
>
> ON UPDATE DO ALSO INSERT INTO foolog VALUES (new.a, new.b, ...);
>
> The existing implementation relies on being able to pull expressions
> for individual fields' new values out of the UPDATE targetlist; but
> there is no independent expression for the new value of "a" here.
> Worse yet, the NEW references might be in WHERE quals, or some other
> place outside the targetlist of the rule query, which pretty much
> breaks the implementation I'd sketched earlier.
>
> The best that I think is reasonable to do in such cases is to pull out
> a separate copy of the sub-select for each actual NEW reference in a
> rule query. So the example above would give rise to an expanded
> rule query along the lines of
>
> INSERT INTO foolog VALUES ( (SELECT x as a, y as b, ...).a,
> (SELECT x as a, y as b, ...).b,
> ... );
>
> which would work, but it would re-evaluate the sub-select more times
> than the user might be hoping. (Of course, if there are volatile
> functions in the sub-select, he's screwed, but that's not a new
> problem with rules.)
>
> Given that ON UPDATE rules are close to being a deprecated feature,
> it doesn't seem appropriate to work harder than this; and frankly
> I don't see how we could avoid multiple sub-select evaluations anyway,
> if the NEW references are in WHERE or other odd places.
>
> Another possible answer is to just throw a "not implemented" error;
> but that doesn't seem terribly helpful, and I think it wouldn't save
> a lot of code anyway.
>
> Thoughts?
>
> regards, tom lane
>
>

--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ

On 06/17/2014 09:43 AM, Hannu Krosing wrote:
> On 06/14/2014 09:35 PM, Tom Lane wrote:
>> > As I mentioned awhile ago, I'm thinking about implementing the
>> > SQL-standard construct
>> >
>> > UPDATE foo SET ..., (a,b,...) = (SELECT x,y,...), ...
>> >
>> > I've run into a rather nasty problem, which is how does this interact
>> > with expansion of NEW references in ON UPDATE rules?
>
> Was'nt there a plan (consensus?) about deprecating rules altogether ?

I believe that was just for user access to them, ie CREATE RULE. I
don't think there was ever question of purging them from the code base.
--
Vik

On 06/17/2014 11:22 AM, Vik Fearing wrote:
> On 06/17/2014 09:43 AM, Hannu Krosing wrote:
>> On 06/14/2014 09:35 PM, Tom Lane wrote:
>>>> As I mentioned awhile ago, I'm thinking about implementing the
>>>> SQL-standard construct
>>>>
>>>> UPDATE foo SET ..., (a,b,...) = (SELECT x,y,...), ...
>>>>
>>>> I've run into a rather nasty problem, which is how does this interact
>>>> with expansion of NEW references in ON UPDATE rules?
>> Was'nt there a plan (consensus?) about deprecating rules altogether ?
> I believe that was just for user access to them, ie CREATE RULE. I
> don't think there was ever question of purging them from the code base.
But are there any cases, where UPDATE rules are created behind the scenes ?

--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ

On 2014-06-17 11:22:17 +0200, Vik Fearing wrote:
> On 06/17/2014 09:43 AM, Hannu Krosing wrote:
> > On 06/14/2014 09:35 PM, Tom Lane wrote:
> >> > As I mentioned awhile ago, I'm thinking about implementing the
> >> > SQL-standard construct
> >> >
> >> > UPDATE foo SET ..., (a,b,...) = (SELECT x,y,...), ...
> >> >
> >> > I've run into a rather nasty problem, which is how does this interact
> >> > with expansion of NEW references in ON UPDATE rules?
> >
> > Was'nt there a plan (consensus?) about deprecating rules altogether ?
>
> I believe that was just for user access to them, ie CREATE RULE. I
> don't think there was ever question of purging them from the code base.

I don't think any such concensus has been made? I wish it were, but the
last discussions about it imo ended quite differently.

Greetings,

Andres Freund

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

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2014-06-17 11:22:17 +0200, Vik Fearing wrote:
>> On 06/17/2014 09:43 AM, Hannu Krosing wrote:
>>> Was'nt there a plan (consensus?) about deprecating rules altogether ?

>> I believe that was just for user access to them, ie CREATE RULE. I
>> don't think there was ever question of purging them from the code base.

> I don't think any such concensus has been made? I wish it were, but the
> last discussions about it imo ended quite differently.

Yeah, I don't think there's any prospect of removing them in the near
future. We'd need a (better-designed) replacement feature first.

regards, tom lane

On 2014-06-17 09:46:13 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-06-17 11:22:17 +0200, Vik Fearing wrote:
> >> On 06/17/2014 09:43 AM, Hannu Krosing wrote:
> >>> Was'nt there a plan (consensus?) about deprecating rules altogether ?
>
> >> I believe that was just for user access to them, ie CREATE RULE. I
> >> don't think there was ever question of purging them from the code base.
>
> > I don't think any such concensus has been made? I wish it were, but the
> > last discussions about it imo ended quite differently.
>
> Yeah, I don't think there's any prospect of removing them in the near
> future. We'd need a (better-designed) replacement feature first.

IMO INSTEAD triggers pretty much are that. We only need to make them
work for normal relations as well (partitioning!) and we're pretty much
there.

Greetings,

Andres Freund

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

Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> Would it not be possible to use WITH here, like:

> WITH bar AS ( ... subselect ... )
> INSERT INTO foolog VALUES (bar.a, bar.b, ...)

Don't think it works if the sub-select is correlated.

Consider something like

UPDATE summary_table s
SET (sumx, sumy) = (SELECT sum(x), sum(y) FROM detail_table d
WHERE d.group = s.group)

and suppose we have a logging rule like the above on summary_table.
You can't push the sub-select into a WITH because it depends on
s.group. With sufficient intelligence you could rewrite the query
entirely, I guess, but no simple transformation is going to cope.

But come to think of it, WITH is already an interesting precedent: if you
look into rewriteHandler.c you'll notice a boatload of corner cases where
the rewriter just throws up its hands for various combinations of rules
and statements containing WITH. So maybe that lends a bit more weight
to Andres' position that it's okay to consider this an unimplemented
feature.

regards, tom lane

On Tue, Jun 17, 2014 at 9:02 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> But come to think of it, WITH is already an interesting precedent: if you
> look into rewriteHandler.c you'll notice a boatload of corner cases where
> the rewriter just throws up its hands for various combinations of rules
> and statements containing WITH. So maybe that lends a bit more weight
> to Andres' position that it's okay to consider this an unimplemented
> feature.

This reflects previous consensus AIUI. RULES came up in similar way
with the 'data modifying with' feature; it was decided that as long as
old stuff didn't break new features don't necessarily have to go
through the motions. This essentially deprecates rules IMO, which is
fine. Maybe a small adjustment of the note in the rule documentation
couldn't hurt; it currently warns based on performance...a heads up
that current and future SQL features might not be fully supported
would be nice.

merlin

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2014-06-14 19:27:03 -0500, Jim Nasby wrote:
>> I'm in favor of doing the substitution, just like we do today with
>> RULES, warts and all. We already warn people against using rules and
>> that they're very difficult to get correct, so I don't think double
>> eval of an expression should surprise anyone.

> It makes a formerly correct/safe rule unsafe. That's a showstopper from
> my POV.

Andres' objection has some merit, and it doesn't seem like very many
people are concerned about throwing errors for new constructs if it's hard
to make them work with rules. So I've updated the patch to throw an error
for now. We can always revisit it later if someone has a good idea about
how to implement it.

> There's *STILL* no proper warning against rules in the manual, btw.

Not sure what you want for a "proper warning", but I put in a <caution>
that I think mentions the known gotchas.

This patch is now complete as far as I'm concerned. If there are not
objections I'll be committing it before long.

regards, tom lane