From 000c325874cc07eb944447d046a122dcc8642520 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Tue, 26 Feb 2019 16:10:29 +0900 Subject: [PATCH] Correctly handle planner nesting pg_hint_plan assumed that plpgsql is the only source of nested planner calls. Actually nested call can be made in any shapes. Most of the cases doesn't harm but in a special case where pg_dbms_stats makes a SPI call during query planning, that affects subsequent planner work. Hints lose effect when pg_dbms_stats searches "locked statistics" tables while planning the target query. 9.5 is not affected. Back patched back to 9.6. --- pg_hint_plan.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/pg_hint_plan.c b/pg_hint_plan.c index 4e2f5e5..c6cd055 100644 --- a/pg_hint_plan.c +++ b/pg_hint_plan.c @@ -516,6 +516,7 @@ static int pg_hint_plan_debug_message_level = LOG; static bool pg_hint_plan_enable_hint_table = false; static int plpgsql_recurse_level = 0; /* PLpgSQL recursion level */ +static int recurse_level = 0; /* recursion level incl. direct SPI calls */ static int hint_inhibit_level = 0; /* Inhibit hinting if this is above 0 */ /* (This could not be above 1) */ static int max_hint_nworkers = -1; /* Maximum nworkers of Workers hints */ @@ -2988,6 +2989,7 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) int save_nestlevel; PlannedStmt *result; HintState *hstate; + const char *prev_hint_str; /* * Use standard planner if pg_hint_plan is disabled or current nesting @@ -3016,9 +3018,6 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) { MemoryContext oldcontext; - if (current_hint_str) - pfree((void *)current_hint_str); - oldcontext = MemoryContextSwitchTo(TopMemoryContext); current_hint_str = get_hints_from_comment((char *)error_context_stack->arg); @@ -3088,10 +3087,21 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) */ PG_TRY(); { + /* + * The planner call below may replace current_hint_str. Store and + * restore it so that the subsequent planning in the upper level + * doesn't get confused. + */ + recurse_level++; + prev_hint_str = current_hint_str; + if (prev_planner) result = (*prev_planner) (parse, cursorOptions, boundParams); else result = standard_planner(parse, cursorOptions, boundParams); + + current_hint_str = prev_hint_str; + recurse_level--; } PG_CATCH(); { @@ -3099,6 +3109,8 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) * Rollback changes of GUC parameters, and pop current hint context * from hint stack to rewind the state. */ + current_hint_str = prev_hint_str; + recurse_level--; AtEOXact_GUC(true, save_nestlevel); pop_hint(); PG_RE_THROW(); @@ -3109,7 +3121,7 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) /* * current_hint_str is useless after planning of the top-level query. */ - if (plpgsql_recurse_level < 1 && current_hint_str) + if (recurse_level < 1 && current_hint_str) { pfree((void *)current_hint_str); current_hint_str = NULL; -- 2.11.0