OSDN Git Service

Fixed a crash bug by DECLARE CURSOR and enable_hint_table = on
authorKyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Thu, 27 Jul 2017 03:26:55 +0000 (12:26 +0900)
committerKyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Thu, 27 Jul 2017 10:20:42 +0000 (19:20 +0900)
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.

Makefile
expected/ut-T.out [new file with mode: 0644]
pg_hint_plan.c
sql/ut-T.sql [new file with mode: 0644]

index f8e6c23..fdae55f 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -7,7 +7,7 @@
 MODULES = pg_hint_plan
 HINTPLANVER = 1.2.1
 
-REGRESS = init base_plan pg_hint_plan ut-init ut-A ut-S ut-J ut-L ut-G ut-R ut-fdw ut-W 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-W 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
 
diff --git a/expected/ut-T.out b/expected/ut-T.out
new file mode 100644 (file)
index 0000000..d5fe9e0
--- /dev/null
@@ -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;
index 1fe1cbb..3aa2604 100644 (file)
@@ -1786,8 +1786,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)
@@ -1797,15 +1798,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;
 
@@ -1815,34 +1823,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;
@@ -1851,15 +1868,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;
@@ -2849,7 +2867,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;
        }
diff --git a/sql/ut-T.sql b/sql/ut-T.sql
new file mode 100644 (file)
index 0000000..98caf68
--- /dev/null
@@ -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;