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()

Author: funbringer

README.md

@@ -165,9 +165,9 @@ add_range_partition(relation       REGCLASS,
 Create new RANGE partition for `relation` with specified range bounds.
 
 ```plpgsql
-drop_range_partition(partition TEXT)
+drop_range_partition(partition TEXT, delete_data BOOLEAN DEFAULT TRUE)
 ```
-Drop RANGE partition and all its data.
+Drop RANGE partition and all of its data if `delete_data` is true.
 
 ```plpgsql
 attach_range_partition(relation    REGCLASS,
@@ -221,8 +221,7 @@ CREATE TABLE IF NOT EXISTS pathman_config (
     parttype        INTEGER NOT NULL,
     range_interval  TEXT,
 
-    CHECK (parttype IN (1, 2)) /* check for allowed part types */
-);
+    CHECK (parttype IN (1, 2)) /* check for allowed part types */ );
 ```
 This table stores a list of partitioned tables.
 
@@ -232,8 +231,7 @@ CREATE TABLE IF NOT EXISTS pathman_config_params (
     partrel        REGCLASS NOT NULL PRIMARY KEY,
     enable_parent  BOOLEAN NOT NULL DEFAULT TRUE,
     auto           BOOLEAN NOT NULL DEFAULT TRUE,
-    init_callback  REGPROCEDURE NOT NULL DEFAULT 0
-);
+    init_callback  REGPROCEDURE NOT NULL DEFAULT 0);
 ```
 This table stores optional parameters which override standard behavior.
 
@@ -259,7 +257,7 @@ This view lists all currently running concurrent partitioning tasks.
 #### `pathman_partition_list` --- list of all existing partitions
 ```plpgsql
 -- helper SRF function
-CREATE OR REPLACE FUNCTION @extschema@.show_partition_list()
+CREATE OR REPLACE FUNCTION show_partition_list()
 RETURNS TABLE (
     parent     REGCLASS,
     partition  REGCLASS,
@@ -471,7 +469,7 @@ Notice that the `Append` node contains only one child scan which corresponds to
 > **Important:** pay attention to the fact that `pg_pathman` excludes the parent table from the query plan.
 
 To access parent table use ONLY modifier:
-```
+```plpgsql
 EXPLAIN SELECT * FROM ONLY items;
                       QUERY PLAN
 ------------------------------------------------------
@@ -484,8 +482,7 @@ CREATE TABLE journal (
     id      SERIAL,
     dt      TIMESTAMP NOT NULL,
     level   INTEGER,
-    msg     TEXT
-);
+    msg     TEXT);
 
 -- similar index will also be created for each partition
 CREATE INDEX ON journal(dt);
@@ -515,8 +512,8 @@ CREATE FOREIGN TABLE journal_archive (
     id      INTEGER NOT NULL,
     dt      TIMESTAMP NOT NULL,
     level   INTEGER,
-    msg     TEXT
-) SERVER archive_server;
+    msg     TEXT)
+SERVER archive_server;
 
 SELECT attach_range_partition('journal', 'journal_archive', '2014-01-01'::date, '2015-01-01'::date);
 ```

init.sql

@@ -511,7 +511,7 @@ RETURNS INTEGER AS
 $$
 DECLARE
 	v_rec			RECORD;
-	v_rows			INTEGER;
+	v_rows			BIGINT;
 	v_part_count	INTEGER := 0;
 	conf_num_del	INTEGER;
 	v_relkind		CHAR;
@@ -539,10 +539,9 @@ BEGIN
 				  ORDER BY inhrelid ASC)
 	LOOP
 		IF NOT delete_data THEN
-			EXECUTE format('WITH part_data AS (DELETE FROM %s RETURNING *)
-							INSERT INTO %s SELECT * FROM part_data',
-							v_rec.tbl::TEXT,
-							parent_relid::text);
+			EXECUTE format('INSERT INTO %s SELECT * FROM %s',
+							parent_relid::TEXT,
+							v_rec.tbl::TEXT);
 			GET DIAGNOSTICS v_rows = ROW_COUNT;
 
 			/* Show number of copied rows */

range.sql

@@ -1027,12 +1027,15 @@ LANGUAGE plpgsql;
  * Drop range partition
  */
 CREATE OR REPLACE FUNCTION @extschema@.drop_range_partition(
-	p_partition		REGCLASS)
+	p_partition		REGCLASS,
+	delete_data		BOOLEAN DEFAULT TRUE)
 RETURNS TEXT AS
 $$
 DECLARE
 	parent_relid	REGCLASS;
 	part_name		TEXT;
+	v_relkind		CHAR;
+	v_rows			BIGINT;
 
 BEGIN
 	parent_relid := @extschema@.get_parent_of_partition(p_partition);
@@ -1041,16 +1044,39 @@ BEGIN
 	/* Acquire lock on parent */
 	PERFORM @extschema@.lock_partitioned_relation(parent_relid);
 
-	/* Drop table */
-	EXECUTE format('DROP TABLE %s', part_name);
+	IF NOT delete_data THEN
+		EXECUTE format('INSERT INTO %s SELECT * FROM %s',
+						parent_relid::TEXT,
+						p_partition::TEXT);
+		GET DIAGNOSTICS v_rows = ROW_COUNT;
+
+		/* Show number of copied rows */
+		RAISE NOTICE '% rows copied from %', v_rows, p_partition::TEXT;
+	END IF;
+
+	SELECT relkind FROM pg_catalog.pg_class
+	WHERE oid = p_partition
+	INTO v_relkind;
+
+	/*
+	 * Determine the kind of child relation. It can be either regular
+	 * table (r) or foreign table (f). Depending on relkind we use
+	 * DROP TABLE or DROP FOREIGN TABLE.
+	 */
+	IF v_relkind = 'f' THEN
+		EXECUTE format('DROP FOREIGN TABLE %s', p_partition::TEXT);
+	ELSE
+		EXECUTE format('DROP TABLE %s', p_partition::TEXT);
+	END IF;
 
 	/* Invalidate cache */
 	PERFORM @extschema@.on_update_partitions(parent_relid);
 
 	RETURN part_name;
 END
 $$
-LANGUAGE plpgsql;
+LANGUAGE plpgsql
+SET pg_pathman.enable_partitionfilter = off; /* ensures that PartitionFilter is OFF */
 
 
 /*

src/hooks.c

@@ -18,6 +18,7 @@
 #include "utils.h"
 #include "xact_handling.h"
 
+#include "access/transam.h"
 #include "miscadmin.h"
 #include "optimizer/cost.h"
 #include "optimizer/restrictinfo.h"
@@ -546,6 +547,10 @@ pathman_relcache_hook(Datum arg, Oid relid)
 	if (!IsPathmanReady())
 		return;
 
+	/* We shouldn't even consider special OIDs */
+	if (relid < FirstNormalObjectId)
+		return;
+
 	/* Invalidation event for PATHMAN_CONFIG table (probably DROP) */
 	if (relid == get_pathman_config_relid())
 		delay_pathman_shutdown();
@@ -569,7 +574,8 @@ pathman_relcache_hook(Datum arg, Oid relid)
 		/* Both syscache and pathman's cache say it isn't a partition */
 		case PPS_ENTRY_NOT_FOUND:
 			{
-				delay_invalidation_parent_rel(partitioned_table);
+				if (partitioned_table != InvalidOid)
+					delay_invalidation_parent_rel(partitioned_table);
 #ifdef NOT_USED
 				elog(DEBUG2, "Invalidation message for relation %u [%u]",
 					 relid, MyProcPid);
@@ -588,7 +594,8 @@ pathman_relcache_hook(Datum arg, Oid relid)
 			break;
 
 		default:
-			elog(ERROR, "Not implemented yet");
+			elog(ERROR, "Not implemented yet (%s)",
+				 CppAsString(pathman_relcache_hook));
 			break;
 	}
 }

src/pathman_workers.c

@@ -213,7 +213,7 @@ start_bg_worker(const char bgworker_name[BGW_MAXLEN],
 		case BGW_PM_DIED:
 			ereport(ERROR,
 					(errmsg("Postmaster died during the pg_pathman background worker process"),
-					errhint("More details may be available in the server log.")));
+					 errhint("More details may be available in the server log.")));
 			break;
 
 		default:
@@ -300,9 +300,10 @@ create_partitions_bg_worker(Oid relid, Datum value, Oid value_type)
 	dsm_detach(segment);
 
 	if (child_oid == InvalidOid)
-		elog(ERROR,
-			 "Attempt to append new partitions to relation \"%s\" failed",
-			 get_rel_name_or_relid(relid));
+		ereport(ERROR,
+				(errmsg("Attempt to spawn new partitions of relation \"%s\" failed",
+						get_rel_name_or_relid(relid)),
+				 errhint("See server log for more details.")));
 
 	return child_oid;
 }

src/pl_funcs.c

@@ -106,13 +106,13 @@ on_partitions_created_internal(Oid partitioned_table, bool add_callbacks)
 static void
 on_partitions_updated_internal(Oid partitioned_table, bool add_callbacks)
 {
-	bool found;
+	bool entry_found;
 
 	elog(DEBUG2, "on_partitions_updated() [add_callbacks = %s] "
 				 "triggered for relation %u",
 		 (add_callbacks ? "true" : "false"), partitioned_table);
 
-	invalidate_pathman_relation_info(partitioned_table, &found);
+	invalidate_pathman_relation_info(partitioned_table, &entry_found);
 }
 
 static void

src/relation_info.c

@@ -15,6 +15,7 @@
 
 #include "access/htup_details.h"
 #include "access/xact.h"
+#include "catalog/catalog.h"
 #include "catalog/indexing.h"
 #include "catalog/pg_inherits.h"
 #include "miscadmin.h"
@@ -242,12 +243,11 @@ get_pathman_relation_info(Oid relid)
 			part_type = DatumGetPartType(values[Anum_pathman_config_parttype - 1]);
 			attname = TextDatumGetCString(values[Anum_pathman_config_attname - 1]);
 
-			/* Refresh partitioned table cache entry */
+			/* Refresh partitioned table cache entry (might turn NULL) */
 			/* TODO: possible refactoring, pass found 'prel' instead of searching */
 			prel = refresh_pathman_relation_info(relid,
 												 part_type,
 												 attname);
-			Assert(PrelIsValid(prel)); /* it MUST be valid if we got here */
 		}
 		/* Else clear remaining cache entry */
 		else remove_pathman_relation_info(relid);
@@ -346,7 +346,7 @@ finish_delayed_invalidation(void)
 		/* Handle the probable 'DROP EXTENSION' case */
 		if (delayed_shutdown)
 		{
-			Oid	cur_pathman_config_relid;
+			Oid		cur_pathman_config_relid;
 
 			/* Unset 'shutdown' flag */
 			delayed_shutdown = false;
@@ -376,9 +376,14 @@ finish_delayed_invalidation(void)
 		{
 			Oid		parent = lfirst_oid(lc);
 
+			/* Skip if it's a TOAST table */
+			if (IsToastNamespace(get_rel_namespace(parent)))
+				continue;
+
 			if (!pathman_config_contains_relation(parent, NULL, NULL, NULL))
 				remove_pathman_relation_info(parent);
 			else
+				/* get_pathman_relation_info() will refresh this entry */
 				invalidate_pathman_relation_info(parent, NULL);
 		}
 
@@ -387,6 +392,10 @@ finish_delayed_invalidation(void)
 		{
 			Oid		vague_rel = lfirst_oid(lc);
 
+			/* Skip if it's a TOAST table */
+			if (IsToastNamespace(get_rel_namespace(vague_rel)))
+				continue;
+
 			/* It might be a partitioned table or a partition */
 			if (!try_perform_parent_refresh(vague_rel))
 			{
@@ -656,7 +665,7 @@ shout_if_prel_is_invalid(Oid parent_oid,
 						 PartType expected_part_type)
 {
 	if (!prel)
-		elog(ERROR, "relation \"%s\" is not partitioned by pg_pathman",
+		elog(ERROR, "relation \"%s\" has no partitions",
 			 get_rel_name_or_relid(parent_oid));
 
 	if (!PrelIsValid(prel))