Horton
Menu

Thread

  1. Writeable CTEs

    Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> — 2010-01-05T16:42:27Z

    Hi,

Attached is a patch where I've taken into account the problems Tom 
pointed out in his review [1].  It still needs a bit cleaning up, but 
I'm not planning on any big changes.  Any feedback welcome.

One thing I only noticed now is this:

=> select * from foo;
  a
---
  0
(1 row)

=> with t as (delete from foo returning *)
-> insert into bar
-> select * from t;
INSERT 0 2

It correctly reports 2 affected rows (one deleted and one inserted), but 
is this the answer we want?  It doesn't seem all that useful to know the 
total amount of affected rows.


Regards,
Marko Tiikkaja

[1] http://archives.postgresql.org/pgsql-hackers/2009-11/msg01860.php

  2. Re: Writeable CTEs

    Greg Stark <gsstark@mit.edu> — 2010-01-05T17:21:12Z

    On Tue, Jan 5, 2010 at 4:42 PM, Marko Tiikkaja
<marko.tiikkaja@cs.helsinki.fi> wrote:
> => with t as (delete from foo returning *)
> -> insert into bar
> -> select * from t;
> INSERT 0 2
>
> It correctly reports 2 affected rows (one deleted and one inserted), but is
> this the answer we want?  It doesn't seem all that useful to know the total
> amount of affected rows.

My first thought is that the number should correspond to the INSERT.
It didn't INSERT two rows so it seems wrong. More importantly in a
case like

with t as (delete from foo returning *)
select * from t where x=?

applications will almost certainly expect the number to match the
actual number of rows returned and may well misbehave if they don't.

-- 
greg

  3. Re: Writeable CTEs

    David Fetter <david@fetter.org> — 2010-01-05T17:34:47Z

    On Tue, Jan 05, 2010 at 05:21:12PM +0000, Greg Stark wrote:
> On Tue, Jan 5, 2010 at 4:42 PM, Marko Tiikkaja
> <marko.tiikkaja@cs.helsinki.fi> wrote:
> > => with t as (delete from foo returning *)
> > -> insert into bar
> > -> select * from t;
> > INSERT 0 2
> >
> > It correctly reports 2 affected rows (one deleted and one
> > inserted), but is this the answer we want?  It doesn't seem all
> > that useful to know the total amount of affected rows.
> 
> My first thought is that the number should correspond to the INSERT.
> It didn't INSERT two rows so it seems wrong. More importantly in a
> case like
> 
> with t as (delete from foo returning *) select * from t where x=?
> 
> applications will almost certainly expect the number to match the
> actual number of rows returned and may well misbehave if they don't.

I'm not sure how relevant this could be, as existing apps can't use
future functionality.  We have precedents with RULEs, which can make
the arguments pretty meaningless.

In some future version, we may want to redo the infrastructure to
support "modified" values for multiple statements, but for now, that
seems like an unnecessary frill.

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  4. Re: Writeable CTEs

    Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> — 2010-01-05T17:45:30Z

    On 2010-01-05 19:21 +0200, Greg Stark wrote:
> with t as (delete from foo returning *)
> select * from t where x=?
>
> applications will almost certainly expect the number to match the
> actual number of rows returned and may well misbehave if they don't.

I probably wasn't clear about the actual problem in the original post. 
The problem only affects INSERT, UDPATE and DELETE where you are 
actually counting affected rows (i.e. PQcmdTuples(), not PQntuples()) so 
the this example would work as expected.


Regards,
Marko Tiikkaja

  5. Re: Writeable CTEs

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-05T18:40:28Z

    Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes:
> => with t as (delete from foo returning *)
> -> insert into bar
> -> select * from t;
> INSERT 0 2

> It correctly reports 2 affected rows (one deleted and one inserted), but 
> is this the answer we want?

No.  The returned tag should consider only the top-level operation,
not what happened inside any CTEs.

			regards, tom lane

  6. Re: Writeable CTEs

    Josh Berkus <josh@agliodbs.com> — 2010-01-05T21:20:52Z

    On 1/5/10 9:45 AM, Marko Tiikkaja wrote:
> On 2010-01-05 19:21 +0200, Greg Stark wrote:
>> with t as (delete from foo returning *)
>> select * from t where x=?
>>
>> applications will almost certainly expect the number to match the
>> actual number of rows returned and may well misbehave if they don't.
> 
> I probably wasn't clear about the actual problem in the original post.
> The problem only affects INSERT, UDPATE and DELETE where you are
> actually counting affected rows (i.e. PQcmdTuples(), not PQntuples()) so
> the this example would work as expected.

I don't think there is an "as expected" for this situation; people won't
know what to expect. So what do we think is resonable?  The current
behavior, which reports the total count of rows expected, works for me.

--Josh Berkus

  7. Re: Writeable CTEs

    Robert Haas <robertmhaas@gmail.com> — 2010-01-05T21:46:48Z

    On Tue, Jan 5, 2010 at 4:20 PM, Josh Berkus <josh@agliodbs.com> wrote:
> On 1/5/10 9:45 AM, Marko Tiikkaja wrote:
>> On 2010-01-05 19:21 +0200, Greg Stark wrote:
>>> with t as (delete from foo returning *)
>>> select * from t where x=?
>>>
>>> applications will almost certainly expect the number to match the
>>> actual number of rows returned and may well misbehave if they don't.
>>
>> I probably wasn't clear about the actual problem in the original post.
>> The problem only affects INSERT, UDPATE and DELETE where you are
>> actually counting affected rows (i.e. PQcmdTuples(), not PQntuples()) so
>> the this example would work as expected.
>
> I don't think there is an "as expected" for this situation; people won't
> know what to expect. So what do we think is resonable?  The current
> behavior, which reports the total count of rows expected, works for me.

I agree with Tom's statement upthread that we should only count the
rows affected by the top-level query.  Anything else seems extremely
counter-intuitive.

...Robert

  8. Re: Writeable CTEs

    Josh Berkus <josh@agliodbs.com> — 2010-01-05T21:58:37Z

    > I agree with Tom's statement upthread that we should only count the
> rows affected by the top-level query.  Anything else seems extremely
> counter-intuitive.

I'm ok with that.  I don't think there is any kind of intuitive behavior
in this situation, and we just need to pick something and document it.

--Josh

  9. Re: Writeable CTEs

    Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> — 2010-01-12T18:55:54Z

    On 2010-01-05 20:40 +0200, Tom Lane wrote:
> Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes:
>> => with t as (delete from foo returning *)
>> -> insert into bar
>> -> select * from t;
>> INSERT 0 2
> 
>> It correctly reports 2 affected rows (one deleted and one inserted), but 
>> is this the answer we want?
> 
> No.  The returned tag should consider only the top-level operation,
> not what happened inside any CTEs.

Attached is the latest version of the patch.  This fixes all issues I'm
aware of.  This one also allows forward-referencing and recursive CTEs
in the same CTE list with writeable CTEs, using the RECURSIVE keyword.


Regards,
Marko Tiikkaja