Re: ModifyTable overheads in generic plans

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)
> {
> ModifyTable *node = (ModifyTable *) mtstate->ps.plan;
> EState *estate = mtstate->ps.state;
>
> if (node->withCheckOptionLists == NIL)
> return;
>
> /* Initialize expression state if not already done. */
> if (resultRelInfo->ri_WithCheckOptions == NIL)
> {
> int whichrel = resultRelInfo - mtstate->resultRelInfo;
> List *wcoList;
> List *wcoExprs = NIL;
> ListCell *ll;
>
> Assert(whichrel >= 0 && whichrel < mtstate->mt_nplans);
> wcoList = (List *) list_nth(node->withCheckOptionLists, whichrel);
> foreach(ll, wcoList)
> {
> WithCheckOption *wco = (WithCheckOption *) lfirst(ll);
> ExprState *wcoExpr = ExecInitQual((List *) wco->qual,
> &mtstate->ps);
>
> wcoExprs = lappend(wcoExprs, wcoExpr);
> }
>
> resultRelInfo->ri_WithCheckOptions = wcoList;
> resultRelInfo->ri_WithCheckOptionExprs = wcoExprs;
> }
>
> /*
> * ExecWithCheckOptions() will skip any WCOs which are not of the kind
> * we are looking for at this point.
> */
> ExecWithCheckOptions(wco_kind, resultRelInfo, slot, estate);
> }

Can we do this initialization in ExecGetResultRelation()? That would
seem much more straightforward. Is there any advantage to delaying it
here? And same thing with the junk filter and the RETURNING list.

(/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.

- Heikki