Skip to content

Commit e0cfda6

Browse files
funbringerfunbringer
committed
fix incorrect assert in get_pathman_relation_info(), function drop_range_partition() can drop both foreign and local tables and preserve partition's data, insert a few otimizations into pathman_relcache_hook() and finish_delayed_invalidation()
1 parent 1d8eeb5 commit e0cfda6

File tree

7 files changed

+72
-33
lines changed

7 files changed

+72
-33
lines changed

‎README.md

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,9 @@ add_range_partition(relation REGCLASS,
165165
Create new RANGE partition for `relation` with specified range bounds.
166166

167167
```plpgsql
168-
drop_range_partition(partition TEXT)
168+
drop_range_partition(partition TEXT, delete_data BOOLEAN DEFAULT TRUE)
169169
```
170-
Drop RANGE partition and all its data.
170+
Drop RANGE partition and all of its data if `delete_data` is true.
171171

172172
```plpgsql
173173
attach_range_partition(relation REGCLASS,
@@ -221,8 +221,7 @@ CREATE TABLE IF NOT EXISTS pathman_config (
221221
parttype INTEGER NOT NULL,
222222
range_interval TEXT,
223223

224-
CHECK (parttype IN (1, 2)) /* check for allowed part types */
225-
);
224+
CHECK (parttype IN (1, 2)) /* check for allowed part types */ );
226225
```
227226
This table stores a list of partitioned tables.
228227

@@ -232,8 +231,7 @@ CREATE TABLE IF NOT EXISTS pathman_config_params (
232231
partrel REGCLASS NOT NULL PRIMARY KEY,
233232
enable_parent BOOLEAN NOT NULL DEFAULT TRUE,
234233
auto BOOLEAN NOT NULL DEFAULT TRUE,
235-
init_callback REGPROCEDURE NOT NULL DEFAULT 0
236-
);
234+
init_callback REGPROCEDURE NOT NULL DEFAULT 0);
237235
```
238236
This table stores optional parameters which override standard behavior.
239237

@@ -259,7 +257,7 @@ This view lists all currently running concurrent partitioning tasks.
259257
#### `pathman_partition_list` --- list of all existing partitions
260258
```plpgsql
261259
-- helper SRF function
262-
CREATE OR REPLACE FUNCTION @extschema@.show_partition_list()
260+
CREATE OR REPLACE FUNCTION show_partition_list()
263261
RETURNS TABLE (
264262
parent REGCLASS,
265263
partition REGCLASS,
@@ -471,7 +469,7 @@ Notice that the `Append` node contains only one child scan which corresponds to
471469
> **Important:** pay attention to the fact that `pg_pathman` excludes the parent table from the query plan.
472470
473471
To access parent table use ONLY modifier:
474-
```
472+
```plpgsql
475473
EXPLAIN SELECT * FROM ONLY items;
476474
QUERY PLAN
477475
------------------------------------------------------
@@ -484,8 +482,7 @@ CREATE TABLE journal (
484482
id SERIAL,
485483
dt TIMESTAMP NOT NULL,
486484
level INTEGER,
487-
msg TEXT
488-
);
485+
msg TEXT);
489486

490487
-- similar index will also be created for each partition
491488
CREATE INDEX ON journal(dt);
@@ -515,8 +512,8 @@ CREATE FOREIGN TABLE journal_archive (
515512
id INTEGER NOT NULL,
516513
dt TIMESTAMP NOT NULL,
517514
level INTEGER,
518-
msg TEXT
519-
) SERVER archive_server;
515+
msg TEXT)
516+
SERVER archive_server;
520517

521518
SELECT attach_range_partition('journal', 'journal_archive', '2014-01-01'::date, '2015-01-01'::date);
522519
```

‎init.sql

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ RETURNS INTEGER AS
511511
$$
512512
DECLARE
513513
v_rec RECORD;
514-
v_rows INTEGER;
514+
v_rows BIGINT;
515515
v_part_count INTEGER := 0;
516516
conf_num_del INTEGER;
517517
v_relkind CHAR;
@@ -539,10 +539,9 @@ BEGIN
539539
ORDER BY inhrelid ASC)
540540
LOOP
541541
IF NOT delete_data THEN
542-
EXECUTE format('WITH part_data AS (DELETE FROM %s RETURNING *)
543-
INSERT INTO %s SELECT * FROM part_data',
544-
v_rec.tbl::TEXT,
545-
parent_relid::text);
542+
EXECUTE format('INSERT INTO %s SELECT * FROM %s',
543+
parent_relid::TEXT,
544+
v_rec.tbl::TEXT);
546545
GET DIAGNOSTICS v_rows = ROW_COUNT;
547546

548547
/* Show number of copied rows */

‎range.sql

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,12 +1027,15 @@ LANGUAGE plpgsql;
10271027
* Drop range partition
10281028
*/
10291029
CREATE OR REPLACE FUNCTION @extschema@.drop_range_partition(
1030-
p_partition REGCLASS)
1030+
p_partition REGCLASS,
1031+
delete_data BOOLEAN DEFAULT TRUE)
10311032
RETURNS TEXT AS
10321033
$$
10331034
DECLARE
10341035
parent_relid REGCLASS;
10351036
part_name TEXT;
1037+
v_relkind CHAR;
1038+
v_rows BIGINT;
10361039

10371040
BEGIN
10381041
parent_relid := @extschema@.get_parent_of_partition(p_partition);
@@ -1041,16 +1044,39 @@ BEGIN
10411044
/* Acquire lock on parent */
10421045
PERFORM @extschema@.lock_partitioned_relation(parent_relid);
10431046

1044-
/* Drop table */
1045-
EXECUTE format('DROP TABLE %s', part_name);
1047+
IF NOT delete_data THEN
1048+
EXECUTE format('INSERT INTO %s SELECT * FROM %s',
1049+
parent_relid::TEXT,
1050+
p_partition::TEXT);
1051+
GET DIAGNOSTICS v_rows = ROW_COUNT;
1052+
1053+
/* Show number of copied rows */
1054+
RAISE NOTICE '% rows copied from %', v_rows, p_partition::TEXT;
1055+
END IF;
1056+
1057+
SELECT relkind FROM pg_catalog.pg_class
1058+
WHERE oid = p_partition
1059+
INTO v_relkind;
1060+
1061+
/*
1062+
* Determine the kind of child relation. It can be either regular
1063+
* table (r) or foreign table (f). Depending on relkind we use
1064+
* DROP TABLE or DROP FOREIGN TABLE.
1065+
*/
1066+
IF v_relkind = 'f' THEN
1067+
EXECUTE format('DROP FOREIGN TABLE %s', p_partition::TEXT);
1068+
ELSE
1069+
EXECUTE format('DROP TABLE %s', p_partition::TEXT);
1070+
END IF;
10461071

10471072
/* Invalidate cache */
10481073
PERFORM @extschema@.on_update_partitions(parent_relid);
10491074

10501075
RETURN part_name;
10511076
END
10521077
$$
1053-
LANGUAGE plpgsql;
1078+
LANGUAGE plpgsql
1079+
SET pg_pathman.enable_partitionfilter = off; /* ensures that PartitionFilter is OFF */
10541080

10551081

10561082
/*

‎src/hooks.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "utils.h"
1919
#include "xact_handling.h"
2020

21+
#include "access/transam.h"
2122
#include "miscadmin.h"
2223
#include "optimizer/cost.h"
2324
#include "optimizer/restrictinfo.h"
@@ -546,6 +547,10 @@ pathman_relcache_hook(Datum arg, Oid relid)
546547
if (!IsPathmanReady())
547548
return;
548549

550+
/* We shouldn't even consider special OIDs */
551+
if (relid < FirstNormalObjectId)
552+
return;
553+
549554
/* Invalidation event for PATHMAN_CONFIG table (probably DROP) */
550555
if (relid == get_pathman_config_relid())
551556
delay_pathman_shutdown();
@@ -569,7 +574,8 @@ pathman_relcache_hook(Datum arg, Oid relid)
569574
/* Both syscache and pathman's cache say it isn't a partition */
570575
case PPS_ENTRY_NOT_FOUND:
571576
{
572-
delay_invalidation_parent_rel(partitioned_table);
577+
if (partitioned_table != InvalidOid)
578+
delay_invalidation_parent_rel(partitioned_table);
573579
#ifdef NOT_USED
574580
elog(DEBUG2, "Invalidation message for relation %u [%u]",
575581
relid, MyProcPid);
@@ -588,7 +594,8 @@ pathman_relcache_hook(Datum arg, Oid relid)
588594
break;
589595

590596
default:
591-
elog(ERROR, "Not implemented yet");
597+
elog(ERROR, "Not implemented yet (%s)",
598+
CppAsString(pathman_relcache_hook));
592599
break;
593600
}
594601
}

‎src/pathman_workers.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ start_bg_worker(const char bgworker_name[BGW_MAXLEN],
213213
case BGW_PM_DIED:
214214
ereport(ERROR,
215215
(errmsg("Postmaster died during the pg_pathman background worker process"),
216-
errhint("More details may be available in the server log.")));
216+
errhint("More details may be available in the server log.")));
217217
break;
218218

219219
default:
@@ -300,9 +300,10 @@ create_partitions_bg_worker(Oid relid, Datum value, Oid value_type)
300300
dsm_detach(segment);
301301

302302
if (child_oid == InvalidOid)
303-
elog(ERROR,
304-
"Attempt to append new partitions to relation \"%s\" failed",
305-
get_rel_name_or_relid(relid));
303+
ereport(ERROR,
304+
(errmsg("Attempt to spawn new partitions of relation \"%s\" failed",
305+
get_rel_name_or_relid(relid)),
306+
errhint("See server log for more details.")));
306307

307308
return child_oid;
308309
}

‎src/pl_funcs.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,13 @@ on_partitions_created_internal(Oid partitioned_table, bool add_callbacks)
106106
static void
107107
on_partitions_updated_internal(Oid partitioned_table, bool add_callbacks)
108108
{
109-
bool found;
109+
bool entry_found;
110110

111111
elog(DEBUG2, "on_partitions_updated() [add_callbacks = %s] "
112112
"triggered for relation %u",
113113
(add_callbacks ? "true" : "false"), partitioned_table);
114114

115-
invalidate_pathman_relation_info(partitioned_table, &found);
115+
invalidate_pathman_relation_info(partitioned_table, &entry_found);
116116
}
117117

118118
static void

‎src/relation_info.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include "access/htup_details.h"
1717
#include "access/xact.h"
18+
#include "catalog/catalog.h"
1819
#include "catalog/indexing.h"
1920
#include "catalog/pg_inherits.h"
2021
#include "miscadmin.h"
@@ -242,12 +243,11 @@ get_pathman_relation_info(Oid relid)
242243
part_type = DatumGetPartType(values[Anum_pathman_config_parttype - 1]);
243244
attname = TextDatumGetCString(values[Anum_pathman_config_attname - 1]);
244245

245-
/* Refresh partitioned table cache entry */
246+
/* Refresh partitioned table cache entry (might turn NULL) */
246247
/* TODO: possible refactoring, pass found 'prel' instead of searching */
247248
prel = refresh_pathman_relation_info(relid,
248249
part_type,
249250
attname);
250-
Assert(PrelIsValid(prel)); /* it MUST be valid if we got here */
251251
}
252252
/* Else clear remaining cache entry */
253253
else remove_pathman_relation_info(relid);
@@ -346,7 +346,7 @@ finish_delayed_invalidation(void)
346346
/* Handle the probable 'DROP EXTENSION' case */
347347
if (delayed_shutdown)
348348
{
349-
Oid cur_pathman_config_relid;
349+
Oid cur_pathman_config_relid;
350350

351351
/* Unset 'shutdown' flag */
352352
delayed_shutdown = false;
@@ -376,9 +376,14 @@ finish_delayed_invalidation(void)
376376
{
377377
Oid parent = lfirst_oid(lc);
378378

379+
/* Skip if it's a TOAST table */
380+
if (IsToastNamespace(get_rel_namespace(parent)))
381+
continue;
382+
379383
if (!pathman_config_contains_relation(parent, NULL, NULL, NULL))
380384
remove_pathman_relation_info(parent);
381385
else
386+
/* get_pathman_relation_info() will refresh this entry */
382387
invalidate_pathman_relation_info(parent, NULL);
383388
}
384389

@@ -387,6 +392,10 @@ finish_delayed_invalidation(void)
387392
{
388393
Oid vague_rel = lfirst_oid(lc);
389394

395+
/* Skip if it's a TOAST table */
396+
if (IsToastNamespace(get_rel_namespace(vague_rel)))
397+
continue;
398+
390399
/* It might be a partitioned table or a partition */
391400
if (!try_perform_parent_refresh(vague_rel))
392401
{
@@ -656,7 +665,7 @@ shout_if_prel_is_invalid(Oid parent_oid,
656665
PartType expected_part_type)
657666
{
658667
if (!prel)
659-
elog(ERROR, "relation \"%s\" is not partitioned by pg_pathman",
668+
elog(ERROR, "relation \"%s\" has no partitions",
660669
get_rel_name_or_relid(parent_oid));
661670

662671
if (!PrelIsValid(prel))

0 commit comments

Comments
 (0)