Re: Writeable CTEs patch - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Writeable CTEs patch
Date
Msg-id 603c8f071002091439q45c182b0r6580dc3d011a063d@mail.gmail.com
Whole thread Raw
In response to Re: Writeable CTEs patch  (Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi>)
List pgsql-hackers
On Tue, Feb 9, 2010 at 3:13 PM, Marko Tiikkaja
<marko.tiikkaja@cs.helsinki.fi> wrote:
> On 2010-02-08 18:42 +0200, Robert Haas wrote:
>> On Thu, Feb 4, 2010 at 11:57 AM, Marko Tiikkaja
>> <marko.tiikkaja@cs.helsinki.fi> wrote:
>>> Here's an updated patch.  Only changes from the previous patch are
>>> fixing the above issue and a regression test for it.
>>
>> - I'm not sure that canSetTag is the right name for the additional
>> argument to ExecInsert/ExecUpdate/ExecDelete.  OTOH, I'm not sure it's
>> the wrong name either.  But should we use something like
>> isTopLevelQuery?
>
> I'm going to have to take back my previous statement; this doesn't make
> a lot of sense in the case of DO ALSO rules (or multiple statements in a
> DO INSTEAD RULE).  Those will have canSetTag=false, but they will be at
> the top level.

Ah.  OK.

>> - It appears that we pull out all of the DML statements first and run
>> them in order, but I'm not sure that's the right thing to do.
>> Consider:
>>
>> WITH x AS (INSERT ...), y AS (SELECT ...), z AS (INSERT ...) SELECT ...
>>
>> I would assume we would do x, CCI, do y, do z, CCI, do main query, but
>> I don't think that's what this implements.  The user might be
>> surprised to find out that y sees the effects of z.
>
> I went ahead and implemented this, but there seems to be one small
> problem: RECURSIVE.  If there is a recursive query between those, it
> might loop forever even if the top-level SELECT only wanted to see a few
> rows from it.  The docs already discourage writing recursive ctes like
> that, but still this is a small caveat.

Doesn't seem like a big problem to me.

>> - It seems like the gram.y changes for common_table_expr might benefit
>> from some factoring; that is, create a production (or find a suitable
>> existing one) for "statements of the sort that can appear within
>> CTEs", and then use that in common_table_expr.  Or maybe this doesn't
>> work; I haven't tried it.
>
> This seems to work.  I used PreparableStmt, but I'm not sure how good
> idea that really is.  Maybe I should create a new one?

If it covers the same territory, I wouldn't duplicate it just for fun.Someone might need to split it out in the future,
butthat's not a 
reason to do it now.

...Robert


pgsql-hackers by date:

Previous
From: Greg Smith
Date:
Subject: Re: About psycopg2 (by its author)
Next
From: Tom Lane
Date:
Subject: Re: Some belated patch review for "Buffers" explain analyze patch