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.

diff --git a/pg_hint_plan.c b/pg_hint_plan.c
index 4e2f5e5..c6cd055 100644 (file)
--- 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;