提交: ceb5af4e8d1baaf04faeee21c602bddb1dc75e4c - amalsyah12 (git)

firtst release

Commit MetaInfo

• 修訂: ceb5af4e8d1baaf04faeee21c602bddb1dc75e4c (tree)
• 時間: 2020-02-14 15:43:50
• 作者: Kyotaro Horiguchi <horikyota.ntt@gmai...>
• Commiter: Kyotaro Horiguchi

Log Message

Fix crash bug caused by plancache invalidation

https://github.com/ossc-db/pg_hint_plan/issues/41

After plancache is invalidated then revaliated, get_query_string
accesses query_list of invalid plansource then crash. Ignore invalid
plancache and get the correct hint string and Query node at the next
planning time during revalidation.

On the way fixing this, a bug related to planner reentrance is
fixed. That fix causes behavioral change for nested
planning. Previously outer-level hint (wrongly) overrides inner-level
query but currenlty outer-level hint no longer affects inner-level
query.

Change Summary

• modified: expected/ut-A.out
• modified: pg_hint_plan.c
• modified: sql/ut-A.sql

差異

--- a/expected/ut-A.out
+++ b/expected/ut-A.out
@@ -4464,6 +4464,11 @@ CREATE OR REPLACE FUNCTION recall_planner() RETURNS int AS $$
 	 ORDER BY t_1.c1 LIMIT 1;
 $$ LANGUAGE SQL IMMUTABLE;
 --No.13-4-1
+-- recall_planner() is reduced to constant while planning using the
+-- hint defined in the function. Then the outer query is planned based
+-- on the following hint. pg_hint_plan shows the log for the function
+-- but the resulting explain output doesn't contain the corresponding
+-- plan.
 /*+HashJoin(t_1 t_2)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
@@ -4471,7 +4476,7 @@ EXPLAIN (COSTS false)
   ORDER BY t_1.c1;
 LOG:  pg_hint_plan:
 used hint:
-HashJoin(t_1 t_2)
+IndexScan(t_1)
 not used hint:
 duplication hint:
 error hint:
@@ -4496,6 +4501,7 @@ error hint:
 (7 rows)
 
 --No.13-4-2
+--See description for No.13-4-1
 /*+HashJoin(st_1 st_2)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 st_1
@@ -4503,8 +4509,8 @@ EXPLAIN (COSTS false)
   ORDER BY st_1.c1;
 LOG:  pg_hint_plan:
 used hint:
+IndexScan(t_1)
 not used hint:
-HashJoin(st_1 st_2)
 duplication hint:
 error hint:
 
@@ -4528,6 +4534,7 @@ error hint:
 (7 rows)
 
 --No.13-4-3
+--See description for No.13-4-1
 /*+HashJoin(t_1 t_2)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 st_1
@@ -4535,7 +4542,7 @@ EXPLAIN (COSTS false)
   ORDER BY st_1.c1;
 LOG:  pg_hint_plan:
 used hint:
-HashJoin(t_1 t_2)
+IndexScan(t_1)
 not used hint:
 duplication hint:
 error hint:
@@ -4559,6 +4566,7 @@ error hint:
 (6 rows)
 
 --No.13-4-4
+--See description for No.13-4-1
 /*+HashJoin(st_1 st_2)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
@@ -4566,8 +4574,8 @@ EXPLAIN (COSTS false)
   ORDER BY t_1.c1;
 LOG:  pg_hint_plan:
 used hint:
+IndexScan(t_1)
 not used hint:
-HashJoin(st_1 st_2)
 duplication hint:
 error hint:
 
@@ -4590,19 +4598,18 @@ error hint:
 (6 rows)
 
 --No.13-4-5
+-- See description for No.13-4-1. No joins in ths plan, so
+-- pg_hint_plan doesn't complain on the wrongly written error hint.
 /*+HashJoin(t_1 t_1)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
   ORDER BY t_1.c1;
-INFO:  pg_hint_plan: hint syntax error at or near "HashJoin(t_1 t_1)"
-DETAIL:  Relation name "t_1" is duplicated.
-CONTEXT:  SQL function "recall_planner" during startup
 LOG:  pg_hint_plan:
 used hint:
+IndexScan(t_1)
 not used hint:
 duplication hint:
 error hint:
-HashJoin(t_1 t_1)
 
 CONTEXT:  SQL function "recall_planner" during startup
 LOG:  pg_hint_plan:
@@ -4627,6 +4634,14 @@ EXPLAIN (COSTS false)
  SELECT recall_planner_one_t() FROM s1.t1 t_1
    JOIN s1.t2 t_2 ON (t_1.c1 = t_2.c1)
   ORDER BY t_1.c1;
+LOG:  pg_hint_plan:
+used hint:
+IndexScan(t_1)
+not used hint:
+duplication hint:
+error hint:
+
+CONTEXT:  SQL function "recall_planner_one_t" during startup
                  QUERY PLAN                  
 ---------------------------------------------
  Merge Join
@@ -4644,8 +4659,8 @@ EXPLAIN (COSTS false)
   ORDER BY t_1.c1;
 LOG:  pg_hint_plan:
 used hint:
+IndexScan(t_1)
 not used hint:
-HashJoin(t_1 t_1)
 duplication hint:
 error hint:
 
@@ -4672,20 +4687,18 @@ HashJoin(t_1 t_1)
 DROP FUNCTION recall_planner_one_t(int);
 ERROR:  function recall_planner_one_t(integer) does not exist
 --No.13-4-7
+-- See description for No.13-4-1. Complains on the wrongly written hint.
 /*+HashJoin(t_1 t_1)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
    JOIN s1.t2 t_2 ON (t_1.c1 = t_2.c1)
   ORDER BY t_1.c1;
-INFO:  pg_hint_plan: hint syntax error at or near "HashJoin(t_1 t_1)"
-DETAIL:  Relation name "t_1" is duplicated.
-CONTEXT:  SQL function "recall_planner" during startup
 LOG:  pg_hint_plan:
 used hint:
+IndexScan(t_1)
 not used hint:
 duplication hint:
 error hint:
-HashJoin(t_1 t_1)
 
 CONTEXT:  SQL function "recall_planner" during startup
 INFO:  pg_hint_plan: hint syntax error at or near "HashJoin(t_1 t_1)"
@@ -4715,15 +4728,11 @@ EXPLAIN (COSTS false)
   ORDER BY t_1.c1;
 INFO:  pg_hint_plan: hint syntax error at or near "MergeJoin(t_1 t_2)HashJoin(t_1 t_2)"
 DETAIL:  Conflict join method hint.
-INFO:  pg_hint_plan: hint syntax error at or near "MergeJoin(t_1 t_2)HashJoin(t_1 t_2)"
-DETAIL:  Conflict join method hint.
-CONTEXT:  SQL function "recall_planner" during startup
 LOG:  pg_hint_plan:
 used hint:
-HashJoin(t_1 t_2)
+IndexScan(t_1)
 not used hint:
 duplication hint:
-MergeJoin(t_1 t_2)
 error hint:
 
 CONTEXT:  SQL function "recall_planner" during startup
@@ -4746,3 +4755,93 @@ error hint:
                ->  Seq Scan on t2 t_2
 (7 rows)
 
+--No.14-1-1 plancache invalidation
+CREATE TABLE s1.tpc AS SELECT a FROM generate_series(0, 999) a;
+CREATE INDEX ON s1.tpc(a);
+PREPARE p1 AS SELECT * FROM s1.tpc WHERE a < 999;
+/*+ IndexScan(tpc) */PREPARE p2 AS SELECT * FROM s1.tpc WHERE a < 999;
+/*+ SeqScan(tpc) */PREPARE p3(int) AS SELECT * FROM s1.tpc WHERE a = $1;
+EXPLAIN EXECUTE p1;
+                      QUERY PLAN                      
+------------------------------------------------------
+ Seq Scan on tpc  (cost=0.00..17.50 rows=333 width=4)
+   Filter: (a < 999)
+(2 rows)
+
+EXPLAIN EXECUTE p2;
+LOG:  pg_hint_plan:
+used hint:
+IndexScan(tpc)
+not used hint:
+duplication hint:
+error hint:
+
+                               QUERY PLAN                               
+------------------------------------------------------------------------
+ Index Scan using tpc_a_idx on tpc  (cost=0.28..34.10 rows=333 width=4)
+   Index Cond: (a < 999)
+(2 rows)
+
+EXPLAIN EXECUTE p3(500);
+LOG:  pg_hint_plan:
+used hint:
+SeqScan(tpc)
+not used hint:
+duplication hint:
+error hint:
+
+                     QUERY PLAN                     
+----------------------------------------------------
+ Seq Scan on tpc  (cost=0.00..17.50 rows=5 width=4)
+   Filter: (a = 500)
+(2 rows)
+
+-- The DROP invalidates the plan caches
+DROP TABLE s1.tpc;
+EXPLAIN EXECUTE p1;
+ERROR:  relation "s1.tpc" does not exist
+EXPLAIN EXECUTE p2;
+ERROR:  relation "s1.tpc" does not exist
+EXPLAIN EXECUTE p3(500);
+ERROR:  relation "s1.tpc" does not exist
+CREATE TABLE s1.tpc AS SELECT a FROM generate_series(0, 999) a;
+CREATE INDEX ON s1.tpc(a);
+EXPLAIN EXECUTE p1;
+                      QUERY PLAN                      
+------------------------------------------------------
+ Seq Scan on tpc  (cost=0.00..17.50 rows=333 width=4)
+   Filter: (a < 999)
+(2 rows)
+
+EXPLAIN EXECUTE p2;
+LOG:  pg_hint_plan:
+used hint:
+IndexScan(tpc)
+not used hint:
+duplication hint:
+error hint:
+
+                               QUERY PLAN                               
+------------------------------------------------------------------------
+ Index Scan using tpc_a_idx on tpc  (cost=0.28..34.10 rows=333 width=4)
+   Index Cond: (a < 999)
+(2 rows)
+
+EXPLAIN EXECUTE p3(500);
+LOG:  pg_hint_plan:
+used hint:
+SeqScan(tpc)
+not used hint:
+duplication hint:
+error hint:
+
+                     QUERY PLAN                     
+----------------------------------------------------
+ Seq Scan on tpc  (cost=0.00..17.50 rows=5 width=4)
+   Filter: (a = 500)
+(2 rows)
+
+DEALLOCATE p1;
+DEALLOCATE p2;
+DEALLOCATE p3;
+DROP TABLE s1.tpc;

--- a/pg_hint_plan.c
+++ b/pg_hint_plan.c
@@ -451,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)        */
 
@@ -1679,7 +1680,7 @@ get_query_string(ParseState *pstate, Query *query, Query **jumblequery)
 	 * case of DESCRIBE message handling or EXECUTE command. We may still see a
 	 * candidate top-level query in pstate in the case.
 	 */
-	if (!p && pstate)
+	if (pstate && pstate->p_sourcetext)
 		p = pstate->p_sourcetext;
 
 	/* We don't see a query string, return NULL */
@@ -1756,13 +1757,24 @@ 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)
@@ -2601,6 +2613,14 @@ get_current_hint_string(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)
 	{
@@ -2669,6 +2689,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 
@@ -2761,8 +2782,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();
 	{
@@ -2770,6 +2800,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();
 	{
@@ -2777,6 +2810,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();

--- a/sql/ut-A.sql
+++ b/sql/ut-A.sql
@@ -1150,6 +1150,11 @@ CREATE OR REPLACE FUNCTION recall_planner() RETURNS int AS $$
 $$ LANGUAGE SQL IMMUTABLE;
 
 --No.13-4-1
+-- recall_planner() is reduced to constant while planning using the
+-- hint defined in the function. Then the outer query is planned based
+-- on the following hint. pg_hint_plan shows the log for the function
+-- but the resulting explain output doesn't contain the corresponding
+-- plan.
 /*+HashJoin(t_1 t_2)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
@@ -1157,6 +1162,7 @@ EXPLAIN (COSTS false)
   ORDER BY t_1.c1;
 
 --No.13-4-2
+--See description for No.13-4-1
 /*+HashJoin(st_1 st_2)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 st_1
@@ -1164,6 +1170,7 @@ EXPLAIN (COSTS false)
   ORDER BY st_1.c1;
 
 --No.13-4-3
+--See description for No.13-4-1
 /*+HashJoin(t_1 t_2)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 st_1
@@ -1171,6 +1178,7 @@ EXPLAIN (COSTS false)
   ORDER BY st_1.c1;
 
 --No.13-4-4
+--See description for No.13-4-1
 /*+HashJoin(st_1 st_2)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
@@ -1178,6 +1186,8 @@ EXPLAIN (COSTS false)
   ORDER BY t_1.c1;
 
 --No.13-4-5
+-- See description for No.13-4-1. No joins in ths plan, so
+-- pg_hint_plan doesn't complain on the wrongly written error hint.
 /*+HashJoin(t_1 t_1)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
@@ -1203,6 +1213,7 @@ EXPLAIN (COSTS false)
 DROP FUNCTION recall_planner_one_t(int);
 
 --No.13-4-7
+-- See description for No.13-4-1. Complains on the wrongly written hint.
 /*+HashJoin(t_1 t_1)*/
 EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
@@ -1215,3 +1226,27 @@ EXPLAIN (COSTS false)
  SELECT recall_planner() FROM s1.t1 t_1
    JOIN s1.t2 t_2 ON (t_1.c1 = t_2.c1)
   ORDER BY t_1.c1;
+
+--No.14-1-1 plancache invalidation
+CREATE TABLE s1.tpc AS SELECT a FROM generate_series(0, 999) a;
+CREATE INDEX ON s1.tpc(a);
+PREPARE p1 AS SELECT * FROM s1.tpc WHERE a < 999;
+/*+ IndexScan(tpc) */PREPARE p2 AS SELECT * FROM s1.tpc WHERE a < 999;
+/*+ SeqScan(tpc) */PREPARE p3(int) AS SELECT * FROM s1.tpc WHERE a = $1;
+EXPLAIN EXECUTE p1;
+EXPLAIN EXECUTE p2;
+EXPLAIN EXECUTE p3(500);
+-- The DROP invalidates the plan caches
+DROP TABLE s1.tpc;
+EXPLAIN EXECUTE p1;
+EXPLAIN EXECUTE p2;
+EXPLAIN EXECUTE p3(500);
+CREATE TABLE s1.tpc AS SELECT a FROM generate_series(0, 999) a;
+CREATE INDEX ON s1.tpc(a);
+EXPLAIN EXECUTE p1;
+EXPLAIN EXECUTE p2;
+EXPLAIN EXECUTE p3(500);
+DEALLOCATE p1;
+DEALLOCATE p2;
+DEALLOCATE p3;
+DROP TABLE s1.tpc;