From c4c0e34a6cc74cb8c455c1f26582883457e16630 Mon Sep 17 00:00:00 2001 From: Koval Dmitry Date: Wed, 22 Mar 2023 10:06:36 +0300 Subject: [PATCH] [PGPRO-7928] Variable pg_pathman.enable must be called before any query Tags: pg_pathman --- expected/pathman_runtime_nodes.out | 41 ++++++++++++++++++++++++---- expected/pathman_runtime_nodes_1.out | 41 ++++++++++++++++++++++++---- sql/pathman_runtime_nodes.sql | 32 ++++++++++++++++++---- src/hooks.c | 24 +++++++++++++++- src/include/hooks.h | 1 + src/init.c | 2 +- 6 files changed, 123 insertions(+), 18 deletions(-) diff --git a/expected/pathman_runtime_nodes.out b/expected/pathman_runtime_nodes.out index 17905e59..5d3b5638 100644 --- a/expected/pathman_runtime_nodes.out +++ b/expected/pathman_runtime_nodes.out @@ -58,7 +58,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; create or replace function test.pathman_test_2() returns text as $$ @@ -100,7 +99,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; create or replace function test.pathman_test_3() returns text as $$ @@ -133,7 +131,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; create or replace function test.pathman_test_4() returns text as $$ @@ -172,7 +169,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; create or replace function test.pathman_test_5() returns text as $$ @@ -233,7 +229,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_hashjoin = off set enable_mergejoin = off; create table test.run_values as select generate_series(1, 10000) val; @@ -464,5 +459,41 @@ DROP FUNCTION test.pathman_test_3(); DROP FUNCTION test.pathman_test_4(); DROP FUNCTION test.pathman_test_5(); DROP SCHEMA test; +-- +-- +-- PGPRO-7928 +-- Variable pg_pathman.enable must be called before any query. +-- +CREATE TABLE part_test (val int NOT NULL); +SELECT create_hash_partitions('part_test', 'val', 2, partition_names := array['part_test_1','pg_pathman']); +ERROR: function create_hash_partitions(unknown, unknown, integer, partition_names => text[]) does not exist at character 8 +CREATE OR REPLACE FUNCTION part_test_trigger() RETURNS TRIGGER AS $$ +BEGIN + RAISE NOTICE '%', format('%s %s %s (%s)', TG_WHEN, TG_OP, TG_LEVEL, TG_TABLE_NAME); + IF TG_OP::text = 'DELETE'::text then + SET pg_pathman.enable = f; + RETURN new; + END IF; +END; +$$ LANGUAGE PLPGSQL; +SET pg_pathman.enable_partitionrouter = t; +CREATE TRIGGER ad AFTER DELETE ON part_test_1 FOR EACH ROW EXECUTE PROCEDURE part_test_trigger (); +ERROR: relation "part_test_1" does not exist +INSERT INTO part_test VALUES (1); +UPDATE part_test SET val = val + 1 RETURNING *, tableoid::regclass; + val | tableoid +-----+----------- + 2 | part_test +(1 row) + +UPDATE part_test SET val = val + 1 RETURNING *, tableoid::regclass; + val | tableoid +-----+----------- + 3 | part_test +(1 row) + +RESET pg_pathman.enable_partitionrouter; +DROP TABLE part_test CASCADE; +DROP FUNCTION part_test_trigger(); DROP EXTENSION pg_pathman CASCADE; DROP SCHEMA pathman; diff --git a/expected/pathman_runtime_nodes_1.out b/expected/pathman_runtime_nodes_1.out index 65382269..10435240 100644 --- a/expected/pathman_runtime_nodes_1.out +++ b/expected/pathman_runtime_nodes_1.out @@ -58,7 +58,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; create or replace function test.pathman_test_2() returns text as $$ @@ -100,7 +99,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; create or replace function test.pathman_test_3() returns text as $$ @@ -133,7 +131,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; create or replace function test.pathman_test_4() returns text as $$ @@ -172,7 +169,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; create or replace function test.pathman_test_5() returns text as $$ @@ -233,7 +229,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_hashjoin = off set enable_mergejoin = off; create table test.run_values as select generate_series(1, 10000) val; @@ -464,5 +459,41 @@ DROP FUNCTION test.pathman_test_3(); DROP FUNCTION test.pathman_test_4(); DROP FUNCTION test.pathman_test_5(); DROP SCHEMA test; +-- +-- +-- PGPRO-7928 +-- Variable pg_pathman.enable must be called before any query. +-- +CREATE TABLE part_test (val int NOT NULL); +SELECT create_hash_partitions('part_test', 'val', 2, partition_names := array['part_test_1','pg_pathman']); +ERROR: function create_hash_partitions(unknown, unknown, integer, partition_names => text[]) does not exist at character 8 +CREATE OR REPLACE FUNCTION part_test_trigger() RETURNS TRIGGER AS $$ +BEGIN + RAISE NOTICE '%', format('%s %s %s (%s)', TG_WHEN, TG_OP, TG_LEVEL, TG_TABLE_NAME); + IF TG_OP::text = 'DELETE'::text then + SET pg_pathman.enable = f; + RETURN new; + END IF; +END; +$$ LANGUAGE PLPGSQL; +SET pg_pathman.enable_partitionrouter = t; +CREATE TRIGGER ad AFTER DELETE ON part_test_1 FOR EACH ROW EXECUTE PROCEDURE part_test_trigger (); +ERROR: relation "part_test_1" does not exist +INSERT INTO part_test VALUES (1); +UPDATE part_test SET val = val + 1 RETURNING *, tableoid::regclass; + val | tableoid +-----+----------- + 2 | part_test +(1 row) + +UPDATE part_test SET val = val + 1 RETURNING *, tableoid::regclass; + val | tableoid +-----+----------- + 3 | part_test +(1 row) + +RESET pg_pathman.enable_partitionrouter; +DROP TABLE part_test CASCADE; +DROP FUNCTION part_test_trigger(); DROP EXTENSION pg_pathman CASCADE; DROP SCHEMA pathman; diff --git a/sql/pathman_runtime_nodes.sql b/sql/pathman_runtime_nodes.sql index 81c046db..9fa7028f 100644 --- a/sql/pathman_runtime_nodes.sql +++ b/sql/pathman_runtime_nodes.sql @@ -63,7 +63,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; @@ -106,7 +105,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; @@ -140,7 +138,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; @@ -180,7 +177,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_mergejoin = off set enable_hashjoin = off; @@ -242,7 +238,6 @@ begin return 'ok'; end; $$ language plpgsql -set pg_pathman.enable = true set enable_hashjoin = off set enable_mergejoin = off; @@ -347,6 +342,31 @@ DROP FUNCTION test.pathman_test_3(); DROP FUNCTION test.pathman_test_4(); DROP FUNCTION test.pathman_test_5(); DROP SCHEMA test; +-- +-- +-- PGPRO-7928 +-- Variable pg_pathman.enable must be called before any query. +-- +CREATE TABLE part_test (val int NOT NULL); +SELECT create_hash_partitions('part_test', 'val', 2, partition_names := array['part_test_1','pg_pathman']); +CREATE OR REPLACE FUNCTION part_test_trigger() RETURNS TRIGGER AS $$ +BEGIN + RAISE NOTICE '%', format('%s %s %s (%s)', TG_WHEN, TG_OP, TG_LEVEL, TG_TABLE_NAME); + IF TG_OP::text = 'DELETE'::text then + SET pg_pathman.enable = f; + RETURN new; + END IF; +END; +$$ LANGUAGE PLPGSQL; +SET pg_pathman.enable_partitionrouter = t; +CREATE TRIGGER ad AFTER DELETE ON part_test_1 FOR EACH ROW EXECUTE PROCEDURE part_test_trigger (); +INSERT INTO part_test VALUES (1); +UPDATE part_test SET val = val + 1 RETURNING *, tableoid::regclass; +UPDATE part_test SET val = val + 1 RETURNING *, tableoid::regclass; + +RESET pg_pathman.enable_partitionrouter; +DROP TABLE part_test CASCADE; +DROP FUNCTION part_test_trigger(); + DROP EXTENSION pg_pathman CASCADE; DROP SCHEMA pathman; - diff --git a/src/hooks.c b/src/hooks.c index 65c62494..b4ae796a 100644 --- a/src/hooks.c +++ b/src/hooks.c @@ -39,8 +39,9 @@ #include "optimizer/prep.h" #include "optimizer/restrictinfo.h" #include "rewrite/rewriteManip.h" -#include "utils/typcache.h" #include "utils/lsyscache.h" +#include "utils/typcache.h" +#include "utils/snapmgr.h" #ifdef USE_ASSERT_CHECKING @@ -614,6 +615,27 @@ pathman_rel_pathlist_hook(PlannerInfo *root, close_pathman_relation_info(prel); } +/* + * 'pg_pathman.enable' GUC check. + */ +bool +pathman_enable_check_hook(bool *newval, void **extra, GucSource source) +{ + if (FirstSnapshotSet || + GetTopTransactionIdIfAny() != InvalidTransactionId || +#ifdef PGPRO_EE + getNestLevelATX() > 0 || +#endif + IsSubTransaction()) + { + GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); + GUC_check_errmsg("\"pg_pathman.enable\" must be called before any query"); + return false; + } + + return true; +} + /* * Intercept 'pg_pathman.enable' GUC assignments. */ diff --git a/src/include/hooks.h b/src/include/hooks.h index 813d1342..4d426f5a 100644 --- a/src/include/hooks.h +++ b/src/include/hooks.h @@ -44,6 +44,7 @@ void pathman_rel_pathlist_hook(PlannerInfo *root, RangeTblEntry *rte); void pathman_enable_assign_hook(bool newval, void *extra); +bool pathman_enable_check_hook(bool *newval, void **extra, GucSource source); PlannedStmt * pathman_planner_hook(Query *parse, #if PG_VERSION_NUM >= 130000 diff --git a/src/init.c b/src/init.c index bdec28fd..4341d406 100644 --- a/src/init.c +++ b/src/init.c @@ -166,7 +166,7 @@ init_main_pathman_toggles(void) DEFAULT_PATHMAN_ENABLE, PGC_SUSET, 0, - NULL, + pathman_enable_check_hook, pathman_enable_assign_hook, NULL);