Logical Replication and triggers

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 (/messages/by-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

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
(/messages/by-id/619c557d-93e6-1833-1692-b010b176ff77@2ndquadrant.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
(/messages/by-id/619c557d-93e6-1833-1692-b010b176ff77@2ndquadrant.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
/messages/by-id/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
/messages/by-id/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
(/messages/by-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

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
/messages/by-id/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.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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
/messages/by-id/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
/messages/by-id/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.