Horton

Re: generic plans and "initial" pruning

Amit Langote <amitlangote09@gmail.com>

View thread View on postgr.es
From: Amit Langote <amitlangote09@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, Daniel Gustafsson <daniel@yesql.se>, David Rowley <dgrowleyml@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Thom Brown <thom@linux.com>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2024-09-19T08:39:51Z
Lists: pgsql-hackers

Attachments

On Tue, Sep 17, 2024 at 9:57 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Thu, Aug 29, 2024 at 10:34 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > One idea that I think might be worth trying to reduce the footprint of
> > 0003 is to try to lock the prunable relations in a step of InitPlan()
> > separate from ExecInitNode(), which can be implemented by doing the
> > initial runtime pruning in that separate step.  That way, we'll have
> > all the necessary locks before calling ExecInitNode() and so we don't
> > need to sprinkle the CachedPlanStillValid() checks all over the place
> > and worry about missed checks and dealing with partially initialized
> > PlanState trees.
>
> I've worked on this and found that it results in a much simpler design.
>
> Attached are 0001 and 0002, which contain patches to refactor the
> runtime pruning code. These changes move initial pruning outside of
> ExecInitNode() and use the results during ExecInitNode() to determine
> the set of child subnodes to initialize.
>
> With that in place, the patches (0003, 0004) that move the locking of
> prunable relations from plancache.c into the executor becomes simpler.
> It no longer needs to modify any code called by ExecInitNode(). Since
> no new locks are taken during ExecInitNode(), I didn't have to worry
> about changing all the code involved in PlanState tree initialization
> to add checks for CachedPlan validity.  The check is only needed after
> performing initial pruning, and if the CachedPlan is invalid,
> ExecInitNode() won’t be called in the first place.

Sorry, I had missed merging some hunks into 0002 that fixed obsolete
comments.  Fixed in the attached v54.

Regarding 0002, I was a bit bothered by the need to add a new function
just to iterate over the PartitionPruningDatas and the
PartitionedRelPruningData they contain, solely to initialize the
PartitionPruneContext needed for exec pruning. To address this, I
propose 0003, which moves the initialization of those contexts to be
done "lazily" in find_matching_subplan_recurse(), where they are
actually used. To make this work, I added an is_valid flag to
PartitionPruneContext, which is checked as follows in the code block
where it's initialized:

+        if (unlikely(!pprune->exec_context.is_valid))

I didn't notice any overhead of adding this to
find_matching_partitions_recurse() which is called for every instance
of exec pruning, so I think it's worthwhile to consider 0003.

I realized that I had missed considering, in the
delay-locking-to-executor patch (now 0004), that there may be plan
objects belonging to pruned partitions, such as RowMarks and
ResultRelInfos, which should not be initialized.
ExecGetRangeTableRelation() invoked with the RT indexes in these
objects would cause crashes in Assert builds since the pruned
partitions would not have been locked. I've updated the patch to
ignore RowMarks and result relations (in ModifyTable.resultRelations)
for pruned child relations, which required adding more accounting info
to EState to store the bitmapset of unpruned RT indexes. For
ResultRelInfos, I took the approach of memsetting them to 0 for pruned
result relations and adding checks at multiple sites to ensure the
ResultRelInfo being handled is valid. I recall previously proposing
lazy initialization for these objects when first needed [1], which
would make the added code unnecessary, but I might save that for
another time.

--
Thanks, Amit Langote
[1] https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp

Commits

  1. Stamp 19beta1.

  2. Revert "Don't lock partitions pruned by initial pruning"

  3. Ensure first ModifyTable rel initialized if all are pruned

  4. Fix bug in cbc127917 to handle nested Append correctly

  5. Remove unstable test suite added by 525392d57

  6. Don't lock partitions pruned by initial pruning

  7. Fix an oversight in cbc127917 to handle MERGE correctly

  8. Track unpruned relids to avoid processing pruned relations

  9. Perform runtime initial pruning outside ExecInitNode()

  10. Move PartitionPruneInfo out of plan nodes into PlannedStmt

  11. Fix setrefs.c's failure to do expression processing on prune steps.

  12. Remove obsolete executor cleanup code

  13. Revert "Move PartitionPruneInfo out of plan nodes into PlannedStmt"

  14. Move PartitioPruneInfo out of plan nodes into PlannedStmt

  15. Refactor and cleanup runtime partition prune code a little

  16. Remove some unnecessary fields from Plan trees.

  17. Remove more redundant relation locking during executor startup.

  18. Shut down Gather's children before shutting down Gather itself.