Re: Concurrency bug in UPDATE of partition-key

On Thu, Jul 12, 2018 at 10:14 PM, Andres Freund wrote: > On 2018-07-11 09:16:33 -0400, Alvaro Herrera wrote: >> On 2018-Jul-11, Amit Kapila wrote: >> >> > Attached, please find an updated patch based on comments by Alvaro. >> > See, if this looks okay to you guys. >> >> LGTM as far as my previous

Re: Concurrency bug in UPDATE of partition-key

On 2018-07-11 09:16:33 -0400, Alvaro Herrera wrote: > On 2018-Jul-11, Amit Kapila wrote: > > > Attached, please find an updated patch based on comments by Alvaro. > > See, if this looks okay to you guys. > > LGTM as far as my previous comments are concerned. I see Amit pushed a patch here

Re: Concurrency bug in UPDATE of partition-key

On 2018-Jul-11, Amit Kapila wrote: > Attached, please find an updated patch based on comments by Alvaro. > See, if this looks okay to you guys. LGTM as far as my previous comments are concerned. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support,

Re: Concurrency bug in UPDATE of partition-key

On 11 July 2018 at 09:48, Amit Kapila wrote: > On Wed, Jul 11, 2018 at 8:56 AM, Amit Kapila wrote: >> On Wed, Jul 11, 2018 at 5:59 AM, Alvaro Herrera >> >> >>> Please move the output arguments at the end of argument lists; >> >> make sense. >> >>> also, it >>> would be great if you add

Re: Concurrency bug in UPDATE of partition-key

On Wed, Jul 11, 2018 at 8:56 AM, Amit Kapila wrote: > On Wed, Jul 11, 2018 at 5:59 AM, Alvaro Herrera > > >> Please move the output arguments at the end of argument lists; > > make sense. > >> also, it >> would be great if you add commentary about ExecDelete other undocumented >> arguments

Re: Concurrency bug in UPDATE of partition-key

On Wed, Jul 11, 2018 at 5:59 AM, Alvaro Herrera wrote: > On 2018-Jul-09, Amit Kapila wrote: > >> Alvaro, >> >> Can you please comment whether this addresses your concern? > > I was thinking that it would be a matter of passing the tuple slot to > EvalPlanQual for it to fill, rather than requiring

Re: Concurrency bug in UPDATE of partition-key

On 2018-Jul-09, Amit Kapila wrote: > Alvaro, > > Can you please comment whether this addresses your concern? I was thinking that it would be a matter of passing the tuple slot to EvalPlanQual for it to fill, rather than requiring it to fill some other slot obtained from who-knows-where, but I

Re: Concurrency bug in UPDATE of partition-key

On Mon, Jul 2, 2018 at 5:06 PM, Amit Khandekar wrote: > On 30 June 2018 at 19:20, Amit Kapila wrote: >> On Fri, Jun 29, 2018 at 11:22 PM, Alvaro Herrera >> wrote: >>> I was a bit surprised by the new epqslot output argument being added, >>> and now I think I know why: we already have

Re: Concurrency bug in UPDATE of partition-key

On 30 June 2018 at 19:20, Amit Kapila wrote: > On Fri, Jun 29, 2018 at 11:22 PM, Alvaro Herrera > wrote: >> I was a bit surprised by the new epqslot output argument being added, >> and now I think I know why: we already have es_trig_tuple_slot, so >> shouldn't we be using that here instead?

Re: Concurrency bug in UPDATE of partition-key

On Fri, Jun 29, 2018 at 11:22 PM, Alvaro Herrera wrote: > I was a bit surprised by the new epqslot output argument being added, > and now I think I know why: we already have es_trig_tuple_slot, so > shouldn't we be using that here instead? Seems like it'd all be simpler ... > Hmm, maybe, but

Re: Concurrency bug in UPDATE of partition-key

I was a bit surprised by the new epqslot output argument being added, and now I think I know why: we already have es_trig_tuple_slot, so shouldn't we be using that here instead? Seems like it'd all be simpler ... -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL

Re: Concurrency bug in UPDATE of partition-key

On 26 June 2018 at 17:53, Amit Kapila wrote: > Yeah, so let's leave it as it is in the patch. Ok. > I think now we should wait and see what Alvaro has to say about the overall > patch. Yeah, that's very good that Alvaro is also having a look at this. -- Thanks, -Amit Khandekar EnterpriseDB

Re: Concurrency bug in UPDATE of partition-key

On Tue, Jun 26, 2018 at 11:40 AM, Amit Khandekar wrote: > On 25 June 2018 at 17:20, Amit Kapila wrote: >> On Mon, Jun 25, 2018 at 3:06 PM, Amit Khandekar >> wrote: >>> On 23 June 2018 at 15:46, Amit Kapila wrote: > Why do you need to update when newslot is NULL? Already

Re: Concurrency bug in UPDATE of partition-key

On 25 June 2018 at 17:20, Amit Kapila wrote: > On Mon, Jun 25, 2018 at 3:06 PM, Amit Khandekar > wrote: >> On 23 June 2018 at 15:46, Amit Kapila wrote: >>> >>> Why do you need to update when newslot is NULL? Already *epqslot is >>> initialized as NULL in the caller (ExecUpdate). I think

Re: Concurrency bug in UPDATE of partition-key

On Mon, Jun 25, 2018 at 3:06 PM, Amit Khandekar wrote: > On 23 June 2018 at 15:46, Amit Kapila wrote: >>> >> >> Why do you need to update when newslot is NULL? Already *epqslot is >> initialized as NULL in the caller (ExecUpdate). I think we only want >> to update it when trigtuple is not null.

Re: Concurrency bug in UPDATE of partition-key

On Sat, Jun 23, 2018 at 8:25 PM, Alvaro Herrera wrote: > Would you wait a little bit before pushing this? > Sure. > I'd like to give this > a read too. > That would be great. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: Concurrency bug in UPDATE of partition-key

Would you wait a little bit before pushing this? I'd like to give this a read too. Thanks -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Concurrency bug in UPDATE of partition-key

On Fri, Jun 22, 2018 at 10:25 PM, Amit Khandekar wrote: > On 20 June 2018 at 18:54, Amit Kapila wrote: >> On Tue, Jun 19, 2018 at 9:33 PM, Amit Khandekar >> wrote: >> >> 2. >> ExecBRDeleteTriggers(..) >> { >> .. >> + /* If requested, pass back the concurrently updated tuple, if any. */ >> + if

Re: Concurrency bug in UPDATE of partition-key

On 20 June 2018 at 18:54, Amit Kapila wrote: > On Tue, Jun 19, 2018 at 9:33 PM, Amit Khandekar > wrote: >> >> Could not add RAISE statement, because the isolation test does not >> seem to print those statements in the output. Instead, I have dumped >> some rows in a new table through the

Re: Concurrency bug in UPDATE of partition-key

On Tue, Jun 19, 2018 at 9:33 PM, Amit Khandekar wrote: > > Could not add RAISE statement, because the isolation test does not > seem to print those statements in the output. Instead, I have dumped > some rows in a new table through the trigger function, and did a > select from that table.

Re: Concurrency bug in UPDATE of partition-key

On 19 June 2018 at 13:06, Dilip Kumar wrote: > On Mon, Jun 18, 2018 at 6:19 PM, Amit Khandekar > wrote: >> On 18 June 2018 at 17:56, Amit Kapila wrote: >>> On Mon, Jun 18, 2018 at 11:28 AM, Dilip Kumar wrote: Should we also create a test case where we can verify that some

Re: Concurrency bug in UPDATE of partition-key

On Mon, Jun 18, 2018 at 6:19 PM, Amit Khandekar wrote: > On 18 June 2018 at 17:56, Amit Kapila wrote: >> On Mon, Jun 18, 2018 at 11:28 AM, Dilip Kumar wrote: >>> Should we also create a test case where we can verify that some >>> unnecessary or duplicate triggers are not executed? >>> >> >> I

Re: Concurrency bug in UPDATE of partition-key

On 18 June 2018 at 17:56, Amit Kapila wrote: > On Mon, Jun 18, 2018 at 11:28 AM, Dilip Kumar wrote: >> Should we also create a test case where we can verify that some >> unnecessary or duplicate triggers are not executed? >> > > I am not sure how much value we will add by having such a test. In

Re: Concurrency bug in UPDATE of partition-key

On Mon, Jun 18, 2018 at 11:28 AM, Dilip Kumar wrote: > On Mon, Jun 18, 2018 at 10:21 AM, Amit Khandekar > wrote: >> Attached is v2 version of the patch. It contains the above >> trigger-related issue fixed. >> >> The updated tuple is passed back using the existing newslot parameter >> of

Re: Concurrency bug in UPDATE of partition-key

On Mon, Jun 18, 2018 at 10:21 AM, Amit Khandekar wrote: > Attached is v2 version of the patch. It contains the above > trigger-related issue fixed. > > The updated tuple is passed back using the existing newslot parameter > of GetTupleForTrigger(). When ExecBRDeleteTriggers() is called using a >

Re: Concurrency bug in UPDATE of partition-key

On 11 June 2018 at 15:29, Amit Kapila wrote: > On Mon, Jun 11, 2018 at 3:02 PM, Amit Kapila wrote: >> On Thu, Jun 7, 2018 at 1:53 PM, Amit Khandekar >> wrote: >>> On 7 June 2018 at 11:44, Amit Kapila wrote: On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar wrote: I think

Re: Concurrency bug in UPDATE of partition-key

On Mon, Jun 11, 2018 at 3:02 PM, Amit Kapila wrote: > On Thu, Jun 7, 2018 at 1:53 PM, Amit Khandekar wrote: >> On 7 June 2018 at 11:44, Amit Kapila wrote: >>> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar >>> wrote: >>> >>> I think this will allow before row delete triggers to be fired more

Re: Concurrency bug in UPDATE of partition-key

On Thu, Jun 7, 2018 at 1:53 PM, Amit Khandekar wrote: > On 7 June 2018 at 11:44, Amit Kapila wrote: >> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar >> wrote: >> >> I think this will allow before row delete triggers to be fired more than >> once. Normally, if the EvalPlanQual testing

Re: Concurrency bug in UPDATE of partition-key

On Mon, Jun 11, 2018 at 10:19 AM, Amit Khandekar wrote: > On 8 June 2018 at 16:44, Dilip Kumar wrote: > > On Fri, Jun 8, 2018 at 2:23 PM, Dilip Kumar > wrote: > >> > >> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar > >> wrote: > >>> > >>> Attached is a rebased patch version. Also included it

Re: Concurrency bug in UPDATE of partition-key

On 8 June 2018 at 16:44, Dilip Kumar wrote: > On Fri, Jun 8, 2018 at 2:23 PM, Dilip Kumar wrote: >> >> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar >> wrote: >>> >>> Attached is a rebased patch version. Also included it in the upcoming >>> commitfest : >>>

Re: Concurrency bug in UPDATE of partition-key

On Fri, Jun 8, 2018 at 2:23 PM, Dilip Kumar wrote: > On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar > wrote: > >> Attached is a rebased patch version. Also included it in the upcoming >> commitfest : >> https://commitfest.postgresql.org/18/1660/ >> >> In the rebased version, the new test cases

Re: Concurrency bug in UPDATE of partition-key

On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar wrote: > Attached is a rebased patch version. Also included it in the upcoming > commitfest : > https://commitfest.postgresql.org/18/1660/ > > In the rebased version, the new test cases are added in the existing >

Re: Concurrency bug in UPDATE of partition-key

On 7 June 2018 at 11:44, Amit Kapila wrote: > On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar > wrote: >> >> Attached is a rebased patch version. Also included it in the upcoming >> commitfest : >> https://commitfest.postgresql.org/18/1660/ >> > > Doesn't this belong to PostgreSQL 11 Open Items

Re: Concurrency bug in UPDATE of partition-key

Attached is a rebased patch version. Also included it in the upcoming commitfest : https://commitfest.postgresql.org/18/1660/ In the rebased version, the new test cases are added in the existing isolation/specs/partition-key-update-1.spec test. -- Thanks, -Amit Khandekar EnterpriseDB

Re: Concurrency bug in UPDATE of partition-key

On 8 March 2018 at 16:55, Amit Khandekar wrote: > The attached WIP patch is how the fix is turning out to be. > ExecDelete() passes back the epqslot returned by EvalPlanQual(). > ExecUpdate() uses this re-fetched tuple to re-process the row, just > like it does when

Re: Concurrency bug in UPDATE of partition-key

On Thu, Mar 8, 2018 at 4:55 PM, Amit Khandekar wrote: > (Mail subject changed; original thread : [1]) > > On 8 March 2018 at 11:57, Amit Khandekar wrote: >>> Clearly, ExecUpdate() while moving rows between partitions is missing out on >>>