Re: ModifyTable overheads in generic plans

2021-04-13 Thread Amit Langote
On Wed, Apr 7, 2021 at 5:18 PM Amit Langote wrote: > On Wed, Apr 7, 2021 at 8:24 AM Tom Lane wrote: > > I also could not get excited about postponing initialization of RETURNING > > or WITH CHECK OPTIONS expressions. I grant that that can be helpful > > when those features are used, but I doubt

Re: ModifyTable overheads in generic plans

2021-04-07 Thread Amit Langote
On Thu, Apr 8, 2021 at 1:54 PM David Rowley wrote: > On Thu, 8 Apr 2021 at 15:32, Amit Langote wrote: > > There's 10-20% improvement in this case too for various partition > > counts, which really has more to do with 86dc90056 than the work done > > here. > > I'm not sure of the exact query

Re: ModifyTable overheads in generic plans

2021-04-07 Thread David Rowley
On Thu, 8 Apr 2021 at 15:32, Amit Langote wrote: > There's 10-20% improvement in this case too for various partition > counts, which really has more to do with 86dc90056 than the work done > here. I'm not sure of the exact query you're running, but I imagine the reason that it wasn't that slow

Re: ModifyTable overheads in generic plans

2021-04-07 Thread Amit Langote
On Thu, Apr 8, 2021 at 3:02 AM Tom Lane wrote: > Robert Haas writes: > > On Wed, Apr 7, 2021 at 12:34 PM Tom Lane wrote: > >> Indeed, that's a pretty impressive comparison. > > > +1. That looks like a big improvement. > > > In a vacuum, you'd hope that partitioning a table would make things > >

Re: ModifyTable overheads in generic plans

2021-04-07 Thread Amit Langote
On Thu, Apr 8, 2021 at 1:34 AM Tom Lane wrote: > Amit Langote writes: > > Also, I think we should update the commentary around ri_projectNew a > > bit to make it clear that noplace beside ExecGet{Insert|Update}Tuple > > should be touching it and the associated slots. > > Hm. I pushed your

Re: ModifyTable overheads in generic plans

2021-04-07 Thread Tom Lane
Robert Haas writes: > On Wed, Apr 7, 2021 at 12:34 PM Tom Lane wrote: >> Indeed, that's a pretty impressive comparison. > +1. That looks like a big improvement. > In a vacuum, you'd hope that partitioning a table would make things > faster rather than slower, when only one partition is

Re: ModifyTable overheads in generic plans

2021-04-07 Thread Robert Haas
On Wed, Apr 7, 2021 at 12:34 PM Tom Lane wrote: > > v13.2 > > 64 323127472217 > > 128 152812691121 > > 256 709 652 491 > > 102496 78 67 > > > v14dev HEAD > > 64 14835 14360 14563 > > 128 9469

Re: ModifyTable overheads in generic plans

2021-04-07 Thread Tom Lane
Amit Langote writes: > Also, I think we should update the commentary around ri_projectNew a > bit to make it clear that noplace beside ExecGet{Insert|Update}Tuple > should be touching it and the associated slots. Hm. I pushed your comment fixes in nodeModifyTable.c, but not this change, because

Re: ModifyTable overheads in generic plans

2021-04-07 Thread Amit Langote
On Wed, Apr 7, 2021 at 8:24 AM Tom Lane wrote: > Amit Langote writes: > > On Mon, Apr 5, 2021 at 1:43 AM Tom Lane wrote: > >> OK. Do you want to pull out the bits of the patch that we can still > >> do without postponing BeginDirectModify? > > > I ended up with the attached, whereby

Re: ModifyTable overheads in generic plans

2021-04-06 Thread Andres Freund
Hi, On 2021-04-06 19:24:11 -0400, Tom Lane wrote: > I also could not get excited about postponing initialization of RETURNING > or WITH CHECK OPTIONS expressions. I grant that that can be helpful > when those features are used, but I doubt that RETURNING is used that > heavily, and WITH CHECK

Re: ModifyTable overheads in generic plans

2021-04-06 Thread Tom Lane
Amit Langote writes: > On Mon, Apr 5, 2021 at 1:43 AM Tom Lane wrote: >> OK. Do you want to pull out the bits of the patch that we can still >> do without postponing BeginDirectModify? > I ended up with the attached, whereby ExecInitResultRelation() is now > performed for all relations before

Re: ModifyTable overheads in generic plans

2021-04-05 Thread Amit Langote
On Mon, Apr 5, 2021 at 1:43 AM Tom Lane wrote: > Amit Langote writes: > > On Sun, Apr 4, 2021 at 10:20 AM Tom Lane wrote: > >> In some desultory performance testing here, it seemed like a > >> significant part of the cost is ExecOpenIndices, and I don't see > >> a reason offhand why we could

Re: ModifyTable overheads in generic plans

2021-04-04 Thread Tom Lane
Amit Langote writes: > On Sun, Apr 4, 2021 at 10:20 AM Tom Lane wrote: >> In some desultory performance testing here, it seemed like a >> significant part of the cost is ExecOpenIndices, and I don't see >> a reason offhand why we could not delay/skip that. I also concur >> with delaying

Re: ModifyTable overheads in generic plans

2021-04-04 Thread Amit Langote
On Sun, Apr 4, 2021 at 10:20 AM Tom Lane wrote: > Amit Langote writes: > > On Thu, Apr 1, 2021 at 3:12 AM Tom Lane wrote: > >> Amit Langote writes: > > [ v14-0002-Initialize-result-relation-information-lazily.patch ] > >> Needs YA rebase over 86dc90056. > > > Done. > > I spent some time

Re: ModifyTable overheads in generic plans

2021-04-03 Thread Tom Lane
Amit Langote writes: > On Thu, Apr 1, 2021 at 3:12 AM Tom Lane wrote: >> Amit Langote writes: > [ v14-0002-Initialize-result-relation-information-lazily.patch ] >> Needs YA rebase over 86dc90056. > Done. I spent some time looking this over. There are bits of it we can adopt without too much

Re: ModifyTable overheads in generic plans

2021-04-01 Thread Amit Langote
On Thu, Apr 1, 2021 at 10:12 PM Amit Langote wrote: > > On Thu, Apr 1, 2021 at 3:12 AM Tom Lane wrote: > > Amit Langote writes: > > > [ v14-0002-Initialize-result-relation-information-lazily.patch ] > > Needs YA rebase over 86dc90056. > > Done. I will post the updated results for -Mprepared

Re: ModifyTable overheads in generic plans

2021-04-01 Thread Amit Langote
On Thu, Apr 1, 2021 at 3:12 AM Tom Lane wrote: > Amit Langote writes: > > [ v14-0002-Initialize-result-relation-information-lazily.patch ] > Needs YA rebase over 86dc90056. Done. I will post the updated results for -Mprepared benchmarks I did in the other thread shortly. -- Amit Langote EDB:

Re: ModifyTable overheads in generic plans

2021-03-31 Thread Tom Lane
Amit Langote writes: > [ v14-0002-Initialize-result-relation-information-lazily.patch ] Needs YA rebase over 86dc90056. regards, tom lane

Re: ModifyTable overheads in generic plans

2021-02-09 Thread Amit Langote
On Mon, Jan 25, 2021 at 2:23 PM Amit Langote wrote: > On Tue, Dec 22, 2020 at 5:16 PM Amit Langote wrote: > > On Mon, Dec 7, 2020 at 3:53 PM Amit Langote wrote: > > > On Thu, Nov 12, 2020 at 5:04 PM Amit Langote > > > wrote: > > > > Attached new 0002 which does these adjustments. I went with

Re: ModifyTable overheads in generic plans

2021-01-24 Thread Amit Langote
On Tue, Dec 22, 2020 at 5:16 PM Amit Langote wrote: > On Mon, Dec 7, 2020 at 3:53 PM Amit Langote wrote: > > On Thu, Nov 12, 2020 at 5:04 PM Amit Langote > > wrote: > > > Attached new 0002 which does these adjustments. I went with > > > ri_RootTargetDesc to go along with ri_RelationDesc. > >

Re: ModifyTable overheads in generic plans

2020-12-22 Thread Amit Langote
On Mon, Dec 7, 2020 at 3:53 PM Amit Langote wrote: > > On Thu, Nov 12, 2020 at 5:04 PM Amit Langote wrote: > > Attached new 0002 which does these adjustments. I went with > > ri_RootTargetDesc to go along with ri_RelationDesc. > > > > Also, I have updated the original 0002 (now 0003) to make >

Re: ModifyTable overheads in generic plans

2020-12-06 Thread Amit Langote
On Thu, Nov 12, 2020 at 5:04 PM Amit Langote wrote: > Attached new 0002 which does these adjustments. I went with > ri_RootTargetDesc to go along with ri_RelationDesc. > > Also, I have updated the original 0002 (now 0003) to make > GetChildToRootMap() use ri_RootTargetDesc instead of >

Re: ModifyTable overheads in generic plans

2020-11-12 Thread Amit Langote
On Wed, Nov 11, 2020 at 10:14 PM Heikki Linnakangas wrote: > I'm still a bit confused and unhappy about the initialization of > ResultRelInfos and the various fields in them. We've made progress in > the previous patches, but it's still a bit messy. > > > /* > >* If

Re: ModifyTable overheads in generic plans

2020-11-11 Thread Heikki Linnakangas
I'm still a bit confused and unhappy about the initialization of ResultRelInfos and the various fields in them. We've made progress in the previous patches, but it's still a bit messy. /* * If transition tuples will be captured, initialize a map to convert

Re: ModifyTable overheads in generic plans

2020-11-11 Thread Amit Langote
Thanks for the review. On Wed, Nov 11, 2020 at 5:55 PM Heikki Linnakangas wrote: > On 10/11/2020 17:32, Heikki Linnakangas wrote: > > On 10/11/2020 13:12, Amit Langote wrote: > >> On second thought, it seems A would amount to merely a cosmetic > >> adjustment of the API, nothing more. B seems

Re: ModifyTable overheads in generic plans

2020-11-11 Thread Heikki Linnakangas
On 10/11/2020 17:32, Heikki Linnakangas wrote: On 10/11/2020 13:12, Amit Langote wrote: On second thought, it seems A would amount to merely a cosmetic adjustment of the API, nothing more. B seems to get the job done for me and also doesn't unnecessarily break compatibility, so I've updated

Re: ModifyTable overheads in generic plans

2020-11-10 Thread Heikki Linnakangas
On 10/11/2020 13:12, Amit Langote wrote: On Wed, Nov 4, 2020 at 11:32 AM Amit Langote wrote: On Tue, Nov 3, 2020 at 9:05 PM Heikki Linnakangas wrote: A) We could change FDW API so that BeginDirectModify takes the same arguments as BeginForeignModify(). That avoids the assumption that it's a

Re: ModifyTable overheads in generic plans

2020-11-10 Thread Amit Langote
On Wed, Nov 4, 2020 at 11:32 AM Amit Langote wrote: > On Tue, Nov 3, 2020 at 9:05 PM Heikki Linnakangas wrote: > > On 03/11/2020 10:27, Amit Langote wrote: > > > Please check the attached if that looks better. > > > > Great, thanks! Yeah, I like that much better. > > > > This makes me a bit

Re: ModifyTable overheads in generic plans

2020-11-05 Thread Amit Langote
On Wed, Nov 4, 2020 at 11:32 AM Amit Langote wrote: > On Tue, Nov 3, 2020 at 9:05 PM Heikki Linnakangas wrote: > > A) We could change FDW API so that BeginDirectModify takes the same > > arguments as BeginForeignModify(). That avoids the assumption that it's > > a ForeignScan node, because

Re: ModifyTable overheads in generic plans

2020-11-03 Thread Amit Langote
On Tue, Nov 3, 2020 at 9:05 PM Heikki Linnakangas wrote: > On 03/11/2020 10:27, Amit Langote wrote: > > Please check the attached if that looks better. > > Great, thanks! Yeah, I like that much better. > > This makes me a bit unhappy: > > > > > /* Also let FDWs init themselves for

Re: ModifyTable overheads in generic plans

2020-11-03 Thread Heikki Linnakangas
On 03/11/2020 10:27, Amit Langote wrote: Please check the attached if that looks better. Great, thanks! Yeah, I like that much better. This makes me a bit unhappy: /* Also let FDWs init themselves for foreign-table result rels */ if

Re: ModifyTable overheads in generic plans

2020-11-03 Thread Amit Langote
On Mon, Nov 2, 2020 at 10:53 PM Amit Langote wrote: > On Mon, Nov 2, 2020 at 10:19 PM Heikki Linnakangas wrote: > > (/me reads patch further) I presume that's what you referred to in the > > commit message: > > > > > Also, extend this lazy initialization approach to some of the > > >

Re: ModifyTable overheads in generic plans

2020-11-02 Thread Amit Langote
On Mon, Nov 2, 2020 at 10:19 PM Heikki Linnakangas wrote: > On 30/10/2020 08:13, Amit Langote wrote: > > /* > > * Perform WITH CHECK OPTIONS check, if any. > > */ > > static void > > ExecProcessWithCheckOptions(ModifyTableState *mtstate, ResultRelInfo > > *resultRelInfo, > >

Re: ModifyTable overheads in generic plans

2020-11-02 Thread Heikki Linnakangas
On 30/10/2020 08:13, Amit Langote wrote: /* * Perform WITH CHECK OPTIONS check, if any. */ static void ExecProcessWithCheckOptions(ModifyTableState *mtstate, ResultRelInfo *resultRelInfo, TupleTableSlot *slot, WCOKind wco_kind) {

Re: ModifyTable overheads in generic plans

2020-10-30 Thread Amit Langote
Attached updated patches based on recent the discussion at: * Re: partition routing layering in nodeModifyTable.c * https://www.postgresql.org/message-id/CA%2BHiwqHpmMjenQqNpMHrhg3DRhqqQfby2RCT1HWVwMin3_5vMA%40mail.gmail.com 0001 adjusts how ForeignScanState.resultRelInfo is initialized for use

Re: ModifyTable overheads in generic plans

2020-09-13 Thread Amit Langote
Hello, On Fri, Aug 7, 2020 at 9:26 PM Amit Langote wrote: > On Tue, Aug 4, 2020 at 3:15 PM Amit Langote wrote: > > On Sat, Aug 1, 2020 at 4:46 AM Robert Haas wrote: > > > On Fri, Jun 26, 2020 at 8:36 AM Amit Langote > > > wrote: > > > > 0001 and 0002 are preparatory patches. > > > > > > I

Re: ModifyTable overheads in generic plans

2020-08-07 Thread Amit Langote
On Tue, Aug 4, 2020 at 3:15 PM Amit Langote wrote: > On Sat, Aug 1, 2020 at 4:46 AM Robert Haas wrote: > > On Fri, Jun 26, 2020 at 8:36 AM Amit Langote > > wrote: > > > 0001 and 0002 are preparatory patches. > > > > I read through these patches a bit but it's really unclear what the > > point

Re: ModifyTable overheads in generic plans

2020-08-04 Thread Amit Langote
On Sat, Aug 1, 2020 at 4:46 AM Robert Haas wrote: > On Fri, Jun 26, 2020 at 8:36 AM Amit Langote wrote: > > 0001 and 0002 are preparatory patches. > > I read through these patches a bit but it's really unclear what the > point of them is. I think they need better commit messages, or better >

Re: ModifyTable overheads in generic plans

2020-07-31 Thread Robert Haas
On Fri, Jun 26, 2020 at 8:36 AM Amit Langote wrote: > 0001 and 0002 are preparatory patches. I read through these patches a bit but it's really unclear what the point of them is. I think they need better commit messages, or better comments, or both. -- Robert Haas EnterpriseDB:

Re: ModifyTable overheads in generic plans

2020-07-30 Thread Daniel Gustafsson
> On 1 Jul 2020, at 15:38, Amit Langote wrote: > Another thing I could do is decouple the patches to discuss here from > the patches of the other thread, which should be possible and might be > good to avoid back and forth between the two threads. It sounds like it would make it easier for

Re: ModifyTable overheads in generic plans

2020-07-01 Thread Daniel Gustafsson
> On 1 Jul 2020, at 08:30, Amit Langote wrote: > > On Fri, Jun 26, 2020 at 9:36 PM Amit Langote wrote: >> I would like to discuss a refactoring patch that builds on top of the >> patches at [1] to address $subject. > > I forgot to update a place in postgres_fdw causing one of its tests to

Re: ModifyTable overheads in generic plans

2020-07-01 Thread Amit Langote
On Fri, Jun 26, 2020 at 9:36 PM Amit Langote wrote: > I would like to discuss a refactoring patch that builds on top of the > patches at [1] to address $subject. I forgot to update a place in postgres_fdw causing one of its tests to crash. Fixed in the attached updated version. -- Amit

Re: ModifyTable overheads in generic plans

2020-06-29 Thread Amit Langote
On Mon, Jun 29, 2020 at 10:39 AM David Rowley wrote: > > On Sat, 27 Jun 2020 at 00:36, Amit Langote wrote: > > 2. ExecCheckRTPerms(): checks permissions of *all* partitions before > > executing the plan tree, but maybe it's okay to check only the ones > > that will be accessed > > I don't think

Re: ModifyTable overheads in generic plans

2020-06-28 Thread David Rowley
On Sat, 27 Jun 2020 at 00:36, Amit Langote wrote: > 2. ExecCheckRTPerms(): checks permissions of *all* partitions before > executing the plan tree, but maybe it's okay to check only the ones > that will be accessed I don't think it needs to be quite as complex as that.

Re: ModifyTable overheads in generic plans

2020-06-28 Thread Amit Langote
On Fri, Jun 26, 2020 at 9:36 PM Amit Langote wrote: > I would like to discuss a refactoring patch that builds on top of the > patches at [1] to address $subject. I've added this to the next CF: https://commitfest.postgresql.org/28/2621/ -- Amit Langote EnterpriseDB: http://www.enterprisedb.com

ModifyTable overheads in generic plans

2020-06-26 Thread Amit Langote
Hi, I would like to discuss a refactoring patch that builds on top of the patches at [1] to address $subject. To get an idea for what eliminating these overheads looks like, take a look at the following benchmarking results. Note 1: I've forced the use of generic plan by setting plan_cache_mode