Skip to content

Commit 9c2ab5f

Browse files
arssherarssher
committed
Unify unload_config and DisablePathman and do it (purge caches) on enable = false.
Probably there used to be an idea that with 'set pg_pathman = false' you can just temporary disable planning hooks etc without resetting pathman caches, but that never worked properly. At least, pathman_relcache_hook refuses to inval cache if it is disabled. Also, if extension is disabled we never get to unload_config, which means you could - disable pathman (guc disabled, but caches not destroyed) - drop extension (still caches are here) - create it back again, now cached relids are wrong With some care we could totally separate them by maintaining caches even when pathman is disabled, but is it worth it?
1 parent 3556ae3 commit 9c2ab5f

11 files changed

+164
-103
lines changed

Diff for: ‎Makefile

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ PGFILEDESC = "pg_pathman - partitioning tool for PostgreSQL"
3333
REGRESS = pathman_array_qual \
3434
pathman_basic \
3535
pathman_bgw \
36+
pathman_cache_pranks \
3637
pathman_calamity \
3738
pathman_callbacks \
3839
pathman_column_type \

Diff for: ‎expected/pathman_basic.out

-58
Original file line numberDiff line numberDiff line change
@@ -1804,64 +1804,6 @@ ORDER BY partition;
18041804

18051805
DROP TABLE test.provided_part_names CASCADE;
18061806
NOTICE: drop cascades to 2 other objects
1807-
-- is pathman (caches, in particular) strong enough to carry out this?
1808-
-- 079797e0d5
1809-
CREATE TABLE test.part_test(val serial);
1810-
INSERT INTO test.part_test SELECT generate_series(1, 30);
1811-
SELECT create_range_partitions('test.part_test', 'val', 1, 10);
1812-
create_range_partitions
1813-
-------------------------
1814-
3
1815-
(1 row)
1816-
1817-
SELECT set_interval('test.part_test', 100);
1818-
set_interval
1819-
--------------
1820-
1821-
(1 row)
1822-
1823-
DELETE FROM pathman_config WHERE partrel = 'test.part_test'::REGCLASS;
1824-
SELECT drop_partitions('test.part_test');
1825-
ERROR: table "test.part_test" has no partitions
1826-
SELECT disable_pathman_for('test.part_test');
1827-
disable_pathman_for
1828-
---------------------
1829-
1830-
(1 row)
1831-
1832-
CREATE TABLE test.wrong_partition (LIKE test.part_test) INHERITS (test.part_test);
1833-
NOTICE: merging column "val" with inherited definition
1834-
SELECT add_to_pathman_config('test.part_test', 'val', '10');
1835-
ERROR: constraint "pathman_wrong_partition_check" of partition "wrong_partition" does not exist
1836-
SELECT add_to_pathman_config('test.part_test', 'val');
1837-
ERROR: wrong constraint format for HASH partition "part_test_1"
1838-
DROP TABLE test.part_test CASCADE;
1839-
NOTICE: drop cascades to 5 other objects
1840-
--
1841-
-- 85fc5ccf121
1842-
CREATE TABLE test.part_test(val serial);
1843-
INSERT INTO test.part_test SELECT generate_series(1, 3000);
1844-
SELECT create_range_partitions('test.part_test', 'val', 1, 10);
1845-
create_range_partitions
1846-
-------------------------
1847-
300
1848-
(1 row)
1849-
1850-
SELECT append_range_partition('test.part_test');
1851-
append_range_partition
1852-
------------------------
1853-
test.part_test_301
1854-
(1 row)
1855-
1856-
DELETE FROM test.part_test;
1857-
SELECT create_single_range_partition('test.part_test', NULL::INT4, NULL); /* not ok */
1858-
ERROR: cannot create partition with range (-inf, +inf)
1859-
DELETE FROM pathman_config WHERE partrel = 'test.part_test'::REGCLASS;
1860-
SELECT create_hash_partitions('test.part_test', 'val', 2, partition_names := ARRAY[]::TEXT[]); /* not ok */
1861-
ERROR: can't partition table "test.part_test" with existing children
1862-
DROP TABLE test.part_test CASCADE;
1863-
NOTICE: drop cascades to 302 other objects
1864-
--
18651807
DROP SCHEMA test CASCADE;
18661808
NOTICE: drop cascades to 28 other objects
18671809
DROP EXTENSION pg_pathman CASCADE;

Diff for: ‎expected/pathman_cache_pranks.out

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
\set VERBOSITY terse
2+
-- is pathman (caches, in particular) strong enough to carry out this?
3+
SET search_path = 'public';
4+
-- wobble with create-drop ext: tests cached relids sanity
5+
CREATE EXTENSION pg_pathman;
6+
SET pg_pathman.enable = f;
7+
NOTICE: RuntimeAppend, RuntimeMergeAppend and PartitionFilter nodes and some other options have been disabled
8+
DROP EXTENSION pg_pathman;
9+
CREATE EXTENSION pg_pathman;
10+
SET pg_pathman.enable = true;
11+
NOTICE: RuntimeAppend, RuntimeMergeAppend and PartitionFilter nodes and some other options have been enabled
12+
DROP EXTENSION pg_pathman;
13+
CREATE EXTENSION pg_pathman;
14+
DROP EXTENSION pg_pathman;
15+
-- create it for further tests
16+
CREATE EXTENSION pg_pathman;
17+
-- 079797e0d5
18+
CREATE TABLE part_test(val serial);
19+
INSERT INTO part_test SELECT generate_series(1, 30);
20+
SELECT create_range_partitions('part_test', 'val', 1, 10);
21+
create_range_partitions
22+
-------------------------
23+
3
24+
(1 row)
25+
26+
SELECT set_interval('part_test', 100);
27+
set_interval
28+
--------------
29+
30+
(1 row)
31+
32+
DELETE FROM pathman_config WHERE partrel = 'part_test'::REGCLASS;
33+
SELECT drop_partitions('part_test');
34+
ERROR: table "part_test" has no partitions
35+
SELECT disable_pathman_for('part_test');
36+
disable_pathman_for
37+
---------------------
38+
39+
(1 row)
40+
41+
CREATE TABLE wrong_partition (LIKE part_test) INHERITS (part_test);
42+
NOTICE: merging column "val" with inherited definition
43+
SELECT add_to_pathman_config('part_test', 'val', '10');
44+
ERROR: constraint "pathman_wrong_partition_check" of partition "wrong_partition" does not exist
45+
SELECT add_to_pathman_config('part_test', 'val');
46+
ERROR: wrong constraint format for HASH partition "part_test_1"
47+
DROP TABLE part_test CASCADE;
48+
NOTICE: drop cascades to 5 other objects
49+
--
50+
-- 85fc5ccf121
51+
CREATE TABLE part_test(val serial);
52+
INSERT INTO part_test SELECT generate_series(1, 3000);
53+
SELECT create_range_partitions('part_test', 'val', 1, 10);
54+
create_range_partitions
55+
-------------------------
56+
300
57+
(1 row)
58+
59+
SELECT append_range_partition('part_test');
60+
append_range_partition
61+
------------------------
62+
part_test_301
63+
(1 row)
64+
65+
DELETE FROM part_test;
66+
SELECT create_single_range_partition('part_test', NULL::INT4, NULL); /* not ok */
67+
ERROR: cannot create partition with range (-inf, +inf)
68+
DELETE FROM pathman_config WHERE partrel = 'part_test'::REGCLASS;
69+
SELECT create_hash_partitions('part_test', 'val', 2, partition_names := ARRAY[]::TEXT[]); /* not ok */
70+
ERROR: can't partition table "part_test" with existing children
71+
DROP TABLE part_test CASCADE;
72+
NOTICE: drop cascades to 302 other objects
73+
--
74+
-- finalize
75+
DROP EXTENSION pg_pathman;

Diff for: ‎sql/pathman_basic.sql

-31
Original file line numberDiff line numberDiff line change
@@ -546,37 +546,6 @@ ORDER BY partition;
546546

547547
DROP TABLE test.provided_part_names CASCADE;
548548

549-
-- is pathman (caches, in particular) strong enough to carry out this?
550-
551-
-- 079797e0d5
552-
CREATE TABLE test.part_test(val serial);
553-
INSERT INTO test.part_test SELECT generate_series(1, 30);
554-
SELECT create_range_partitions('test.part_test', 'val', 1, 10);
555-
SELECT set_interval('test.part_test', 100);
556-
DELETE FROM pathman_config WHERE partrel = 'test.part_test'::REGCLASS;
557-
SELECT drop_partitions('test.part_test');
558-
SELECT disable_pathman_for('test.part_test');
559-
560-
CREATE TABLE test.wrong_partition (LIKE test.part_test) INHERITS (test.part_test);
561-
SELECT add_to_pathman_config('test.part_test', 'val', '10');
562-
SELECT add_to_pathman_config('test.part_test', 'val');
563-
564-
DROP TABLE test.part_test CASCADE;
565-
--
566-
567-
-- 85fc5ccf121
568-
CREATE TABLE test.part_test(val serial);
569-
INSERT INTO test.part_test SELECT generate_series(1, 3000);
570-
SELECT create_range_partitions('test.part_test', 'val', 1, 10);
571-
SELECT append_range_partition('test.part_test');
572-
DELETE FROM test.part_test;
573-
SELECT create_single_range_partition('test.part_test', NULL::INT4, NULL); /* not ok */
574-
DELETE FROM pathman_config WHERE partrel = 'test.part_test'::REGCLASS;
575-
SELECT create_hash_partitions('test.part_test', 'val', 2, partition_names := ARRAY[]::TEXT[]); /* not ok */
576-
577-
DROP TABLE test.part_test CASCADE;
578-
--
579-
580549
DROP SCHEMA test CASCADE;
581550
DROP EXTENSION pg_pathman CASCADE;
582551
DROP SCHEMA pathman CASCADE;

Diff for: ‎sql/pathman_cache_pranks.sql

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
\set VERBOSITY terse
2+
-- is pathman (caches, in particular) strong enough to carry out this?
3+
4+
SET search_path = 'public';
5+
6+
-- wobble with create-drop ext: tests cached relids sanity
7+
CREATE EXTENSION pg_pathman;
8+
SET pg_pathman.enable = f;
9+
DROP EXTENSION pg_pathman;
10+
CREATE EXTENSION pg_pathman;
11+
SET pg_pathman.enable = true;
12+
DROP EXTENSION pg_pathman;
13+
CREATE EXTENSION pg_pathman;
14+
DROP EXTENSION pg_pathman;
15+
16+
-- create it for further tests
17+
CREATE EXTENSION pg_pathman;
18+
19+
-- 079797e0d5
20+
CREATE TABLE part_test(val serial);
21+
INSERT INTO part_test SELECT generate_series(1, 30);
22+
SELECT create_range_partitions('part_test', 'val', 1, 10);
23+
SELECT set_interval('part_test', 100);
24+
DELETE FROM pathman_config WHERE partrel = 'part_test'::REGCLASS;
25+
SELECT drop_partitions('part_test');
26+
SELECT disable_pathman_for('part_test');
27+
28+
CREATE TABLE wrong_partition (LIKE part_test) INHERITS (part_test);
29+
SELECT add_to_pathman_config('part_test', 'val', '10');
30+
SELECT add_to_pathman_config('part_test', 'val');
31+
32+
DROP TABLE part_test CASCADE;
33+
--
34+
35+
-- 85fc5ccf121
36+
CREATE TABLE part_test(val serial);
37+
INSERT INTO part_test SELECT generate_series(1, 3000);
38+
SELECT create_range_partitions('part_test', 'val', 1, 10);
39+
SELECT append_range_partition('part_test');
40+
DELETE FROM part_test;
41+
SELECT create_single_range_partition('part_test', NULL::INT4, NULL); /* not ok */
42+
DELETE FROM pathman_config WHERE partrel = 'part_test'::REGCLASS;
43+
SELECT create_hash_partitions('part_test', 'val', 2, partition_names := ARRAY[]::TEXT[]); /* not ok */
44+
45+
DROP TABLE part_test CASCADE;
46+
--
47+
48+
-- finalize
49+
DROP EXTENSION pg_pathman;

Diff for: ‎src/hooks.c

+18-2
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,12 @@ pathman_enable_assign_hook(bool newval, void *extra)
615615
"RuntimeAppend, RuntimeMergeAppend and PartitionFilter nodes "
616616
"and some other options have been %s",
617617
newval ? "enabled" : "disabled");
618+
619+
/* Purge caches if pathman was disabled */
620+
if (!newval)
621+
{
622+
unload_config();
623+
}
618624
}
619625

620626
static void
@@ -850,6 +856,8 @@ pathman_shmem_startup_hook(void)
850856
void
851857
pathman_relcache_hook(Datum arg, Oid relid)
852858
{
859+
Oid pathman_config_relid;
860+
853861
/* See cook_partitioning_expression() */
854862
if (!pathman_hooks_enabled)
855863
return;
@@ -863,10 +871,18 @@ pathman_relcache_hook(Datum arg, Oid relid)
863871
invalidate_bounds_cache();
864872
invalidate_parents_cache();
865873
invalidate_status_cache();
874+
delay_pathman_shutdown(); /* see below */
866875
}
867876

868-
/* Invalidation event for PATHMAN_CONFIG table (probably DROP) */
869-
if (relid == get_pathman_config_relid(false))
877+
/*
878+
* Invalidation event for PATHMAN_CONFIG table (probably DROP EXTENSION).
879+
* Digging catalogs here is expensive and probably illegal, so we take
880+
* cached relid. It is possible that we don't know it atm (e.g. pathman
881+
* was disabled). However, in this case caches must have been cleaned
882+
* on disable, and there is no DROP-specific additional actions.
883+
*/
884+
pathman_config_relid = get_pathman_config_relid(true);
885+
if (relid == pathman_config_relid)
870886
{
871887
delay_pathman_shutdown();
872888
}

Diff for: ‎src/include/init.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ simplify_mcxt_name(MemoryContext mcxt)
139139
pathman_init_state.pg_pathman_enable = false; \
140140
pathman_init_state.auto_partition = false; \
141141
pathman_init_state.override_copy = false; \
142-
pathman_init_state.initialization_needed = true; \
142+
unload_config(); \
143143
} while (0)
144144

145145

Diff for: ‎src/init.c

+8-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,14 @@ save_pathman_init_state(PathmanInitState *temp_init_state)
134134
void
135135
restore_pathman_init_state(const PathmanInitState *temp_init_state)
136136
{
137-
pathman_init_state = *temp_init_state;
137+
/*
138+
* initialization_needed is not restored: it is not just a setting but
139+
* internal thing, caches must be inited when it is set. Better would be
140+
* to separate it from this struct entirely.
141+
*/
142+
pathman_init_state.pg_pathman_enable = temp_init_state->pg_pathman_enable;
143+
pathman_init_state.auto_partition = temp_init_state->auto_partition;
144+
pathman_init_state.override_copy = temp_init_state->override_copy;
138145
}
139146

140147
/*

Diff for: ‎src/pg_pathman.c

+4-9
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,6 @@ estimate_paramsel_using_prel(const PartRelationInfo *prel, int strategy)
284284
void
285285
_PG_init(void)
286286
{
287-
PathmanInitState temp_init_state;
288-
289287
if (!process_shared_preload_libraries_in_progress)
290288
{
291289
elog(ERROR, "pg_pathman module must be initialized by Postmaster. "
@@ -297,13 +295,10 @@ _PG_init(void)
297295
RequestAddinShmemSpace(estimate_pathman_shmem_size());
298296

299297
/* Assign pg_pathman's initial state */
300-
temp_init_state.pg_pathman_enable = DEFAULT_PATHMAN_ENABLE;
301-
temp_init_state.auto_partition = DEFAULT_PATHMAN_AUTO;
302-
temp_init_state.override_copy = DEFAULT_PATHMAN_OVERRIDE_COPY;
303-
temp_init_state.initialization_needed = true; /* ofc it's needed! */
304-
305-
/* Apply initial state */
306-
restore_pathman_init_state(&temp_init_state);
298+
pathman_init_state.pg_pathman_enable = DEFAULT_PATHMAN_ENABLE;
299+
pathman_init_state.auto_partition = DEFAULT_PATHMAN_AUTO;
300+
pathman_init_state.override_copy = DEFAULT_PATHMAN_OVERRIDE_COPY;
301+
pathman_init_state.initialization_needed = true; /* ofc it's needed! */
307302

308303
/* Set basic hooks */
309304
pathman_set_rel_pathlist_hook_next = set_rel_pathlist_hook;

Diff for: ‎src/pl_funcs.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -859,7 +859,7 @@ add_to_pathman_config(PG_FUNCTION_ARGS)
859859
}
860860
PG_CATCH();
861861
{
862-
/* We have to restore all changed flags */
862+
/* We have to restore changed flags */
863863
restore_pathman_init_state(&init_state);
864864

865865
/* Rethrow ERROR */

Diff for: ‎src/relation_info.c

+7
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,13 @@ build_pathman_relation_info(Oid relid, Datum *values)
501501
* cache now might have obsolete data for something that probably is
502502
* not a partitioned table at all. Remove it.
503503
*/
504+
if (!IsPathmanInitialized())
505+
/*
506+
* ... unless failure was so hard that caches were already destoyed,
507+
* i.e. extension disabled
508+
*/
509+
PG_RE_THROW();
510+
504511
if (prel->children != NULL)
505512
{
506513
uint32 i;

0 commit comments

Comments
 (0)