Re: making update/delete of inheritance trees scale better

On Tue, Jun 2, 2020 at 1:15 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> So, I think I have a patch that seems to work, but not all the way,
> more on which below.
>
> Here is the commit message in the attached patch.
>
> ===
> Subject: [PATCH] Overhaul UPDATE's targetlist processing
>
> Instead of emitting the full tuple matching the target table's tuple
> descriptor, make the plan emit only the attributes that are assigned
> values in the SET clause, plus row-identity junk attributes as before.
> This allows us to avoid making a separate plan for each target
> relation in the inheritance case, because the only reason it is so
> currently is to account for the fact that each target relations may
> have a set of attributes that is different from others. Having only
> one plan suffices, because the set of assigned attributes must be same
> in all the result relations.
>
> While the plan will now produce only the assigned attributes and
> row-identity junk attributes, other columns' values are filled by
> refetching the old tuple. To that end, there will be a targetlist for
> each target relation to compute the full tuple, that is, by combining
> the values from the plan tuple and the old tuple, but they are passed
> separately in the ModifyTable node.
>
> Implementation notes:
>
> * In the inheritance case, as the same plan produces tuples to be
> updated from multiple result relations, the tuples now need to also
> identity which table they come from, so an additional junk attribute
> "tableoid" is present in that case.
>
> * Considering that the inheritance set may contain foreign tables that
> require a different (set of) row-identity junk attribute(s), the plan
> needs to emit multiple distinct junk attributes. When transposed to a
> child scan node, this targetlist emits a non-NULL value for the junk
> attribute that's valid for the child relation and NULL for others.
>
> * Executor and FDW execution APIs can no longer assume any specific
> order in which the result relations will be processed. For each
> tuple to be updated/deleted, result relation is selected by looking it
> up in a hash table using the "tableoid" value as the key.
>
> * Since the plan does not emit values for all the attributes, FDW APIs
> may not assume that the individual column values in the TupleTableSlot
> containing the plan tuple are accessible by their attribute numbers.
>
> TODO:
>
> * Reconsider having only one plan!
> * Update FDW handler docs to reflect the API changes
> ===

I divided that into two patches:

1. Make the plan producing tuples to be updated emit only the columns
that are actually updated. postgres_fdw test fails unless you also
apply the patch I posted at [1], because there is an unrelated bug in
UPDATE tuple routing code that manifests due to some changes of this
patch.

2. Due to 1, inheritance_planner() is no longer needed, that is,
inherited update/delete can be handled by pulling the rows to
update/delete from only one plan, not one per child result relation.
This one makes that so.

There are some unsolved problems having to do with foreign tables in
both 1 and 2:

In 1, FDW update APIs still assume that the plan produces "full" tuple
for update. That needs to be fixed so that FDWs deal with getting
only the updated columns in the plan's output targetlist.

In 2, still haven't figured out a way to call PlanDirectModify() on
child foreign tables. Lacking that, inherited updates on foreign
tables are now slower, because they are not pushed down. I'd like to
figure something out to fix that situation.

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

[1] https://www.postgresql.org/message-id/CA%2BHiwqE_UK1jTSNrjb8mpTdivzd3dum6mK--xqKq0Y9VmfwWQA%40mail.gmail.com