Re: Logical Replication and triggers

2017-11-21 Thread Craig Ringer 
On 22 November 2017 at 05:35, Robert Haas  wrote:

> On Tue, Nov 21, 2017 at 4:28 PM, Simon Riggs 
> wrote:
> > I would have acted on this myself a few days back if I thought the
> > patch was correct, but I see multiple command counter increments
> > there, so suspect an alternate fix is correct.
>
> Oh, well, I'm glad you said something.  I was actually thinking about
> committing the patch Peter posted in
> http://postgr.es/m/619c557d-93e6-1833-1692-b010b176f...@2ndquadrant.com
> because it looks correct to me, but I'm certainly not an expert on
> that code so I'll wait for your analysis.
>

I'll read over the patch and respond on that thread.

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

Re: Logical Replication and triggers

2017-11-21 Thread Craig Ringer 
On 22 November 2017 at 02:27, Robert Haas  wrote:

> On Sat, Nov 18, 2017 at 7:30 PM, Craig Ringer 
> wrote:
> > On 15 November 2017 at 21:12, Thomas Rosenstein
> >  wrote:
> >> I would like somebody to consider Petr Jelineks patch for worker.c from
> >> here
> >> (https://www.postgresql.org/message-id/619c557d-93e6-1833-
> 1692-b010b176ff77%402ndquadrant.com)
> >>
> >> I'm was facing the same issue with 10.1 and BEFORE INSERT OR UPDATE
> >> triggers.
> >
> > Please:
> >
> > - Apply it to current HEAD
> > - Test its functionality
> > - Report back on the patch thread
> > - Update the commitfest app with your results and sign on as a reviewer
> > - If you're able, read over the patch and make any comments you can
> >
> > "Somebody" needs to be you, if you want this functionality.
>
> You realize we're talking about a bug fix, right?  And for a feature
> that was developed and committed by your colleagues?
>

I did not realise it was a bug fix, and agree that changes things.

There was discussion at a similar time around people wanting extra features
for triggers and incorrectly assumed this was regarding that post.

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

Re: Logical Replication and triggers

2017-11-21 Thread Robert Haas 
On Tue, Nov 21, 2017 at 4:28 PM, Simon Riggs  wrote:
> I would have acted on this myself a few days back if I thought the
> patch was correct, but I see multiple command counter increments
> there, so suspect an alternate fix is correct.

Oh, well, I'm glad you said something.  I was actually thinking about
committing the patch Peter posted in
http://postgr.es/m/619c557d-93e6-1833-1692-b010b176f...@2ndquadrant.com
because it looks correct to me, but I'm certainly not an expert on
that code so I'll wait for your analysis.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Logical Replication and triggers

2017-11-21 Thread Simon Riggs 
On 21 November 2017 at 16:13, Thomas Rosenstein
 wrote:

> To weigh in here, I actually find it's a big hurdle
>
> I'm a postgres user and not a postgres dev, so I definitly have the feeling
> I'm not qualified to answer if this really does what it's intended todo.
> Further more not beeing in the processes it will take me probably 2 - 3
> hours (if I have time) to figure out everything I should do and how I should
> do it,
> somebody doing this regularly might take 5 minutes.
>
> Yes it fixes my replication issues, yes it seems to work on the first look,
> but what does it really do - no idea!

Fair enough, but that's not what Craig asked is it?

Seems like he gave a helpful, long explanation of how you can help
expedite the process of fixing the bug, to which he received no reply.

We're trying to ensure that bugs are fixed, but also take care not to
introduce further bugs or false fixes that don't move us forwards.

I would have acted on this myself a few days back if I thought the
patch was correct, but I see multiple command counter increments
there, so suspect an alternate fix is correct.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Logical Replication and triggers

2017-11-21 Thread Thomas Rosenstein 
On Tue, Nov 21, 2017 at 3:29 PM, Simon Riggs  
wrote:

You realize we're talking about a bug fix, right?  And for a feature
that was developed and committed by your colleagues?


Craig is asking Thomas to confirm the proposed bug fix works. How is
this not normal?


That's not exactly how I read Craig's email, but it's of course
difficult to know what tone somebody intended from an email.  Suffice
it to say that I think "please pay attention to this proposed bug-fix
patch" is a pretty legitimate request.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


To weigh in here, I actually find it's a big hurdle

I'm a postgres user and not a postgres dev, so I definitly have the 
feeling I'm not qualified to answer if this really does what it's 
intended todo.
Further more not beeing in the processes it will take me probably 2 - 3 
hours (if I have time) to figure out everything I should do and how I 
should do it,

somebody doing this regularly might take 5 minutes.

Yes it fixes my replication issues, yes it seems to work on the first 
look, but what does it really do - no idea!

Re: Logical Replication and triggers

2017-11-21 Thread Robert Haas 
On Tue, Nov 21, 2017 at 3:29 PM, Simon Riggs  wrote:
>> You realize we're talking about a bug fix, right?  And for a feature
>> that was developed and committed by your colleagues?
>
> Craig is asking Thomas to confirm the proposed bug fix works. How is
> this not normal?

That's not exactly how I read Craig's email, but it's of course
difficult to know what tone somebody intended from an email.  Suffice
it to say that I think "please pay attention to this proposed bug-fix
patch" is a pretty legitimate request.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Logical Replication and triggers

2017-11-21 Thread Simon Riggs 
On 21 November 2017 at 13:27, Robert Haas  wrote:
> On Sat, Nov 18, 2017 at 7:30 PM, Craig Ringer  wrote:
>> On 15 November 2017 at 21:12, Thomas Rosenstein
>>  wrote:
>>> I would like somebody to consider Petr Jelineks patch for worker.c from
>>> here
>>> (https://www.postgresql.org/message-id/619c557d-93e6-1833-1692-b010b176ff77%402ndquadrant.com)
>>>
>>> I'm was facing the same issue with 10.1 and BEFORE INSERT OR UPDATE
>>> triggers.
>>
>> Please:
>>
>> - Apply it to current HEAD
>> - Test its functionality
>> - Report back on the patch thread
>> - Update the commitfest app with your results and sign on as a reviewer
>> - If you're able, read over the patch and make any comments you can
>>
>> "Somebody" needs to be you, if you want this functionality.
>
> You realize we're talking about a bug fix, right?  And for a feature
> that was developed and committed by your colleagues?

Craig is asking Thomas to confirm the proposed bug fix works. How is
this not normal?

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Logical Replication and triggers

2017-11-18 Thread Craig Ringer 
On 15 November 2017 at 21:12, Thomas Rosenstein <
thomas.rosenst...@creamfinance.com> wrote:

> I would like somebody to consider Petr Jelineks patch for worker.c from
> here (https://www.postgresql.org/message-id/619c557d-93e6-1833-16
> 92-b010b176ff77%402ndquadrant.com)
>
> I'm was facing the same issue with 10.1 and BEFORE INSERT OR UPDATE
> triggers.
>

Please:

- Apply it to current HEAD
- Test its functionality
- Report back on the patch thread
- Update the commitfest app with your results and sign on as a reviewer
- If you're able, read over the patch and make any comments you can

"Somebody" needs to be you, if you want this functionality.


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

Logical Replication and triggers

2017-11-15 Thread Thomas Rosenstein 
I would like somebody to consider Petr Jelineks patch for worker.c from 
here 
(https://www.postgresql.org/message-id/619c557d-93e6-1833-1692-b010b176ff77%402ndquadrant.com)


I'm was facing the same issue with 10.1 and BEFORE INSERT OR UPDATE 
triggers.


BR
Thomas Rosenstein