From 7a41b301487648ac1d7079441e3c7a36a95c0bc2 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 3 Sep 2026 15:59:42 -0400 Subject: [PATCH v2 2/2] pg_plan_advice: Add CHECK_FOR_INTERRUPTS() and check_stack_depth() Without CHECK_FOR_INTERRUPTS(), backends might not respond to query cancellations in a timely fashion, especially for JOIN_ORDER() specifications, where matching against each join can take O(n^2) time in the length of the join order list in degenerate cases. Recursive functions should check_stack_depth(). Reported-by: Noah Misch --- contrib/pg_plan_advice/pgpa_ast.c | 14 ++++++++++++++ contrib/pg_plan_advice/pgpa_planner.c | 7 +++++++ contrib/pg_plan_advice/pgpa_trove.c | 3 +++ 3 files changed, 24 insertions(+) diff --git a/contrib/pg_plan_advice/pgpa_ast.c b/contrib/pg_plan_advice/pgpa_ast.c index 01db8d24cd0..82254a9589c 100644 --- a/contrib/pg_plan_advice/pgpa_ast.c +++ b/contrib/pg_plan_advice/pgpa_ast.c @@ -15,6 +15,7 @@ #include "pgpa_ast.h" #include "funcapi.h" +#include "miscadmin.h" #include "utils/array.h" #include "utils/builtins.h" @@ -169,6 +170,8 @@ pgpa_parse_advice_tag(const char *tag, bool *fail) void pgpa_format_advice_target(StringInfo str, pgpa_advice_target *target) { + check_stack_depth(); + if (target->ttype != PGPA_TARGET_IDENTIFIER) { bool first = true; @@ -234,6 +237,8 @@ pgpa_index_targets_equal(pgpa_index_target *i1, pgpa_index_target *i2) bool pgpa_identifier_matches_target(pgpa_identifier *rid, pgpa_advice_target *target) { + check_stack_depth(); + /* For non-identifiers, check all descendants. */ if (target->ttype != PGPA_TARGET_IDENTIFIER) { @@ -288,6 +293,13 @@ pgpa_identifiers_match_target(int nrids, pgpa_identifier *rids, bool all_targets_used; bool *rids_used = palloc0_array(bool, nrids); + /* + * This function is called from within various loops within pgpa_planner.c; + * to avoid needing a separate CHECK_FOR_INTERRUPTS() in each one, we check + * here instead. + */ + CHECK_FOR_INTERRUPTS(); + all_targets_used = pgpa_identifiers_cover_target(nrids, rids, target, rids_used); @@ -330,6 +342,8 @@ pgpa_identifiers_cover_target(int nrids, pgpa_identifier *rids, { bool result = false; + check_stack_depth(); + if (target->ttype != PGPA_TARGET_IDENTIFIER) { result = true; diff --git a/contrib/pg_plan_advice/pgpa_planner.c b/contrib/pg_plan_advice/pgpa_planner.c index b3329b793aa..c57df4aa7f0 100644 --- a/contrib/pg_plan_advice/pgpa_planner.c +++ b/contrib/pg_plan_advice/pgpa_planner.c @@ -25,6 +25,7 @@ #include "commands/defrem.h" #include "common/hashfn_unstable.h" +#include "miscadmin.h" #include "nodes/makefuncs.h" #include "optimizer/extendplan.h" #include "optimizer/pathnode.h" @@ -1649,6 +1650,8 @@ pgpa_planner_apply_scan_advice(RelOptInfo *rel, pgpa_trove_entry *my_entry = &scan_entries[i]; uint64 my_scan_type = all_scan_mask; + CHECK_FOR_INTERRUPTS(); + /* Translate our advice tags to a scan strategy advice value. */ if (my_entry->tag == PGPA_TAG_DO_NOT_SCAN) my_scan_type = 0; @@ -1711,6 +1714,8 @@ pgpa_planner_apply_scan_advice(RelOptInfo *rel, uint64 my_gather_mask = 0; bool just_one_rel; + CHECK_FOR_INTERRUPTS(); + just_one_rel = my_entry->target->ttype == PGPA_TARGET_IDENTIFIER || list_length(my_entry->target->children) == 1; @@ -1882,6 +1887,8 @@ pgpa_planner_append_feedback(List *list, pgpa_trove *trove, pgpa_trove_entry *entry = &entries[i]; DefElem *item; + CHECK_FOR_INTERRUPTS(); + /* * If this entry was fully matched, check whether generating advice * from this plan would produce such an entry. If not, label the entry diff --git a/contrib/pg_plan_advice/pgpa_trove.c b/contrib/pg_plan_advice/pgpa_trove.c index ca69f3bd3df..e5840051386 100644 --- a/contrib/pg_plan_advice/pgpa_trove.c +++ b/contrib/pg_plan_advice/pgpa_trove.c @@ -27,6 +27,7 @@ #include "pgpa_trove.h" #include "common/hashfn_unstable.h" +#include "miscadmin.h" /* * An advice trove is organized into a series of "slices", each of which @@ -402,6 +403,8 @@ pgpa_trove_add_to_hash(pgpa_trove_entry_hash *hash, pgpa_advice_target *target, pgpa_trove_entry_element *element; bool found; + check_stack_depth(); + /* For non-identifiers, add entries for all descendants. */ if (target->ttype != PGPA_TARGET_IDENTIFIER) { -- 2.51.0