From: Kyotaro Horiguchi Date: Thu, 27 Jul 2017 03:26:55 +0000 (+0900) Subject: Fixed a crash bug by DECLARE CURSOR and enable_hint_table = on X-Git-Tag: REL95_1_1_5~1 X-Git-Url: http://git.osdn.net/view?p=pghintplan%2Fpg_hint_plan.git;a=commitdiff_plain;h=de449945fe23b70d284a2240085620b708d0ef57 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. --- diff --git a/Makefile b/Makefile index 23fe74d..51f9b10 100644 --- 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 diff --git a/expected/ut-T.out b/expected/ut-T.out new file mode 100644 index 0000000..d5fe9e0 --- /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; diff --git a/pg_hint_plan.c b/pg_hint_plan.c index 1afe86b..fa08ed3 100644 --- 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; } diff --git a/sql/ut-T.sql b/sql/ut-T.sql new file mode 100644 index 0000000..98caf68 --- /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;