• R/O
  • HTTP
  • SSH
  • HTTPS

pg_hint_plan: Commit

firtst release


Commit MetaInfo

Revisioncde640fdefad9b0d6cc50985fdf93eb50bf01903 (tree)
Time2022-01-14 17:51:08
AuthorKyotaro Horiguchi <horikyota.ntt@gmai...>
CommiterKyotaro Horiguchi

Log Message

Followup change to support PG14

One improvement and on fix.

Ressurect post_parse_analysys_hook function to avoid redandunt
computation of jumble state if it is generated in-core.

Properly handle the case where compute_query_id is turned off while
pg_hint_plan.enable_hint_talbe is on. This operation lead to a crash.

Change Summary

Incremental Difference

--- a/doc/pg_hint_plan-ja.html
+++ b/doc/pg_hint_plan-ja.html
@@ -101,7 +101,7 @@ postgres=#
101101
102102 <h4 id="hint-table">テーブルでの指定</h4>
103103 <p>指定したいヒントを、実行計画を制御したいクエリと併せてヒント用のテーブルに登録します。</p>
104-<p>デフォルトでは無効なので、テーブルで指定したヒントは適用されません。そのため、ヒントをテーブルで指定する場合は、<a href="#hint-GUC">pg_hint_planのGUCパラメータ</a>pg_hint_plan.enable_hint_tableの設定を変更します。</p>
104+<p>デフォルトでは無効なので、テーブルで指定したヒントは適用されません。そのため、ヒントをテーブルで指定する場合は、<a href="#hint-GUC">pg_hint_planのGUCパラメータ</a>pg_hint_plan.enable_hint_tableの設定を変更します。この機能を利用する場合にはcompute_query_idが"auto"または"on"である必要があります。</p>
105105 <p>ヒント用のテーブルは「<span class="bold">hint_plan.hints</span>」です。hint_plan.hintsテーブルには、以下の情報を登録します。</p>
106106 <table>
107107 <thead>
--- a/doc/pg_hint_plan.html
+++ b/doc/pg_hint_plan.html
@@ -81,7 +81,8 @@ postgres=# </pre>
8181 <p> Hints are described in a comment in a special form in the above
8282 section. This is inconvenient in the case where queries cannot be
8383 edited. In the case hints can be placed in a special table named
84-"hint_plan.hints". The table consists of the following columns.</p>
84+"hint_plan.hints". This feature requires compute_query_id to be "on"
85+or "auto". The table consists of the following columns.</p>
8586 <table>
8687 <thead>
8788 <tr>
--- a/expected/pg_hint_plan.out
+++ b/expected/pg_hint_plan.out
@@ -17,12 +17,75 @@ EXPLAIN (COSTS false) SELECT * FROM t1, t2 WHERE t1.val = t2.val;
1717 -> Seq Scan on t2
1818 -> Memoize
1919 Cache Key: t2.val
20+ Cache Mode: logical
2021 -> Index Scan using t1_val on t1
2122 Index Cond: (val = t2.val)
22-(6 rows)
23+(7 rows)
2324
2425 LOAD 'pg_hint_plan';
2526 SET pg_hint_plan.debug_print TO on;
27+SELECT setting <> 'off' FROM pg_settings WHERE name = 'compute_query_id';
28+ ?column?
29+----------
30+ t
31+(1 row)
32+
33+SHOW pg_hint_plan.enable_hint_table;
34+ pg_hint_plan.enable_hint_table
35+--------------------------------
36+ off
37+(1 row)
38+
39+/* query-id related test */
40+SET compute_query_id to off;
41+SET pg_hint_plan.enable_hint_table to on; -- error
42+ERROR: table hint is not activated because queryid is not available
43+HINT: Set compute_query_id to on or auto to use hint table.
44+SET compute_query_id to on;
45+SET pg_hint_plan.enable_hint_table to on;
46+SET compute_query_id to off;
47+SELECT 1; -- gets warining
48+WARNING: hint table feature is deactivated because queryid is not available
49+HINT: Set compute_query_id to "auto" or "on" to use hint table.
50+ ?column?
51+----------
52+ 1
53+(1 row)
54+
55+SELECT 1; -- not
56+ ?column?
57+----------
58+ 1
59+(1 row)
60+
61+SET compute_query_id to on;
62+SELECT 1; -- reactivated
63+LOG: hint table feature is reactivated
64+ ?column?
65+----------
66+ 1
67+(1 row)
68+
69+SET compute_query_id to off;
70+SELECT 1; -- gets warining
71+WARNING: hint table feature is deactivated because queryid is not available
72+HINT: Set compute_query_id to "auto" or "on" to use hint table.
73+ ?column?
74+----------
75+ 1
76+(1 row)
77+
78+SET pg_hint_plan.enable_hint_table to off;
79+SET compute_query_id to on;
80+SET pg_hint_plan.enable_hint_table to on;
81+SELECT 1; -- no warining
82+ ?column?
83+----------
84+ 1
85+(1 row)
86+
87+RESET compute_query_id;
88+RESET pg_hint_plan.enable_hint_table;
2689 EXPLAIN (COSTS false) SELECT * FROM t1, t2 WHERE t1.id = t2.id;
2790 QUERY PLAN
2891 --------------------------------------
@@ -39,9 +102,10 @@ EXPLAIN (COSTS false) SELECT * FROM t1, t2 WHERE t1.val = t2.val;
39102 -> Seq Scan on t2
40103 -> Memoize
41104 Cache Key: t2.val
105+ Cache Mode: logical
42106 -> Index Scan using t1_val on t1
43107 Index Cond: (val = t2.val)
44-(6 rows)
108+(7 rows)
45109
46110 /*+ Test (t1 t2) */
47111 EXPLAIN (COSTS false) SELECT * FROM t1, t2 WHERE t1.id = t2.id;
--- a/expected/ut-J.out
+++ b/expected/ut-J.out
@@ -4730,9 +4730,10 @@ EXPLAIN (COSTS false) SELECT * FROM t1, t2, t3 WHERE t1.val = t2.val and t2.id =
47304730 -> Seq Scan on t3
47314731 -> Memoize
47324732 Cache Key: t2.val
4733+ Cache Mode: logical
47334734 -> Index Scan using t1_val on t1
47344735 Index Cond: (val = t2.val)
4735-(11 rows)
4736+(12 rows)
47364737
47374738 /*+ nomemoize(t1 t2)*/
47384739 EXPLAIN (COSTS false) SELECT * FROM t1, t2, t3 WHERE t1.val = t2.val and t2.id = t3.id; -- doesn't work
@@ -4754,9 +4755,10 @@ error hint:
47544755 -> Seq Scan on t3
47554756 -> Memoize
47564757 Cache Key: t2.val
4758+ Cache Mode: logical
47574759 -> Index Scan using t1_val on t1
47584760 Index Cond: (val = t2.val)
4759-(11 rows)
4761+(12 rows)
47604762
47614763 /*+ nomemoize(t1 t2 t3)*/
47624764 EXPLAIN (COSTS false) SELECT * FROM t1, t2, t3 WHERE t1.val = t2.val and t2.id = t3.id;
--- a/expected/ut-W.out
+++ b/expected/ut-W.out
@@ -1167,7 +1167,7 @@ EXPLAIN (COSTS false) SELECT id FROM p1 UNION ALL SELECT id FROM p2;
11671167 INFO: pg_hint_plan: hint syntax error at or near "100x"
11681168 DETAIL: number of workers must be a number: Parallel
11691169 INFO: pg_hint_plan: hint syntax error at or near "-1000"
1170-DETAIL: number of workers must be positive: Parallel
1170+DETAIL: number of workers must be greater than zero: Parallel
11711171 INFO: pg_hint_plan: hint syntax error at or near "1000000"
11721172 DETAIL: number of workers = 1000000 is larger than max_worker_processes(8): Parallel
11731173 INFO: pg_hint_plan: hint syntax error at or near "hoge"
--- a/pg_hint_plan.c
+++ b/pg_hint_plan.c
@@ -237,6 +237,16 @@ static unsigned int msgqno = 0;
237237 static char qnostr[32];
238238 static const char *current_hint_str = NULL;
239239
240+/*
241+ * We can utilize in-core generated jumble state in post_parse_analyze_hook.
242+ * On the other hand there's a case where we're forced to get hints in
243+ * planner_hook, where we don't have a jumble state. If we a query had not a
244+ * hint, we need to try to retrieve hints twice or more for one query, which is
245+ * the quite common case. To avoid such case, this variables is set true when
246+ * we *try* hint retrieval.
247+ */
248+static bool current_hint_retrieved = false;
249+
240250 /* common data for all hints. */
241251 struct Hint
242252 {
@@ -394,6 +404,9 @@ typedef struct HintParser
394404 HintKeyword hint_keyword;
395405 } HintParser;
396406
407+static bool enable_hint_table_check(bool *newval, void **extra, GucSource source);
408+static void assign_enable_hint_table(bool newval, void *extra);
409+
397410 /* Module callbacks */
398411 void _PG_init(void);
399412 void _PG_fini(void);
@@ -401,6 +414,8 @@ void _PG_fini(void);
401414 static void push_hint(HintState *hstate);
402415 static void pop_hint(void);
403416
417+static void pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query,
418+ JumbleState *jstate);
404419 static PlannedStmt *pg_hint_plan_planner(Query *parse, const char *query_string,
405420 int cursorOptions,
406421 ParamListInfo boundParams);
@@ -522,6 +537,8 @@ static int hint_inhibit_level = 0; /* Inhibit hinting if this is above 0 */
522537 /* (This could not be above 1) */
523538 static int max_hint_nworkers = -1; /* Maximum nworkers of Workers hints */
524539
540+static bool hint_table_deactivated = false;
541+
525542 static const struct config_enum_entry parse_messages_level_options[] = {
526543 {"debug", DEBUG2, true},
527544 {"debug5", DEBUG5, false},
@@ -558,6 +575,7 @@ static const struct config_enum_entry parse_debug_level_options[] = {
558575 };
559576
560577 /* Saved hook values in case of unload */
578+static post_parse_analyze_hook_type prev_post_parse_analyze_hook = NULL;
561579 static planner_hook_type prev_planner = NULL;
562580 static join_search_hook_type prev_join_search = NULL;
563581 static set_rel_pathlist_hook_type prev_set_rel_pathlist = NULL;
@@ -679,11 +697,15 @@ _PG_init(void)
679697 false,
680698 PGC_USERSET,
681699 0,
682- NULL,
683- NULL,
700+ enable_hint_table_check,
701+ assign_enable_hint_table,
684702 NULL);
685703
704+ EmitWarningsOnPlaceholders("pg_hint_plan");
705+
686706 /* Install hooks. */
707+ prev_post_parse_analyze_hook = post_parse_analyze_hook;
708+ post_parse_analyze_hook = pg_hint_plan_post_parse_analyze;
687709 prev_planner = planner_hook;
688710 planner_hook = pg_hint_plan_planner;
689711 prev_join_search = join_search_hook;
@@ -717,6 +739,31 @@ _PG_fini(void)
717739 *var_ptr = NULL;
718740 }
719741
742+static bool
743+enable_hint_table_check(bool *newval, void **extra, GucSource source)
744+{
745+ if (*newval)
746+ {
747+ EnableQueryId();
748+
749+ if (!IsQueryIdEnabled())
750+ {
751+ GUC_check_errmsg("table hint is not activated because queryid is not available");
752+ GUC_check_errhint("Set compute_query_id to on or auto to use hint table.");
753+ return false;
754+ }
755+ }
756+
757+ return true;
758+}
759+
760+static void
761+assign_enable_hint_table(bool newval, void *extra)
762+{
763+ if (!newval)
764+ hint_table_deactivated = false;
765+}
766+
720767 /*
721768 * create and delete functions the hint object
722769 */
@@ -2399,7 +2446,7 @@ ParallelHintParse(ParallelHint *hint, HintState *hstate, Query *parse,
23992446 hint->base.keyword));
24002447 else if (nworkers < 0)
24012448 hint_ereport(hint->nworkers_str,
2402- ("number of workers must be positive: %s",
2449+ ("number of workers must be greater than zero: %s",
24032450 hint->base.keyword));
24042451 else if (nworkers > max_worker_processes)
24052452 hint_ereport(hint->nworkers_str,
@@ -2743,14 +2790,28 @@ pop_hint(void)
27432790 * Retrieve and store hint string from given query or from the hint table.
27442791 */
27452792 static void
2746-get_current_hint_string(Query *query, const char *query_str)
2793+get_current_hint_string(Query *query, const char *query_str,
2794+ JumbleState *jstate)
27472795 {
27482796 MemoryContext oldcontext;
27492797
2750- /* do nothing while scanning hint table */
2751- if (hint_inhibit_level > 0)
2798+ /* We shouldn't get here for internal queries. */
2799+ Assert (hint_inhibit_level == 0);
2800+
2801+ /* We shouldn't get here if hint is disabled. */
2802+ Assert (pg_hint_plan_enable_hint);
2803+
2804+ /* Do not anything if we have already tried to get hints for this query. */
2805+ if (current_hint_retrieved)
2806+ return;
2807+
2808+ /* No way to retrieve hints from empty string. */
2809+ if (!query_str)
27522810 return;
27532811
2812+ /* Don't parse the current query hereafter */
2813+ current_hint_retrieved = true;
2814+
27542815 /* Make sure trashing old hint string */
27552816 if (current_hint_str)
27562817 {
@@ -2758,10 +2819,6 @@ get_current_hint_string(Query *query, const char *query_str)
27582819 current_hint_str = NULL;
27592820 }
27602821
2761- /* Return if nothing to do. */
2762- if (!pg_hint_plan_enable_hint || !query_str)
2763- return;
2764-
27652822 /* increment the query number */
27662823 qnostr[0] = 0;
27672824 if (debug_level > 1)
@@ -2771,11 +2828,36 @@ get_current_hint_string(Query *query, const char *query_str)
27712828 /* search the hint table for a hint if requested */
27722829 if (pg_hint_plan_enable_hint_table)
27732830 {
2774- JumbleState *jstate;
2775- int query_len;
2776- char *normalized_query;
2831+ int query_len;
2832+ char *normalized_query;
27772833
2778- jstate = JumbleQuery(query, query_str);
2834+ if (!IsQueryIdEnabled())
2835+ {
2836+ /*
2837+ * compute_query_id was turned off while enable_hint_table is
2838+ * on. Do not go ahead and complain once until it is turned on
2839+ * again.
2840+ */
2841+ if (!hint_table_deactivated)
2842+ ereport(WARNING,
2843+ (errmsg ("hint table feature is deactivated because queryid is not available"),
2844+ errhint("Set compute_query_id to \"auto\" or \"on\" to use hint table.")));
2845+
2846+ hint_table_deactivated = true;
2847+ return;
2848+ }
2849+
2850+ if (hint_table_deactivated)
2851+ {
2852+ ereport(LOG, (errmsg ("hint table feature is reactivated")));
2853+ hint_table_deactivated = false;
2854+ }
2855+
2856+ if (!jstate)
2857+ jstate = JumbleQuery(query, query_str);
2858+
2859+ if (!jstate)
2860+ return;
27792861
27802862 /*
27812863 * Normalize the query string by replacing constants with '?'
@@ -2859,6 +2941,32 @@ get_current_hint_string(Query *query, const char *query_str)
28592941 }
28602942
28612943 /*
2944+ * Retrieve hint string from the current query.
2945+ */
2946+static void
2947+pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query,
2948+ JumbleState *jstate)
2949+{
2950+ if (prev_post_parse_analyze_hook)
2951+ prev_post_parse_analyze_hook(pstate, query, jstate);
2952+
2953+ if (!pg_hint_plan_enable_hint || hint_inhibit_level > 0)
2954+ return;
2955+
2956+ /* always retrieve hint from the top-level query string */
2957+ if (plpgsql_recurse_level == 0)
2958+ current_hint_retrieved = false;
2959+
2960+ /*
2961+ * Jumble state is required when hint table is used. This is the only
2962+ * chance to have one already generated in-core. If it's not the case, no
2963+ * use to do the work now and pg_hint_plan_planner() will do the all work.
2964+ */
2965+ if (jstate)
2966+ get_current_hint_string(query, pstate->p_sourcetext, jstate);
2967+}
2968+
2969+/*
28622970 * Read and set up hint information
28632971 */
28642972 static PlannedStmt *
@@ -2888,16 +2996,30 @@ pg_hint_plan_planner(Query *parse, const char *query_string, int cursorOptions,
28882996 goto standard_planner_proc;
28892997 }
28902998
2891- /* always retrieve hint from the top-level query string */
2892- if (plpgsql_recurse_level == 0 && current_hint_str)
2999+ /*
3000+ * SQL commands invoked in plpgsql functions may also have hints. In that
3001+ * case override the upper level hint by the new hint.
3002+ */
3003+ if (plpgsql_recurse_level > 0)
28933004 {
2894- pfree((void *)current_hint_str);
3005+ const char *tmp_hint_str = current_hint_str;
3006+
3007+ /* don't let get_current_hint_string free this string */
28953008 current_hint_str = NULL;
2896- }
28973009
2898- get_current_hint_string(parse, query_string);
3010+ current_hint_retrieved = false;
28993011
2900- /* No hint, go the normal way */
3012+ get_current_hint_string(parse, query_string, NULL);
3013+
3014+ if (current_hint_str == NULL)
3015+ current_hint_str = tmp_hint_str;
3016+ else if (tmp_hint_str != NULL)
3017+ pfree((void *)tmp_hint_str);
3018+ }
3019+ else
3020+ get_current_hint_string(parse, query_string, NULL);
3021+
3022+ /* No hints, go the normal way */
29013023 if (!current_hint_str)
29023024 goto standard_planner_proc;
29033025
@@ -2985,7 +3107,8 @@ pg_hint_plan_planner(Query *parse, const char *query_string, int cursorOptions,
29853107 {
29863108 /*
29873109 * Rollback changes of GUC parameters, and pop current hint context
2988- * from hint stack to rewind the state.
3110+ * from hint stack to rewind the state. current_hint_str will be freed
3111+ * by context deletion.
29893112 */
29903113 current_hint_str = prev_hint_str;
29913114 recurse_level--;
@@ -2998,12 +3121,13 @@ pg_hint_plan_planner(Query *parse, const char *query_string, int cursorOptions,
29983121
29993122 /*
30003123 * current_hint_str is useless after planning of the top-level query.
3124+ * There's a case where the caller has multiple queries. This causes hint
3125+ * parsing multiple times for the same string but we don't have a simple
3126+ * and reliable way to distinguish that case from the case where of
3127+ * separate queries.
30013128 */
3002- if (recurse_level < 1 && current_hint_str)
3003- {
3004- pfree((void *)current_hint_str);
3005- current_hint_str = NULL;
3006- }
3129+ if (recurse_level < 1)
3130+ current_hint_retrieved = false;
30073131
30083132 /* Print hint in debug mode. */
30093133 if (debug_level == 1)
--- a/sql/pg_hint_plan.sql
+++ b/sql/pg_hint_plan.sql
@@ -5,8 +5,31 @@ SET client_min_messages TO log;
55 EXPLAIN (COSTS false) SELECT * FROM t1, t2 WHERE t1.id = t2.id;
66 EXPLAIN (COSTS false) SELECT * FROM t1, t2 WHERE t1.val = t2.val;
77
8+
89 LOAD 'pg_hint_plan';
910 SET pg_hint_plan.debug_print TO on;
11+SELECT setting <> 'off' FROM pg_settings WHERE name = 'compute_query_id';
12+SHOW pg_hint_plan.enable_hint_table;
13+
14+/* query-id related test */
15+SET compute_query_id to off;
16+SET pg_hint_plan.enable_hint_table to on; -- error
17+SET compute_query_id to on;
18+SET pg_hint_plan.enable_hint_table to on;
19+SET compute_query_id to off;
20+SELECT 1; -- gets warining
21+SELECT 1; -- not
22+SET compute_query_id to on;
23+SELECT 1; -- reactivated
24+SET compute_query_id to off;
25+SELECT 1; -- gets warining
26+SET pg_hint_plan.enable_hint_table to off;
27+SET compute_query_id to on;
28+SET pg_hint_plan.enable_hint_table to on;
29+SELECT 1; -- no warining
30+
31+RESET compute_query_id;
32+RESET pg_hint_plan.enable_hint_table;
1033
1134 EXPLAIN (COSTS false) SELECT * FROM t1, t2 WHERE t1.id = t2.id;
1235 EXPLAIN (COSTS false) SELECT * FROM t1, t2 WHERE t1.val = t2.val;
Show on old repository browser