Re: making update/delete of inheritance trees scale better

On Thu, May 14, 2020 at 7:55 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> On Wed, 13 May 2020 at 19:02, Amit Langote <amitlangote09(at)gmail(dot)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(at)gmail(dot)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?

Oh, that example is showing run-time pruning for a generic plan. If
planner prunes partitions, of course, their result relation indexes
are not present in ModifyTablePath.resultRelations.

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

Yep, I think it might be worthwhile to delay ResultRelInfo building
for UPDATE/DELETE too. I would like to leave that for another patch
though.

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