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: Tomas Vondra <tomas@vondra.me>
Cc: Robert Haas <robertmhaas@gmail.com>, 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: 2025-01-23T07:15:48Z
Lists: pgsql-hackers

Attachments

On Thu, Dec 12, 2024 at 4:58 PM Amit Langote <amitlangote09@gmail.com> wrote:
> To summarize how extensions can be affected:
>
> 1. Plan invalidation during standard_ExecutorStart(): A plan tree
> originating from a CachedPlan can become invalid during
> standard_ExecutorStart() due to locks taken on leaf partitions that
> survive initial pruning. Extensions should be updated to handle this
> scenario by checking ExecPlanStillValid(estate) immediately after
> calling standard_ExecutorStart() in their ExecutorStart_hook. If it
> returns false, the extensions should avoid further processing.
>
> 2. Validation of RT indexes: If the plan tree remains valid, any
> direct manipulation of relations using RT indexes must first verify
> that the RT index is present in the EState.es_unpruned_relids
> bitmapset. This bitmapset includes: a) RT indexes of relations that
> are originally unprunable (and locked during GetCachedPlan()), and
> b) RT indexes of leaf partitions that survive initial partition
> pruning. This step is crucial because pruned relations are not locked.
> Additionally, with the update in 0004, attempting to open pruned
> relations using ExecGetRangeTableRelation() will result in an error.
>
> I’d love to hear from anyone maintaining executor hooks, such as those
> from Timescale, Citus, or other extension developers. Please give this
> patch set (0001-0004) a try and let me know if you run into any issues
> or have feedback.  0005 is a sketch of an approach that eliminates the
> need for extensions to check ExecPlanStillValid() in their
> ExecutorStart_hook.

I’ve rebased over recent changes to setrefs.c (commit bf826ea0629).
During the rebase, I realized that the patch
0002-Initialize-PartitionPruneContexts-lazily wasn’t a good idea after
all.

The test case added by bf826ea0629 highlighted an issue: initializing
pruning expressions lazily during execution could leave the
Append/MergeAppend node’s PlanState.subPlan uninitialized at
ExecInitNode() time. Initially, I thought this would have only
cosmetic consequences -- such as changes in test case output where
SubPlans referenced in "exec" pruning expressions wouldn’t appear --
but I may have underestimated the problem. As a result, I’ve abandoned
that approach and the patch in favor of initializing all pruning
expressions during plan initialization.

Additionally, I revisited the impact of the main patch on
ExecutorStart_hooks. It seems better to change the return type from
void to bool, returning the result of
ExecPlanStillValid(queryDesc->estate). This change has the added
benefit of breaking extensions that use ExecutorStart_hook at compile
time, encouraging authors to update their code. The updated commit
message includes details on additional checks extensions must
implement, particularly for cases where they might access pruned and
thus unlocked relations.

I've stared at the refactoring patches 0001 and 0002 for long enough
at this point that I'd like to commit them early next week, barring
further comments or objections.  I'll keep staring at 0003.


--
Thanks, Amit Langote

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.