Re: Logical Replication and triggers
On 22 November 2017 at 08: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. OK, I've re-reviewed it and found it good, so have committed it. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services Thanks everyone for getting this done.
Re: Logical Replication and triggers
On 22 November 2017 at 08: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. OK, I've re-reviewed it and found it good, so have committed it. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Logical Replication and triggers
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
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
On Wed, Nov 22, 2017 at 6:35 AM, 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. 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
Re: Logical Replication and triggers
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
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
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
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
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
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? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Logical Replication and triggers
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