Re: making update/delete of inheritance trees scale better

On Wed, 13 May 2020 at 19:02, Amit Langote <amitlangote09@gmail.com> wrote:
> > As for which ResultRelInfos to initialize, couldn't we just have the
> > planner generate an OidList of all the ones that we could need.
> > Basically, all the non-pruned partitions.
>
> Why would replacing list of RT indexes by OIDs be better?

TBH, I didn't refresh my memory of the code before saying that.
However, if we have a list of RT index for which rangetable entries we
must build ResultRelInfos for, then why is it a problem that plan-time
pruning is not allowing you to eliminate the excess ResultRelInfos,
like you mentioned in:

On Sat, 9 May 2020 at 01:33, Amit Langote <amitlangote09@gmail.com> wrote:
> prepare q as update foo set a = 250001 where a = $1;
> set plan_cache_mode to 'force_generic_plan';
> explain execute q(1);
>                              QUERY PLAN
> --------------------------------------------------------------------
>  Update on foo  (cost=0.00..142.20 rows=40 width=14)
>    Update on foo_1
>    Update on foo_2 foo
>    Update on foo_3 foo
>    Update on foo_4 foo
>    ->  Append  (cost=0.00..142.20 rows=40 width=14)
>          Subplans Removed: 3
>          ->  Seq Scan on foo_1  (cost=0.00..35.50 rows=10 width=14)
>                Filter: (a = $1)
> (9 rows)

Shouldn't you just be setting the ModifyTablePath.resultRelations to
the non-pruned RT indexes?

> > Perhaps we could even be
> > pretty lazy about building those ResultRelInfos during execution too.
> > We'd need to grab the locks first, but, without staring at the code, I
> > doubt there's a reason we'd need to build them all upfront.  That
> > would help in cases where pruning didn't prune much, but due to
> > something else in the WHERE clause, the results only come from some
>
> Late ResultRelInfo initialization is worth considering, given that
> doing it for tuple-routing target relations works.  I don't know why
> we are still Initializing them all in InitPlan(), because the only
> justification given for doing so that I know of is that it prevents
> lock-upgrade.  I think we discussed somewhat recently that that is not
> really a hazard.

Looking more closely at ExecGetRangeTableRelation(), we'll already
have the lock by that time, there's an Assert to verify that too.
It'll have been acquired either during planning or during
AcquireExecutorLocks(). So it seems doing anything for delaying the
building of ResultRelInfos wouldn't need to account for taking the
lock at a different time.

David