On Thu, May 14, 2020 at 7:55 AM David Rowley <dgrowle...@gmail.com> wrote: > On Wed, 13 May 2020 at 19:02, Amit Langote <amitlangot...@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 <amitlangot...@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?
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
