From 9d0d4e2bb4559097e7bb0d84d71274822ee2fa2c Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Mon, 17 Feb 2020 19:46:28 +0900 Subject: [PATCH] Restore current hint state when returned from non-hinted query planning. If no hint is given for the current level query, pg_hint_plan_planner calls the next level of planner after erasing the current_hint_state. But it forgot to restore the state before the planning of the rest part of the current-level query. It is (a-kind-of) broken by the commit d422966 but overlooked as an inevitable side-effect of the fix. Get back the behavior by restoring current_hint_state after returned from the lower level planner with unhinted query. Issue: https://github.com/ossc-db/pg_hint_plan/issues/30 Reported-by: higuchi-daisuke --- expected/ut-A.out | 25 +++++++++++++++++++------ expected/ut-S.out | 4 ++-- pg_hint_plan.c | 10 ++++++++-- sql/ut-A.sql | 8 +++++++- 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/expected/ut-A.out b/expected/ut-A.out index 86134a9..d0f4f37 100644 --- a/expected/ut-A.out +++ b/expected/ut-A.out @@ -4338,6 +4338,9 @@ BEGIN RETURN new_cnt; END; $$ LANGUAGE plpgsql IMMUTABLE; +-- The function called at the bottom desn't use a hint, the immediate +-- caller level should restore its own hint. So, the first LOG from +-- pg_hint_plan should use the IndexScan(t_1) hint EXPLAIN (COSTS false) SELECT nested_planner(5) FROM s1.t1 t_1 ORDER BY t_1.c1; NOTICE: nested_planner(5) NOTICE: nested_planner(4) @@ -4345,7 +4348,12 @@ NOTICE: nested_planner(3) NOTICE: nested_planner(2) NOTICE: nested_planner(1) LOG: pg_hint_plan: -no hint +used hint: +IndexScan(t_1) +not used hint: +duplication hint: +error hint: + LOG: pg_hint_plan: used hint: IndexScan(t_1) @@ -4372,7 +4380,9 @@ error hint: Index Only Scan using t1_i1 on t1 t_1 (1 row) -/*+SeqScan(t_2)*/ +-- The top level uses SeqScan(t_1), but the function should use only +-- the hint in the function. +/*+SeqScan(t_1) SeqScan(t_2)*/ EXPLAIN (COSTS false) SELECT nested_planner(5) FROM s1.t1 t_1 ORDER BY t_1.c1; NOTICE: nested_planner(5) NOTICE: nested_planner(4) @@ -4409,15 +4419,18 @@ error hint: LOG: pg_hint_plan: used hint: +SeqScan(t_1) not used hint: SeqScan(t_2) duplication hint: error hint: - QUERY PLAN ---------------------------------------- - Index Only Scan using t1_i1 on t1 t_1 -(1 row) + QUERY PLAN +-------------------------- + Sort + Sort Key: c1 + -> Seq Scan on t1 t_1 +(3 rows) ---- ---- No. A-13-4 output of debugging log on hint status diff --git a/expected/ut-S.out b/expected/ut-S.out index 7ab53b2..442a606 100644 --- a/expected/ut-S.out +++ b/expected/ut-S.out @@ -3761,9 +3761,8 @@ error hint: c4 | text | | | Indexes: "ti1_pkey" PRIMARY KEY, btree (c1) - "ti1_c2_key" UNIQUE CONSTRAINT, btree (c2) - "ti1_uniq" UNIQUE, btree (c1) "ti1_btree" btree (c1) + "ti1_c2_key" UNIQUE CONSTRAINT, btree (c2) "ti1_expr" btree ((c1 < 100)) "ti1_gin" gin (c1) "ti1_gist" gist (c1) @@ -3775,6 +3774,7 @@ Indexes: "ti1_multi" btree (c1, c2, c3, c4) "ti1_pred" btree (lower(c4)) "ti1_ts" gin (to_tsvector('english'::regconfig, c4)) + "ti1_uniq" UNIQUE, btree (c1) EXPLAIN (COSTS false) SELECT * FROM s1.ti1 WHERE c1 < 100 AND c2 = 1 AND lower(c4) = '1' AND to_tsvector('english', c4) @@ 'a & b' AND ctid = '(1,1)'; QUERY PLAN diff --git a/pg_hint_plan.c b/pg_hint_plan.c index 841499f..7b7f5b5 100644 --- a/pg_hint_plan.c +++ b/pg_hint_plan.c @@ -3201,9 +3201,15 @@ standard_planner_proc: } current_hint_state = NULL; if (prev_planner) - return (*prev_planner) (parse, cursorOptions, boundParams); + result = (*prev_planner) (parse, cursorOptions, boundParams); else - return standard_planner(parse, cursorOptions, boundParams); + result = standard_planner(parse, cursorOptions, boundParams); + + /* The upper-level planner still needs the current hint state */ + if (HintStateStack != NIL) + current_hint_state = (HintState *) lfirst(list_head(HintStateStack)); + + return result; } /* diff --git a/sql/ut-A.sql b/sql/ut-A.sql index 2b601d7..346f18f 100644 --- a/sql/ut-A.sql +++ b/sql/ut-A.sql @@ -1137,8 +1137,14 @@ BEGIN END; $$ LANGUAGE plpgsql IMMUTABLE; +-- The function called at the bottom desn't use a hint, the immediate +-- caller level should restore its own hint. So, the first LOG from +-- pg_hint_plan should use the IndexScan(t_1) hint EXPLAIN (COSTS false) SELECT nested_planner(5) FROM s1.t1 t_1 ORDER BY t_1.c1; -/*+SeqScan(t_2)*/ + +-- The top level uses SeqScan(t_1), but the function should use only +-- the hint in the function. +/*+SeqScan(t_1) SeqScan(t_2)*/ EXPLAIN (COSTS false) SELECT nested_planner(5) FROM s1.t1 t_1 ORDER BY t_1.c1; ---- -- 2.11.0