Commit: de449945fe23b70d284a2240085620b708d0ef57 - amalsyah12 (git)

firtst release

Commit MetaInfo

• Revision: de449945fe23b70d284a2240085620b708d0ef57 (tree)
• Time: 2017-07-27 19:19:57
• Author: Kyotaro Horiguchi <horiguchi.kyotaro@lab....>
• Commiter: Kyotaro Horiguchi

Log Message

Fixed a crash bug by DECLARE CURSOR and enable_hint_table = on

The previous version causes assertion failure by DECLARE CURSOR syntax
when table hint is activated. The cause is that the version forgot the
fact that DelcareCursorStmt is in a bit strange shape. Add support of
DECLARE CURSOR and regression test for table hinting.

Change Summary

• modified: Makefile
• add: expected/ut-T.out
• modified: pg_hint_plan.c
• add: sql/ut-T.sql

Incremental Difference

--- a/Makefile
+++ b/Makefile
@@ -7,7 +7,7 @@
 MODULES = pg_hint_plan
 HINTPLANVER = 1.1.4
 
-REGRESS = init base_plan pg_hint_plan ut-init ut-A ut-S ut-J ut-L ut-G ut-R ut-fdw ut-fini
+REGRESS = init base_plan pg_hint_plan ut-init ut-A ut-S ut-J ut-L ut-G ut-R ut-fdw ut-T ut-fini
 
 REGRESSION_EXPECTED = expected/init.out expected/base_plan.out expected/pg_hint_plan.out expected/ut-A.out expected/ut-S.out expected/ut-J.out expected/ut-L.out expected/ut-G.out
 

--- /dev/null
+++ b/expected/ut-T.out
@@ -0,0 +1,131 @@
+-- ut-T: tests for table hints
+-- This test is focusing on hint retrieval from table
+LOAD 'pg_hint_plan';
+SET pg_hint_plan.enable_hint TO on;
+SET pg_hint_plan.debug_print TO on;
+SET client_min_messages TO LOG;
+SET search_path TO public;
+-- test for get_query_string
+INSERT INTO hint_plan.hints VALUES(DEFAULT,'EXPLAIN SELECT * FROM t1 WHERE id = ?;', '', 'SeqScan(t1)');
+INSERT INTO hint_plan.hints VALUES(DEFAULT,'PREPARE p1 AS SELECT * FROM t1 WHERE id = ?;', '', 'SeqScan(t1)');
+INSERT INTO hint_plan.hints VALUES(DEFAULT,'EXPLAIN DECLARE c1 CURSOR FOR SELECT * FROM t1 WHERE id = ?;', '', 'SeqScan(t1)');
+INSERT INTO hint_plan.hints VALUES(DEFAULT,'EXPLAIN CREATE TABLE ct1 AS SELECT * FROM t1 WHERE id = ?;', '', 'SeqScan(t1)');
+PREPARE p1 AS SELECT * FROM t1 WHERE id = 100;
+-- These queries uses IndexScan without hints
+SET pg_hint_plan.enable_hint_table to off;
+EXPLAIN SELECT * FROM t1 WHERE id = 100;
+                            QUERY PLAN                            
+------------------------------------------------------------------
+ Index Scan using t1_pkey on t1  (cost=0.29..8.30 rows=1 width=8)
+   Index Cond: (id = 100)
+(2 rows)
+
+EXPLAIN DECLARE c1 CURSOR FOR SELECT * FROM t1 WHERE id = 100;
+                            QUERY PLAN                            
+------------------------------------------------------------------
+ Index Scan using t1_pkey on t1  (cost=0.29..8.30 rows=1 width=8)
+   Index Cond: (id = 100)
+(2 rows)
+
+EXPLAIN CREATE TABLE ct1 AS SELECT * FROM t1 WHERE id = 100;
+                            QUERY PLAN                            
+------------------------------------------------------------------
+ Index Scan using t1_pkey on t1  (cost=0.29..8.30 rows=1 width=8)
+   Index Cond: (id = 100)
+(2 rows)
+
+EXPLAIN EXECUTE p1;
+                            QUERY PLAN                            
+------------------------------------------------------------------
+ Index Scan using t1_pkey on t1  (cost=0.29..8.30 rows=1 width=8)
+   Index Cond: (id = 100)
+(2 rows)
+
+DEALLOCATE p1;
+PREPARE p1 AS SELECT * FROM t1 WHERE id = 100;
+EXPLAIN CREATE TABLE ct1 AS EXECUTE p1;
+                            QUERY PLAN                            
+------------------------------------------------------------------
+ Index Scan using t1_pkey on t1  (cost=0.29..8.30 rows=1 width=8)
+   Index Cond: (id = 100)
+(2 rows)
+
+DEALLOCATE p1;
+PREPARE p1 AS SELECT * FROM t1 WHERE id = 100;
+-- Forced to use SeqScan by table hints
+SET pg_hint_plan.enable_hint_table to on;
+EXPLAIN SELECT * FROM t1 WHERE id = 100;
+LOG:  pg_hint_plan:
+used hint:
+SeqScan(t1)
+not used hint:
+duplication hint:
+error hint:
+
+                     QUERY PLAN                     
+----------------------------------------------------
+ Seq Scan on t1  (cost=0.00..170.00 rows=1 width=8)
+   Filter: (id = 100)
+(2 rows)
+
+EXPLAIN DECLARE c1 CURSOR FOR SELECT * FROM t1 WHERE id = 100;
+LOG:  pg_hint_plan:
+used hint:
+SeqScan(t1)
+not used hint:
+duplication hint:
+error hint:
+
+                     QUERY PLAN                     
+----------------------------------------------------
+ Seq Scan on t1  (cost=0.00..170.00 rows=1 width=8)
+   Filter: (id = 100)
+(2 rows)
+
+EXPLAIN CREATE TABLE ct1 AS SELECT * FROM t1 WHERE id = 100;
+LOG:  pg_hint_plan:
+used hint:
+SeqScan(t1)
+not used hint:
+duplication hint:
+error hint:
+
+                     QUERY PLAN                     
+----------------------------------------------------
+ Seq Scan on t1  (cost=0.00..170.00 rows=1 width=8)
+   Filter: (id = 100)
+(2 rows)
+
+EXPLAIN EXECUTE p1;
+LOG:  pg_hint_plan:
+used hint:
+SeqScan(t1)
+not used hint:
+duplication hint:
+error hint:
+
+                     QUERY PLAN                     
+----------------------------------------------------
+ Seq Scan on t1  (cost=0.00..170.00 rows=1 width=8)
+   Filter: (id = 100)
+(2 rows)
+
+DEALLOCATE p1;
+PREPARE p1 AS SELECT * FROM t1 WHERE id = 100;
+EXPLAIN CREATE TABLE ct1 AS EXECUTE p1;
+LOG:  pg_hint_plan:
+used hint:
+SeqScan(t1)
+not used hint:
+duplication hint:
+error hint:
+
+                     QUERY PLAN                     
+----------------------------------------------------
+ Seq Scan on t1  (cost=0.00..170.00 rows=1 width=8)
+   Filter: (id = 100)
+(2 rows)
+
+DEALLOCATE p1;
+SET pg_hint_plan.enable_hint_table to off;
+DELETE FROM hint_plan.hints;

--- a/pg_hint_plan.c
+++ b/pg_hint_plan.c
@@ -1650,8 +1650,9 @@ get_hints_from_table(const char *client_query, const char *client_application)
 /*
  * Get client-supplied query string. Addtion to that the jumbled query is
  * supplied if the caller requested. From the restriction of JumbleQuery, some
- * kind of query needs special amendments. Reutrns NULL if the current hint
- * string is still valid.
+ * kind of query needs special amendments. Reutrns NULL if this query doesn't
+ * change the current hint. This function returns NULL also when something
+ * wrong has happend and let the caller continue using the current hints.
  */
 static const char *
 get_query_string(ParseState *pstate, Query *query, Query **jumblequery)
@@ -1661,15 +1662,22 @@ get_query_string(ParseState *pstate, Query *query, Query **jumblequery)
 	if (jumblequery != NULL)
 		*jumblequery = query;
 
-	if (query->commandType == CMD_UTILITY)
+	/* Query for DeclareCursorStmt is CMD_SELECT and has query->utilityStmt */
+	if (query->commandType == CMD_UTILITY || query->utilityStmt)
 	{
 		Query *target_query = query;
 
-		/* Use the target query if EXPLAIN */
-		if (IsA(query->utilityStmt, ExplainStmt))
+		/*
+		 * Some utility statements have a subquery that we can hint on.  Since
+		 * EXPLAIN can be placed before other kind of utility statements and
+		 * EXECUTE can be contained other kind of utility statements, these
+		 * conditions are not mutually exclusive and should be considered in
+		 * this order.
+		 */
+		if (IsA(target_query->utilityStmt, ExplainStmt))
 		{
-			ExplainStmt *stmt = (ExplainStmt *)(query->utilityStmt);
-
+			ExplainStmt *stmt = (ExplainStmt *)target_query->utilityStmt;
+			
 			Assert(IsA(stmt->query, Query));
 			target_query = (Query *)stmt->query;
 
@@ -1679,34 +1687,43 @@ get_query_string(ParseState *pstate, Query *query, Query **jumblequery)
 				target_query = (Query *)target_query->utilityStmt;
 		}
 
-		if (IsA(target_query, CreateTableAsStmt))
+		/*
+		 * JumbleQuery does  not accept  a Query that  has utilityStmt.  On the
+		 * other  hand DeclareCursorStmt  is in  a  bit strange  shape that  is
+		 * flipped upside down.
+		 */
+ 		if (IsA(target_query, Query) &&
+			target_query->utilityStmt &&
+			IsA(target_query->utilityStmt, DeclareCursorStmt))
 		{
 			/*
-			 * Use the the body query for CREATE AS. The Query for jumble also
-			 * replaced with the corresponding one.
+			 * The given Query cannot be modified so copy it and modify so that
+			 * JumbleQuery can accept it.
 			 */
+			Assert(IsA(target_query, Query) &&
+				   target_query->commandType == CMD_SELECT);
+			target_query = copyObject(target_query);
+			target_query->utilityStmt = NULL;
+		}
+
+		if (IsA(target_query, CreateTableAsStmt))
+		{
 			CreateTableAsStmt  *stmt = (CreateTableAsStmt *) target_query;
-			PreparedStatement  *entry;
-			Query			   *tmp_query;
 
 			Assert(IsA(stmt->query, Query));
-			tmp_query = (Query *) stmt->query;
+			target_query = (Query *) stmt->query;
 
-			if (tmp_query->commandType == CMD_UTILITY &&
-				IsA(tmp_query->utilityStmt, ExecuteStmt))
-			{
-				ExecuteStmt *estmt = (ExecuteStmt *) tmp_query->utilityStmt;
-				entry = FetchPreparedStatement(estmt->name, true);
-				p = entry->plansource->query_string;
-				target_query = (Query *) linitial (entry->plansource->query_list);
-			}
+			/* strip out the top-level query for further processing */
+			if (target_query->commandType == CMD_UTILITY &&
+				target_query->utilityStmt != NULL)
+				target_query = (Query *)target_query->utilityStmt;
 		}
-		else
+
 		if (IsA(target_query, ExecuteStmt))
 		{
 			/*
-			 * Use the prepared query for EXECUTE. The Query for jumble also
-			 * replaced with the corresponding one.
+			 * Use the prepared query for EXECUTE. The Query for jumble
+			 * also replaced with the corresponding one.
 			 */
 			ExecuteStmt *stmt = (ExecuteStmt *)target_query;
 			PreparedStatement  *entry;
@@ -1715,15 +1732,16 @@ get_query_string(ParseState *pstate, Query *query, Query **jumblequery)
 			p = entry->plansource->query_string;
 			target_query = (Query *) linitial (entry->plansource->query_list);
 		}
-
-		/* We don't accept other than a Query other than a CMD_UTILITY */
+			
+		/* JumbleQuery accespts only a non-utility Query */
 		if (!IsA(target_query, Query) ||
-			target_query->commandType == CMD_UTILITY)
+			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)
 		p = NULL;
@@ -2531,7 +2549,7 @@ pg_hint_plan_post_parse_analyze(ParseState *pstate, Query *query)
 			}
 		}
 
-		/* retrun if we have hint here*/
+		/* retrun if we have hint here */
 		if (current_hint_str)
 			return;
 	}

--- /dev/null
+++ b/sql/ut-T.sql
@@ -0,0 +1,45 @@
+-- ut-T: tests for table hints
+-- This test is focusing on hint retrieval from table
+
+LOAD 'pg_hint_plan';
+SET pg_hint_plan.enable_hint TO on;
+SET pg_hint_plan.debug_print TO on;
+SET client_min_messages TO LOG;
+SET search_path TO public;
+
+-- test for get_query_string
+INSERT INTO hint_plan.hints VALUES(DEFAULT,'EXPLAIN SELECT * FROM t1 WHERE id = ?;', '', 'SeqScan(t1)');
+INSERT INTO hint_plan.hints VALUES(DEFAULT,'PREPARE p1 AS SELECT * FROM t1 WHERE id = ?;', '', 'SeqScan(t1)');
+INSERT INTO hint_plan.hints VALUES(DEFAULT,'EXPLAIN DECLARE c1 CURSOR FOR SELECT * FROM t1 WHERE id = ?;', '', 'SeqScan(t1)');
+INSERT INTO hint_plan.hints VALUES(DEFAULT,'EXPLAIN CREATE TABLE ct1 AS SELECT * FROM t1 WHERE id = ?;', '', 'SeqScan(t1)');
+
+PREPARE p1 AS SELECT * FROM t1 WHERE id = 100;
+
+-- These queries uses IndexScan without hints
+SET pg_hint_plan.enable_hint_table to off;
+EXPLAIN SELECT * FROM t1 WHERE id = 100;
+EXPLAIN DECLARE c1 CURSOR FOR SELECT * FROM t1 WHERE id = 100;
+EXPLAIN CREATE TABLE ct1 AS SELECT * FROM t1 WHERE id = 100;
+
+EXPLAIN EXECUTE p1;
+DEALLOCATE p1;
+PREPARE p1 AS SELECT * FROM t1 WHERE id = 100;
+EXPLAIN CREATE TABLE ct1 AS EXECUTE p1;
+
+DEALLOCATE p1;
+PREPARE p1 AS SELECT * FROM t1 WHERE id = 100;
+
+-- Forced to use SeqScan by table hints
+SET pg_hint_plan.enable_hint_table to on;
+EXPLAIN SELECT * FROM t1 WHERE id = 100;
+EXPLAIN DECLARE c1 CURSOR FOR SELECT * FROM t1 WHERE id = 100;
+EXPLAIN CREATE TABLE ct1 AS SELECT * FROM t1 WHERE id = 100;
+EXPLAIN EXECUTE p1;
+DEALLOCATE p1;
+PREPARE p1 AS SELECT * FROM t1 WHERE id = 100;
+EXPLAIN CREATE TABLE ct1 AS EXECUTE p1;
+
+DEALLOCATE p1;
+
+SET pg_hint_plan.enable_hint_table to off;
+DELETE FROM hint_plan.hints;