Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Mon, Jun 19, 2017 at 11:06 AM, Andrew Gierth wrote: >> "Thomas" == Thomas Munro writes: > > Thomas> Thanks both for the review. New version of patch #2 attached. > > I'm looking to commit this soon; if anyone has any further

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

> "Thomas" == Thomas Munro writes: Thomas> Thanks both for the review. New version of patch #2 attached. I'm looking to commit this soon; if anyone has any further comment now would be a good time to speak up. -- Andrew (irc:RhodiumToad) -- Sent via

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Mon, Jun 12, 2017 at 9:29 AM, Andrew Gierth wrote: >> "Robert" == Robert Haas writes: > > Robert> I don't see a reason why MakeTransitionCaptureState needs to > Robert> force the tuplestores into TopTransactionContext or make them >

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

> "Robert" == Robert Haas writes: Robert> I don't see a reason why MakeTransitionCaptureState needs to Robert> force the tuplestores into TopTransactionContext or make them Robert> owned by TopTransactionResourceOwner. Nor do I, and I'm pretty sure it's leaking

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

Robert Haas wrote: > Not this patch's problem directly, but while scrutinizing this it > crossed my mind that we would need to prohibit constraint triggers > with transition tables. It turns out that we do, in the parser: > > create constraint trigger table2_trig > after insert on table2

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 3:13 PM, Robert Haas wrote: > On Tue, Jun 6, 2017 at 8:19 PM, Peter Geoghegan wrote: >> On Tue, Jun 6, 2017 at 5:01 PM, Peter Geoghegan wrote: >>> Also, ISTM that the code within ENRMetadataGetTupDesc() probably >>>

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Fri, Jun 9, 2017 at 12:00 AM, Thomas Munro wrote: > On Wed, Jun 7, 2017 at 5:36 PM, Thomas Munro > wrote: >> I spent a couple of hours drafting a proof-of-concept to see if my >> hunch was right. It seems to work correctly so far

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 5:36 PM, Thomas Munro wrote: > I spent a couple of hours drafting a proof-of-concept to see if my > hunch was right. It seems to work correctly so far and isn't huge > (but certainly needs more testing and work): > > 6 files changed, 156

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 11:50 AM, Thomas Munro wrote: > On Thu, Jun 8, 2017 at 11:05 AM, Thomas Munro > wrote: >> 1. Keep the current behaviour. [...] >> >> 2. Make a code change that would split the 'new table' tuplestore in >> two:

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 8:19 PM, Peter Geoghegan wrote: > On Tue, Jun 6, 2017 at 5:01 PM, Peter Geoghegan wrote: >> Also, ISTM that the code within ENRMetadataGetTupDesc() probably >> requires more explanation, resource management wise. Why so? I'm not saying you're

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 5:01 PM, Andrew Gierth wrote: >> "Robert" == Robert Haas writes: > Robert> unless some other committer volunteers. (Of course, anyone > Robert> could step in to do the work, as Thomas already has to a > Robert>

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

Robert Haas writes: > I don't really like either option, because, on the one hand, this is a > pretty invasive thing to go rip out -- maybe we don't have to rip out > the entire patch series, but just some of the later patches? -- and on > the other hand, keeping it in the

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Fri, Jun 9, 2017 at 8:41 AM, Andrew Gierth wrote: >> "Thomas" == Thomas Munro writes: > > Thomas> So, afterTriggers.query_stack is used to handle the reentrancy > Thomas> that results from triggers running further statements

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

> "Robert" == Robert Haas writes: Robert> unless some other committer volunteers. (Of course, anyone Robert> could step in to do the work, as Thomas already has to a Robert> considerable degree, but without a committer involved it Robert> doesn't fix the problem.)

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

> "Thomas" == Thomas Munro writes: Thomas> So, afterTriggers.query_stack is used to handle the reentrancy Thomas> that results from triggers running further statements that Thomas> might fire triggers. It isn't used for dealing with extra Thomas>

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 1:20 PM, Robert Haas wrote: > I am not sure that I entirely agree with you on these points, but I > don't want to debate it further and especially not on a public mailing > list. Fair enough. > I'm going to spend some time over the next ~24 hours

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 2:55 PM, Peter Geoghegan wrote: > It does seem like Kevin could have done better there. However, I think > that Kevin was taking responsibility for the situation, and that it > wasn't accurate to suggest he blamed others. I thought his account of > the

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 11:32 AM, Robert Haas wrote: > I understood the same. However, he didn't review or commit any of the > bug fix patches that Thomas posted in May, or even respond to the > mailing list threads. I eventually reviewed and committed them to > avoid

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 2:05 PM, Peter Geoghegan wrote: > On Thu, Jun 8, 2017 at 10:59 AM, Robert Haas wrote: >> More generally, I don't think there's ever a >> time when it's OK to commit a patch that you're not willing to put at >> least some effort into

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 11:05:43AM -0700, Peter Geoghegan wrote: > On Thu, Jun 8, 2017 at 10:59 AM, Robert Haas wrote: > > More generally, I don't think there's ever a > > time when it's OK to commit a patch that you're not willing to put at > > least some effort into

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 10:59 AM, Robert Haas wrote: > More generally, I don't think there's ever a > time when it's OK to commit a patch that you're not willing to put at > least some effort into fixing up afterwards. Kevin said "It has become clear that the scope of

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 1:48 PM, Heikki Linnakangas wrote: > On 06/08/2017 08:36 PM, Robert Haas wrote: >> >> I freely admit I encouraged you to commit this. I did not imagine >> that would be followed immediately by abdicating all responsibility >> for it. My mistake, I guess.

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On 06/08/2017 08:36 PM, Robert Haas wrote: I freely admit I encouraged you to commit this. I did not imagine that would be followed immediately by abdicating all responsibility for it. My mistake, I guess. Robert, chill out. Kevin offered to revert. It's perhaps not the best way forward -

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 5:47 PM, Kevin Grittner wrote: > It has become clear that the scope of problems being found now > exceed what I can be sure of being able to fix in time to make for a > stable release, in spite of the heroic efforts Thomas has been > putting in. I had

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 11:05 AM, Thomas Munro wrote: > 1. Keep the current behaviour. [...] > > 2. Make a code change that would split the 'new table' tuplestore in > two: an insert tuplestore and an update tuplestore (for new images; > old images could remain in

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 10:21 AM, Kevin Grittner wrote: > On Wed, Jun 7, 2017 at 5:00 PM, Peter Geoghegan wrote: > >> My assumption about how transition tables ought to behave here is >> based on the simple fact that we already fire both AFTER >> statement-level

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 3:00 PM, Peter Geoghegan wrote: > My assumption would be that since you have as many as two > statement-level triggers firing that could reference transition tables > when ON CONFLICT DO UPDATE is used (one AFTER UPDATE statement level > trigger, and another

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 5:00 PM, Peter Geoghegan wrote: > My assumption about how transition tables ought to behave here is > based on the simple fact that we already fire both AFTER > statement-level triggers, plus my sense of aesthetics, or bias. I > admit that I might be missing

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 4:48 PM, Thomas Munro wrote: > Is there anything about that semantics that is incompatible with the > incremental matview use case? Nothing incompatible at all. If we had separate "new" tables for UPDATE and DELETE we would logically need

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 2:19 PM, Kevin Grittner wrote: > The idea of transition tables is that you see all changes to the > target of a single statement in the form of delta relations -- with > and "old" table for any rows affected by a delete or update and a > "new" table for

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Thu, Jun 8, 2017 at 9:19 AM, Kevin Grittner wrote: > On Wed, Jun 7, 2017 at 3:42 AM, Thomas Munro > wrote: >> On Wed, Jun 7, 2017 at 7:27 PM, Thomas Munro >> wrote: >>> On Wed, Jun 7, 2017 at 10:47 AM, Peter

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 4:42 PM, Robert Haas wrote: > So, are you willing and able to put any effort into this, like say > reviewing the patch Thomas posted, and if so when and how much? If > you're just done and you aren't going to put any more work into > maintaining it

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 3:42 AM, Thomas Munro wrote: > On Wed, Jun 7, 2017 at 7:27 PM, Thomas Munro > wrote: >> On Wed, Jun 7, 2017 at 10:47 AM, Peter Geoghegan wrote: >>> I suppose you'll need two tuplestores for the ON

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 7:27 PM, Thomas Munro wrote: > On Wed, Jun 7, 2017 at 10:47 AM, Peter Geoghegan wrote: >> I suppose you'll need two tuplestores for the ON CONFLICT DO UPDATE >> case -- one for updated tuples, and the other for inserted tuples.

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 10:47 AM, Peter Geoghegan wrote: > On Mon, Jun 5, 2017 at 6:40 PM, Thomas Munro > wrote: >> After sleeping on it, I don't think we need to make that decision here >> though. I think it's better to just move the tuplestores into

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 12:19 PM, Peter Geoghegan wrote: > On Tue, Jun 6, 2017 at 5:01 PM, Peter Geoghegan wrote: >> Also, ISTM that the code within ENRMetadataGetTupDesc() probably >> requires more explanation, resource management wise. > > Also, it's not clear why it

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Wed, Jun 7, 2017 at 9:42 AM, Robert Haas wrote: > I think I'd like to walk back my earlier statements about reverting > this patch just a little bit. Although putting the tuplestore at the > wrong level does seem like a fairly significant design mistake, Thomas > more

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

> > I'll give it a few days for objections before reverting. >> > > I can only say that the lack of this feature comes up on a weekly basis on > IRC, and a lot of people would be disappointed to see it reverted. > Not that my opinion matters, but I was very much looking forward to this feature in

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 5:01 PM, Peter Geoghegan wrote: > Also, ISTM that the code within ENRMetadataGetTupDesc() probably > requires more explanation, resource management wise. Also, it's not clear why it should be okay that the new type of ephemeral RTEs introduced don't have

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 3:47 PM, Peter Geoghegan wrote: > I suppose you'll need two tuplestores for the ON CONFLICT DO UPDATE > case -- one for updated tuples, and the other for inserted tuples. Also, ISTM that the code within ENRMetadataGetTupDesc() probably requires more

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Mon, Jun 5, 2017 at 6:40 PM, Thomas Munro wrote: > After sleeping on it, I don't think we need to make that decision here > though. I think it's better to just move the tuplestores into > ModifyTableState so that each embedded DML statement has its own, and >

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 4:25 PM, Kevin Grittner wrote: > Nice as it would be to add a SQL standard feature and advance the > effort to get to incremental maintenance of materialized views, and > much as I really appreciate the efforts Thomas has put into trying > to solve these

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 3:41 PM, Marko Tiikkaja wrote: > On Tue, Jun 6, 2017 at 10:25 PM, Kevin Grittner wrote: >> It took years to get an in-depth review, then I was asked >> not to commit it because others were working on patches that would >> conflict. That

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 10:25 PM, Kevin Grittner wrote: > Nice as it would be to add a SQL standard feature and advance the > effort to get to incremental maintenance of materialized views, and > much as I really appreciate the efforts Thomas has put into trying > to solve

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

Nice as it would be to add a SQL standard feature and advance the effort to get to incremental maintenance of materialized views, and much as I really appreciate the efforts Thomas has put into trying to solve these problems, I agree that it is best to revert the feature. It took years to get an

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 12:58 PM, Craig Ringer wrote: > On 4 June 2017 at 06:41, Kevin Grittner wrote: >> On Fri, Jun 2, 2017 at 8:46 PM, Thomas Munro >> wrote: >> >>> So, afterTriggers.query_stack is used to handle the

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On 4 June 2017 at 06:41, Kevin Grittner wrote: > On Fri, Jun 2, 2017 at 8:46 PM, Thomas Munro > wrote: > >> So, afterTriggers.query_stack is used to handle the reentrancy that >> results from triggers running further statements that might fire >>

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 10:11 AM, Tom Lane wrote: > Thomas Munro writes: >> On Tue, Jun 6, 2017 at 2:15 AM, Tom Lane wrote: >>> FWIW, parse analysis is surely NOT the time for such a check. Triggers >>> might get added to a

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

Thomas Munro writes: > On Tue, Jun 6, 2017 at 2:15 AM, Tom Lane wrote: >> FWIW, parse analysis is surely NOT the time for such a check. Triggers >> might get added to a table between analysis and execution. I think you >> might have to do it

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Tue, Jun 6, 2017 at 2:15 AM, Tom Lane wrote: > Robert Haas writes: >> On Sat, Jun 3, 2017 at 10:39 PM, Thomas Munro >> wrote: >>> In the meantime, it seems like you agree that rejecting wCTEs that >>> affect tables

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Mon, Jun 5, 2017 at 10:15 AM, Tom Lane wrote: >> I'm starting to like the approach of reverting the entire transition >> tables patch. Failing to consider the possibility of a plan with >> multiple ModifyTable nodes seems like a pretty fundamental design >> mistake, and

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

Robert Haas writes: > On Sat, Jun 3, 2017 at 10:39 PM, Thomas Munro > wrote: >> In the meantime, it seems like you agree that rejecting wCTEs that >> affect tables with triggers with transition tables is the best >> response to this bug

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Sat, Jun 3, 2017 at 10:39 PM, Thomas Munro wrote: > In the meantime, it seems like you agree that rejecting wCTEs that > affect tables with triggers with transition tables is the best > response to this bug report? Do you think that parse analysis is the > right

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Sun, Jun 4, 2017 at 10:41 AM, Kevin Grittner wrote: > On Fri, Jun 2, 2017 at 8:46 PM, Thomas Munro > wrote: > >> So, afterTriggers.query_stack is used to handle the reentrancy that >> results from triggers running further statements that might

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Fri, Jun 2, 2017 at 8:46 PM, Thomas Munro wrote: > So, afterTriggers.query_stack is used to handle the reentrancy that > results from triggers running further statements that might fire > triggers. It isn't used for dealing with extra ModifyTable nodes that >

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Sat, Jun 3, 2017 at 1:20 AM, Thomas Munro wrote: > On Sat, Jun 3, 2017 at 12:10 AM, Thomas Munro > wrote: >> I would have expected that to be handled by separate transition tables >> at different query levels, but evidently it

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Sat, Jun 3, 2017 at 12:10 AM, Thomas Munro wrote: > I would have expected that to be handled by separate transition tables > at different query levels, but evidently it isn't. I am wondering if we need to wrap the execution of a modifying CTE in

Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Fri, Jun 2, 2017 at 10:48 PM, Marko Tiikkaja wrote: > Since the subject of transition tables came up, I thought I'd test how this > case works: > > =# create table qwr(a int); > CREATE TABLE > > =# create function qq() returns trigger as $$ begin raise notice '%', > (select

[HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

Since the subject of transition tables came up, I thought I'd test how this case works: =# create table qwr(a int); CREATE TABLE =# create function qq() returns trigger as $$ begin raise notice '%', (select count(*) from oo); return null; end $$ language plpgsql; CREATE FUNCTION =# create