Re: partition routing layering in nodeModifyTable.c

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: partition routing layering in nodeModifyTable.c
Date: 2020-10-22 13:49:14
Message-ID: CA+HiwqHwtTQ_pSnxTErdSV11yxeZ8O2akkAtYnqST=E3McmdnQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Oct 20, 2020 at 9:57 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Mon, Oct 19, 2020 at 8:55 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> > It's probably true that there's no performance gain from initializing
> > them more lazily. But the reasoning and logic around the initialization
> > is complicated. After tracing through various path through the code, I'm
> > convinced enough that it's correct, or at least these patches didn't
> > break it, but I still think some sort of lazy initialization on first
> > use would make it more readable. Or perhaps there's some other
> > refactoring we could do.
>
> So the other patch I have mentioned is about lazy initialization of
> the ResultRelInfo itself, not the individual fields, but maybe with
> enough refactoring we can get the latter too.

So, I tried implementing a lazy-initialization-on-first-access
approach for both the ResultRelInfos themselves and some of the
individual fields of ResultRelInfo that don't need to be set right
away. You can see the end result in the attached 0003 patch. This
slims down ExecInitModifyTable() significantly, both in terms of code
footprint and the amount of work that it does.

0001 fixes a thinko of the recent commit 1375422c782 that I discovered
when debugging a problem with 0003.

0002 is for something I have mentioned upthread.
ForeignScanState.resultRelInfo cannot be set in ExecInit* stage as
it's done now, because with 0003, child ResultRelInfos will not have
been added to es_result_relations during that stage.

--
Amit Langote
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
v1-0002-Initialize-ForeignScanState.resultRelInfo-in-Exec.patch application/octet-stream 2.6 KB
v1-0001-Fix-a-thinko-of-1375422c782.patch application/octet-stream 1.0 KB
v1-0003-Initialize-result-relation-information-lazily.patch application/octet-stream 63.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2020-10-22 14:06:48 Re: new heapcheck contrib module
Previous Message Jürgen Purtz 2020-10-22 13:32:00 Re: Change JOIN tutorial to focus more on explicit joins