Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 9c2ab5f

Browse filesBrowse 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
Copy full SHA for 9c2ab5f

11 files changed

+164
-103
lines changed

‎Makefile

Copy file name to clipboardExpand all lines: Makefile
+1Lines changed: 1 addition & 0 deletions
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 \

‎expected/pathman_basic.out

Copy file name to clipboardExpand all lines: expected/pathman_basic.out
-58Lines changed: 0 additions & 58 deletions
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;

‎expected/pathman_cache_pranks.out

Copy file name to clipboard
+75Lines changed: 75 additions & 0 deletions
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;

‎sql/pathman_basic.sql

Copy file name to clipboardExpand all lines: sql/pathman_basic.sql
-31Lines changed: 0 additions & 31 deletions
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;

‎sql/pathman_cache_pranks.sql

Copy file name to clipboard
+49Lines changed: 49 additions & 0 deletions
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;

‎src/hooks.c

Copy file name to clipboardExpand all lines: src/hooks.c
+18-2Lines changed: 18 additions & 2 deletions
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
}

‎src/include/init.h

Copy file name to clipboardExpand all lines: src/include/init.h
+1-1Lines changed: 1 addition & 1 deletion
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

‎src/init.c

Copy file name to clipboardExpand all lines: src/init.c
+8-1Lines changed: 8 additions & 1 deletion
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
/*

‎src/pg_pathman.c

Copy file name to clipboardExpand all lines: src/pg_pathman.c
+4-9Lines changed: 4 additions & 9 deletions
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;

‎src/pl_funcs.c

Copy file name to clipboardExpand all lines: src/pl_funcs.c
+1-1Lines changed: 1 addition & 1 deletion
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 */

‎src/relation_info.c

Copy file name to clipboardExpand all lines: src/relation_info.c
+7Lines changed: 7 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.