Thread: Logical Replication and triggers
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
On 15 November 2017 at 21:12, Thomas Rosenstein <thomas.rosenstein@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.
On Sat, Nov 18, 2017 at 7:30 PM, Craig Ringer <craig@2ndquadrant.com> wrote: > On 15 November 2017 at 21:12, Thomas Rosenstein > <thomas.rosenstein@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-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? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 21 November 2017 at 13:27, Robert Haas <robertmhaas@gmail.com> wrote: > On Sat, Nov 18, 2017 at 7:30 PM, Craig Ringer <craig@2ndquadrant.com> wrote: >> On 15 November 2017 at 21:12, Thomas Rosenstein >> <thomas.rosenstein@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-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 Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Nov 21, 2017 at 3:29 PM, Simon Riggs <simon@2ndquadrant.com> 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
> On Tue, Nov 21, 2017 at 3:29 PM, Simon Riggs <simon@2ndquadrant.com> > 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!
On 21 November 2017 at 16:13, Thomas Rosenstein <thomas.rosenstein@creamfinance.com> 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 Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Nov 21, 2017 at 4:28 PM, Simon Riggs <simon@2ndquadrant.com> 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-b010b176ff77@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
On Wed, Nov 22, 2017 at 6:35 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Nov 21, 2017 at 4:28 PM, Simon Riggs <simon@2ndquadrant.com> 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-b010b176ff77@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. There is still some time until the next round of releases. I am seeing no tracking of this patch in the CF app. It would be good to not lose track of it.. I'll add an entry and reply on the original thread. -- Michael
On 22 November 2017 at 02:27, Robert Haas <robertmhaas@gmail.com> wrote:
On Sat, Nov 18, 2017 at 7:30 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 15 November 2017 at 21:12, Thomas Rosenstein
> <thomas.rosenstein@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- 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.
On 22 November 2017 at 05:35, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Nov 21, 2017 at 4:28 PM, Simon Riggs <simon@2ndquadrant.com> 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-b010b176ff77@ 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.
On 22 November 2017 at 08:35, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Nov 21, 2017 at 4:28 PM, Simon Riggs <simon@2ndquadrant.com> 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-b010b176ff77@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. OK, I've re-reviewed it and found it good, so have committed it. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> On 22 November 2017 at 08:35, Robert Haas <robertmhaas@gmail.com> > wrote: >> On Tue, Nov 21, 2017 at 4:28 PM, Simon Riggs <simon@2ndquadrant.com> >> 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-b010b176ff77@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. > > OK, I've re-reviewed it and found it good, so have committed it. > > -- > Simon Riggs http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services Thanks everyone for getting this done.