Re: ModifyTable overheads in generic plans

Amit Langote <amitlangote09@gmail.com>

From: Amit Langote <amitlangote09@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Robert Haas <robertmhaas@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, David Rowley <dgrowleyml@gmail.com>, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>
Date: 2020-11-03T08:27:52Z
Lists: pgsql-hackers

Attachments

On Mon, Nov 2, 2020 at 10:53 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Mon, Nov 2, 2020 at 10:19 PM Heikki Linnakangas <hlinnaka@iki.fi> 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
> > >     individual fields of ResultRelInfo such that even for the result
> > >     relations that are initialized, those fields are only initialized on
> > >     first access.  While no performance improvement is to be expected
> > >     there, it can lead to a simpler initialization logic of the
> > >     ResultRelInfo itself, because the conditions for whether a given
> > >     field is needed or not tends to look confusing.  One side-effect
> > >     of this is that any "SubPlans" referenced in the expressions of
> > >     those fields are also lazily initialized and hence changes the
> > >     output of EXPLAIN (without ANALYZE) in some regression tests.
> >
> >
> > I'm now curious what the initialization logic would look like, if we
> > initialized those fields in ExecGetResultRelation(). At a quick glance
> > on the conditions on when those initializations are done in the patch
> > now, it would seem pretty straightforward. If the target list contains
> > any junk columns, initialize junk filter, and if
> > ModifyTable->returningLists is set, initialize RETURNING list. Maybe I'm
> > missing something.
>
> Yeah, it's not that complicated to initialize those things in
> ExecGetResultRelation().  In fact, ExecGetResultRelation() (or its
> subroutine ExecBuildResultRelation()) housed those initializations in
> the earlier versions of this patch, but I changed that after our
> discussion about being lazy about initializing as much stuff as we
> can.  Maybe I should revert that?

Please check the attached if that looks better.

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

Commits

  1. Comment cleanup for a1115fa07.

  2. Postpone some more stuff out of ExecInitModifyTable.

  3. Postpone some stuff out of ExecInitModifyTable.