OSDN Git Service

Fix Rows hint parsing
[pghintplan/pg_hint_plan.git] / pg_hint_plan.c
index 213a1c2..b7bc728 100644 (file)
@@ -4,7 +4,7 @@
  *               do instructions or hints to the planner using C-style block comments
  *               of the SQL.
  *
- * Copyright (c) 2012-2014, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
+ * Copyright (c) 2012-2019, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
  *
  *-------------------------------------------------------------------------
  */
@@ -198,6 +198,14 @@ static unsigned int msgqno = 0;
 static char qnostr[32];
 static const char *current_hint_str = NULL;
 
+/*
+ * However we usually take a hint stirng in post_parse_analyze_hook, we still
+ * need to do so in planner_hook when client starts query execution from the
+ * bind message on a prepared query. This variable prevent duplicate and
+ * sometimes harmful hint string retrieval.
+ */
+static bool current_hint_retrieved = false;
+
 /* common data for all hints. */
 struct Hint
 {
@@ -307,6 +315,7 @@ struct HintState
        /* for scan method hints */
        ScanMethodHint **scan_hints;            /* parsed scan hints */
        int                             init_scan_mask;         /* initial value scan parameter */
+       PlannerInfo        *current_root;               /* PlannerInfo for the followings */
        Index                   parent_relid;           /* inherit parent table relid */
        ScanMethodHint *parent_hint;            /* inherit parent table scan hint */
        List               *parent_index_infos; /* information of inherit parent table's
@@ -346,6 +355,10 @@ static void push_hint(HintState *hstate);
 static void pop_hint(void);
 
 static void pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query);
+static void pg_hint_plan_ProcessUtility(Node *parsetree,
+                                       const char *queryString,
+                                       ProcessUtilityContext context, ParamListInfo params,
+                                       DestReceiver *dest, char *completionTag);
 static PlannedStmt *pg_hint_plan_planner(Query *parse, int cursorOptions,
                                                                                 ParamListInfo boundParams);
 static void pg_hint_plan_get_relation_info(PlannerInfo *root,
@@ -438,6 +451,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)        */
 
@@ -481,6 +495,7 @@ static post_parse_analyze_hook_type prev_post_parse_analyze_hook = NULL;
 static planner_hook_type prev_planner = NULL;
 static get_relation_info_hook_type prev_get_relation_info = NULL;
 static join_search_hook_type prev_join_search = NULL;
+static ProcessUtility_hook_type prev_ProcessUtility_hook = NULL;
 
 /* Hold reference to currently active hint */
 static HintState *current_hint = NULL;
@@ -491,12 +506,6 @@ static HintState *current_hint = NULL;
  */
 static List *HintStateStack = NIL;
 
-/*
- * Holds statement name during executing EXECUTE command.  NULL for other
- * statements.
- */
-static char       *stmt_name = NULL;
-
 static const HintParser parsers[] = {
        {HINT_SEQSCAN, ScanMethodHintCreate, HINT_KEYWORD_SEQSCAN},
        {HINT_INDEXSCAN, ScanMethodHintCreate, HINT_KEYWORD_INDEXSCAN},
@@ -611,6 +620,8 @@ _PG_init(void)
        get_relation_info_hook = pg_hint_plan_get_relation_info;
        prev_join_search = join_search_hook;
        join_search_hook = pg_hint_plan_join_search;
+       prev_ProcessUtility_hook = ProcessUtility_hook;
+       ProcessUtility_hook = pg_hint_plan_ProcessUtility;
 
        /* setup PL/pgSQL plugin hook */
        var_ptr = (PLpgSQL_plugin **) find_rendezvous_variable("PLpgSQL_plugin");
@@ -633,6 +644,7 @@ _PG_fini(void)
        planner_hook = prev_planner;
        get_relation_info_hook = prev_get_relation_info;
        join_search_hook = prev_join_search;
+       ProcessUtility_hook = prev_ProcessUtility_hook;
 
        /* uninstall PL/pgSQL plugin hook */
        var_ptr = (PLpgSQL_plugin **) find_rendezvous_variable("PLpgSQL_plugin");
@@ -858,6 +870,7 @@ HintStateCreate(void)
        memset(hstate->num_hints, 0, sizeof(hstate->num_hints));
        hstate->scan_hints = NULL;
        hstate->init_scan_mask = 0;
+       hstate->current_root = NULL;
        hstate->parent_relid = 0;
        hstate->parent_hint = NULL;
        hstate->parent_index_infos = NIL;
@@ -1054,7 +1067,8 @@ RowsHintDesc(RowsHint *hint, StringInfo buf, bool nolf)
                        quote_value(buf, hint->relnames[i]);
                }
        }
-       appendStringInfo(buf, " %s", hint->rows_str);
+       if (hint->rows_str != NULL)
+               appendStringInfo(buf, " %s", hint->rows_str);
        appendStringInfoString(buf, ")");
        if (!nolf)
                appendStringInfoChar(buf, '\n');
@@ -1664,17 +1678,15 @@ get_query_string(ParseState *pstate, Query *query, Query **jumblequery)
        /*
         * If debug_query_string is set, it is the top level statement. But in some
         * cases we reach here with debug_query_string set NULL for example in the
-        * case of DESCRIBE message handling. We may still see a candidate
-        * top-level query in pstate in the case.
+        * case of DESCRIBE message handling or EXECUTE command. We may still see a
+        * candidate top-level query in pstate in the case.
         */
-       if (!p)
-       {
-               /* We don't see a query string, return NULL */
-               if (!pstate->p_sourcetext)
-                       return NULL;
-
+       if (pstate && pstate->p_sourcetext)
                p = pstate->p_sourcetext;
-       }
+
+       /* We don't see a query string, return NULL */
+       if (!p)
+               return NULL;
 
        if (jumblequery != NULL)
                *jumblequery = query;
@@ -1746,21 +1758,35 @@ get_query_string(ParseState *pstate, Query *query, Query **jumblequery)
                        PreparedStatement  *entry;
 
                        entry = FetchPreparedStatement(stmt->name, true);
-                       p = entry->plansource->query_string;
-                       target_query = (Query *) linitial (entry->plansource->query_list);
+
+                       if (entry->plansource->is_valid)
+                       {
+                               p = entry->plansource->query_string;
+                               target_query = (Query *) linitial (entry->plansource->query_list);
+                       }
+                       else
+                       {
+                               /* igonre the hint for EXECUTE if invalidated */
+                               p = NULL;
+                               target_query = NULL;
+                       }
                }
                        
                /* JumbleQuery accespts only a non-utility Query */
-               if (!IsA(target_query, Query) ||
-                       target_query->utilityStmt != NULL)
+               if (target_query &&
+                       (!IsA(target_query, Query) ||
+                        target_query->utilityStmt != NULL))
                        target_query = NULL;
 
                if (jumblequery)
                        *jumblequery = target_query;
        }
-
-       /* Return NULL if the pstate is not identical to the top-level query */
-       else if (strcmp(pstate->p_sourcetext, p) != 0)
+       /*
+        * Return NULL if pstate is not of top-level query.  We don't need this
+        * when jumble info is not requested or cannot do this when pstate is NULL.
+        */
+       else if (!jumblequery && pstate && pstate->p_sourcetext != p &&
+                        strcmp(pstate->p_sourcetext, p) != 0)
                p = NULL;
 
        return p;
@@ -2216,6 +2242,8 @@ RowsHintParse(RowsHint *hint, HintState *hstate, Query *parse,
        List               *name_list = NIL;
        char               *rows_str;
        char               *end_ptr;
+       ListCell   *l;
+       int                     i = 0;
 
        if ((str = parse_parentheses(str, &name_list, hint_keyword)) == NULL)
                return NULL;
@@ -2223,23 +2251,28 @@ RowsHintParse(RowsHint *hint, HintState *hstate, Query *parse,
        /* Last element must be rows specification */
        hint->nrels = list_length(name_list) - 1;
 
-       if (hint->nrels > 0)
+       if (hint->nrels < 1)
        {
-               ListCell   *l;
-               int                     i = 0;
+               hint_ereport(str,
+                                        ("%s hint needs at least one relation followed by one correction term.",
+                                         hint->base.keyword));
+               hint->base.state = HINT_STATE_ERROR;
 
-               /*
-                * Transform relation names from list to array to sort them with qsort
-                * after.
-                */
-               hint->relnames = palloc(sizeof(char *) * hint->nrels);
-               foreach (l, name_list)
-               {
-                       if (hint->nrels <= i)
-                               break;
-                       hint->relnames[i] = lfirst(l);
-                       i++;
-               }
+               return str;
+       }
+       
+
+       /*
+        * Transform relation names from list to array to sort them with qsort
+        * after.
+        */
+       hint->relnames = palloc(sizeof(char *) * hint->nrels);
+       foreach (l, name_list)
+       {
+               if (hint->nrels <= i)
+                       break;
+               hint->relnames[i] = lfirst(l);
+               i++;
        }
 
        /* Retieve rows estimation */
@@ -2442,24 +2475,25 @@ pop_hint(void)
 }
 
 /*
- * Retrieve and store a hint string from given query or from the hint table.
- * If we are using the hint table, the query string is needed to be normalized.
- * However, ParseState, which is not available in planner_hook, is required to
- * check if the query tree (Query) is surely corresponding to the target query.
+ * Retrieve and store hint string from given query or from the hint table.
  */
 static void
-pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query)
+get_current_hint_string(ParseState *pstate, Query *query)
 {
        const char *query_str;
        MemoryContext   oldcontext;
 
-       if (prev_post_parse_analyze_hook)
-               prev_post_parse_analyze_hook(pstate, query);
-
        /* do nothing under hint table search */
        if (hint_inhibit_level > 0)
                return;
 
+       /* We alredy have one, don't parse it again. */
+       if (current_hint_retrieved)
+               return;
+
+       /* Don't parse the current query hereafter */
+       current_hint_retrieved = true;
+
        if (!pg_hint_plan_enable_hint)
        {
                if (current_hint_str)
@@ -2500,8 +2534,8 @@ pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query)
                if (jumblequery)
                {
                        /*
-                        * XXX: normalizing code is copied from pg_stat_statements.c, so be
-                        * careful to PostgreSQL's version up.
+                        * XXX: normalization code is copied from pg_stat_statements.c.
+                        * Make sure to keep up-to-date with it.
                         */
                        jstate.jumble = (unsigned char *) palloc(JUMBLE_SIZE);
                        jstate.jumble_len = 0;
@@ -2577,8 +2611,8 @@ pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query)
        {
                /*
                 * get hints from the comment. However we may have the same query
-                * string with the previous call, but just retrieving hints is expected
-                * to be faster than checking for identicalness before retrieval.
+                * string with the previous call, but the extra comparison seems no
+                * use..
                 */
                if (current_hint_str)
                        pfree((void *)current_hint_str);
@@ -2587,11 +2621,19 @@ pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query)
                current_hint_str = get_hints_from_comment(query_str);
                MemoryContextSwitchTo(oldcontext);
        }
+       else
+       {
+               /*
+                * Failed to get query. We would be in fetching invalidated
+                * plancache. Try the next chance.
+                */
+               current_hint_retrieved = false;
+       }
 
        if (debug_level > 1)
        {
-               if (debug_level == 1 &&
-                       (stmt_name || strcmp(query_str, debug_query_string)))
+               if (debug_level == 1 && query_str && debug_query_string &&
+                       strcmp(query_str, debug_query_string))
                        ereport(pg_hint_plan_debug_message_level,
                                        (errmsg("hints in comment=\"%s\"",
                                                        current_hint_str ? current_hint_str : "(none)"),
@@ -2599,9 +2641,10 @@ pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query)
                                         errhidecontext(msgqno != qno)));
                else
                        ereport(pg_hint_plan_debug_message_level,
-                                       (errmsg("hints in comment=\"%s\", stmt=\"%s\", query=\"%s\", debug_query_string=\"%s\"",
+                                       (errmsg("hints in comment=\"%s\", query=\"%s\", debug_query_string=\"%s\"",
                                                        current_hint_str ? current_hint_str : "(none)",
-                                                       stmt_name, query_str, debug_query_string),
+                                                       query_str ? query_str : "(none)",
+                                                       debug_query_string ? debug_query_string : "(none)"),
                                         errhidestmt(msgqno != qno),
                                         errhidecontext(msgqno != qno)));
                msgqno = qno;
@@ -2609,6 +2652,43 @@ pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query)
 }
 
 /*
+ * Retrieve hint string from the current query.
+ */
+static void
+pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query)
+{
+       if (prev_post_parse_analyze_hook)
+               prev_post_parse_analyze_hook(pstate, query);
+
+       /* always retrieve hint from the top-level query string */
+       if (plpgsql_recurse_level == 0)
+               current_hint_retrieved = false;
+
+       get_current_hint_string(pstate, query);
+}
+
+/*
+ * We need to reset current_hint_retrieved flag always when a command execution
+ * is finished. This is true even for a pure utility command that doesn't
+ * involve planning phase.
+ */
+static void
+pg_hint_plan_ProcessUtility(Node *parsetree, const char *queryString,
+                                       ProcessUtilityContext context, ParamListInfo params,
+                                       DestReceiver *dest, char *completionTag)
+{
+       if (prev_ProcessUtility_hook)
+               prev_ProcessUtility_hook(parsetree, queryString, context, params,
+                                                                dest, completionTag);
+       else
+               standard_ProcessUtility(parsetree, queryString, context, params,
+                                                                dest, completionTag);
+
+       if (plpgsql_recurse_level == 0)
+               current_hint_retrieved = false;
+}
+
+/*
  * Read and set up hint information
  */
 static PlannedStmt *
@@ -2617,6 +2697,7 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
        int                             save_nestlevel;
        PlannedStmt        *result;
        HintState          *hstate;
+       const char         *prev_hint_str = NULL;
 
        /*
         * Use standard planner if pg_hint_plan is disabled or current nesting 
@@ -2641,7 +2722,8 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
         * Support for nested plpgsql functions. This is quite ugly but this is the
         * only point I could find where I can get the query string.
         */
-       if (plpgsql_recurse_level > 0)
+       if (plpgsql_recurse_level > 0 &&
+               error_context_stack && error_context_stack->arg)
        {
                MemoryContext oldcontext;
 
@@ -2654,6 +2736,14 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
                MemoryContextSwitchTo(oldcontext);
        }
 
+       /*
+        * Query execution in extended protocol can be started without the analyze
+        * phase. In the case retrieve hint string here.
+        */
+       if (!current_hint_str)
+               get_current_hint_string(NULL, parse);
+
+       /* No hint, go the normal way */
        if (!current_hint_str)
                goto standard_planner_proc;
 
@@ -2700,8 +2790,17 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
        }
 
        /*
-        * Use PG_TRY mechanism to recover GUC parameters and current_hint to the
-        * state when this planner started when error occurred in planner.
+        * 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;
+       current_hint_str = NULL;
+       
+       /*
+        * Use PG_TRY mechanism to recover GUC parameters and current_hint_state to
+        * the state when this planner started when error occurred in planner.
         */
        PG_TRY();
        {
@@ -2709,6 +2808,9 @@ pg_hint_plan_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
                        result = (*prev_planner) (parse, cursorOptions, boundParams);
                else
                        result = standard_planner(parse, cursorOptions, boundParams);
+
+               current_hint_str = prev_hint_str;
+               recurse_level--;
        }
        PG_CATCH();
        {
@@ -2716,12 +2818,25 @@ 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();
        }
        PG_END_TRY();
 
+
+       /*
+        * current_hint_str is useless after planning of the top-level query.
+        */
+       if (plpgsql_recurse_level < 1 && current_hint_str)
+       {
+               pfree((void *)current_hint_str);
+               current_hint_str = NULL;
+               current_hint_retrieved = false;
+       }
+
        /* Print hint in debug mode. */
        if (debug_level == 1)
                HintStateDump(current_hint);
@@ -3206,7 +3321,11 @@ pg_hint_plan_get_relation_info(PlannerInfo *root, Oid relationObjectId,
                return;
        }
 
-       /* Find the parent for this relation */
+       /* Forget about the parent of another subquery */
+       if (root != current_hint->current_root)
+               current_hint->parent_relid = 0;
+
+       /* Find the parent for this relation other than the registered parent */
        foreach (l, root->append_rel_list)
        {
                AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(l);
@@ -3214,7 +3333,10 @@ pg_hint_plan_get_relation_info(PlannerInfo *root, Oid relationObjectId,
                if (appinfo->child_relid == rel->relid)
                {
                        if (current_hint->parent_relid != appinfo->parent_relid)
+                       {
                                new_parent_relid = appinfo->parent_relid;
+                               current_hint->current_root = root;
+                       }
                        break;
                }
        }