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 comment now
> would be a good time to speak up.
Here's patch #
> "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 pgsql-hackers mailing list (pgsql-h
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
> Robert> owned by TopTransactionResourceOwner.
>
> Nor
> "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 memory wholesale within a
t
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 ref
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
>>> requires more explanation, resource management wise.
>
>
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 and isn't huge
>> (but certainly needs more testing and work):
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 insertions(+), 109 deletions(-)
[A
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: an insert tuplestore and an update tuplestore (for new images;
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 wrong, but I don't see what's
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> considerable degree, but without a committer involved i
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 tree will require work,
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 that
> Thomas> might fire triggers. It isn't used for dealing
> "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.)
I can probably take th
> "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> ModifyTable nodes that can appear in a pl
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 studying
> this thread and
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 constraints that he op
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 having the feature reverted;
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 fixing up afterwards.
>
> Kevin said "It
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 fixing up afterwards.
>
> Ke
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 problems being found
now exce
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.
>
> Robert, chill
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 - I'
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 hoped to get this into
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 the old tuplestore that is also
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 triggers, plus my sense of aestheti
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 AFTER INSERT sta
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 the point, but
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 to do a "counting"-style
UNION of
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 any rows affected by
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 Geoghegan wrote:
I suppose you'll need two tuplestores for the ON CONFLICT DO U
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 (for whatever reasons),
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 CONFLICT DO UPDATE
>>> case -- one for updated tuples, and the other for ins
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.
>
> Hmm. Right. INSERT ... ON CONFLICT DO UP
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
>> ModifyTableState so that each embedded DML
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 should be okay that the new
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 or less convinced me yest
>
> 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
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 permissions checks.
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 explanation, resource ma
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
> have ModifyTable pass them to the
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 problems, I agree th
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 just doesn't leave enough time to a
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 these problems, I agree
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 i
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 reentrancy that
>>> results from triggers running further statements that mig
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
>> triggers. It isn't used for dealing with extra Mo
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 table between analysis and execution. I think you
>>> might have to do
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 during executor startup.
> Hmm. My understanding:
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 with triggers with transition tables is the best
>>> response to this bug repo
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 I'm not eager either to
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 report? Do you think that parse analysis is the
>> right ti
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 time to do the check? Here's a
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 fire
>> triggers. It isn't used for dealing with
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
> can appear in a plan because of w
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 isn't.
>
> I am wondering if we need to wrap the execution of a mod
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
AfterTriggerBeginQuery() and AfterTriggerEndQuer
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 count(*) from oo); r
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 trigge
59 matches
Mail list logo