Merge branch 'picky_nodes' into merge_concurrent

Author: zilder

src/hooks.c

@@ -12,12 +12,13 @@
 #include "init.h"
 #include "runtimeappend.h"
 #include "runtime_merge_append.h"
+#include "partition_filter.h"
 #include "utils.h"
 
 #include "miscadmin.h"
 #include "optimizer/cost.h"
 #include "optimizer/restrictinfo.h"
-#include "partition_filter.h"
+#include "utils/typcache.h"
 
 
 set_join_pathlist_hook_type		set_join_pathlist_next = NULL;

src/init.c

@@ -20,10 +20,10 @@
 #include "access/htup_details.h"
 #include "access/sysattr.h"
 #include "catalog/indexing.h"
+#include "catalog/pg_constraint.h"
+#include "catalog/pg_inherits.h"
 #include "catalog/pg_inherits_fn.h"
 #include "catalog/pg_type.h"
-#include "catalog/pg_inherits.h"
-#include "catalog/pg_constraint.h"
 #include "executor/spi.h"
 #include "miscadmin.h"
 #include "optimizer/clauses.h"
@@ -81,22 +81,22 @@ static int oid_cmp(const void *p1, const void *p2);
 
 /*
  * Create local PartRelationInfo cache & load pg_pathman's config.
+ * Return true on success. May occasionally emit ERROR.
  */
-void
+bool
 load_config(void)
 {
-	elog(DEBUG2, "pg_pathman's config!!! [%u]", MyProcPid);
-
-	/* 
+	/*
 	 * Try to cache important relids.
-	 * 
+	 *
 	 * Once CREATE EXTENSION stmt is processed, get_pathman_schema()
-	 * function starts to return perfectly valid schema Oid, which
-	 * means we have to check that ALL pg_pathman's relations' Oids
-	 * have been cached properly.
+	 * function starts returning perfectly valid schema Oid, which
+	 * means we have to check that *ALL* pg_pathman's relations' Oids
+	 * have been cached properly. Only then can we assume that
+	 * initialization is not needed anymore.
 	 */
 	if (!init_pathman_relation_oids())
-		return; /* remain 'uninitialized', exit */
+		return false; /* remain 'uninitialized', exit before creating main caches */
 
 	init_local_cache();		/* create 'partitioned_rels' hash table */
 	read_pathman_config();	/* read PATHMAN_CONFIG table & fill cache */
@@ -112,6 +112,8 @@ load_config(void)
 	initialization_needed = false;
 
 	elog(DEBUG2, "pg_pathman's config has been loaded successfully [%u]", MyProcPid);
+
+	return true;
 }
 
 /*
@@ -123,7 +125,8 @@ unload_config(void)
 	/* Don't forget to reset pg_pathman's cached relids */
 	fini_pathman_relation_oids();
 
-	fini_local_cache();		/* destroy 'partitioned_rels' hash table */
+	/* Destroy 'partitioned_rels' & 'parent_cache' hash tables */
+	fini_local_cache();
 
 	/* Mark pg_pathman as uninitialized */
 	initialization_needed = true;
@@ -143,36 +146,42 @@ estimate_pathman_shmem_size(void)
 }
 
 /*
- * TODO: write some comment;
+ * Cache *all* important pg_pathman's relids at once.
+ * We should NOT rely on any previously cached values.
  */
 static bool
 init_pathman_relation_oids(void)
 {
 	Oid schema = get_pathman_schema();
+	Assert(schema != InvalidOid);
 
 	/* Cache PATHMAN_CONFIG relation's Oid */
-	pathman_config_params_relid = get_relname_relid(PATHMAN_CONFIG_PARAMS,
-													schema);
 	pathman_config_relid = get_relname_relid(PATHMAN_CONFIG, schema);
+	if (pathman_config_relid == InvalidOid)
+		return false;
 
-	/* Return false if any relation doesn't exist yet */
-	if (pathman_config_params_relid == InvalidOid ||
-		pathman_config_relid == InvalidOid)
-	{
+	/* Cache PATHMAN_CONFIG_PARAMS relation's Oid */
+	pathman_config_params_relid = get_relname_relid(PATHMAN_CONFIG_PARAMS,
+													schema);
+	if (pathman_config_params_relid == InvalidOid)
 		return false;
-	}
 
+	/* NOTE: add more relations to be cached right here ^^^ */
+
+	/* Everything is fine, proceed */
 	return true;
 }
 
 /*
- * TODO: write some comment;
+ * Forget *all* pg_pathman's cached relids.
  */
 static void
 fini_pathman_relation_oids(void)
 {
 	pathman_config_relid = InvalidOid;
 	pathman_config_params_relid = InvalidOid;
+	
+	/* NOTE: add more relations to be forgotten right here ^^^ */
 }
 
 /*

src/init.h

@@ -26,7 +26,7 @@ extern bool		initialization_needed;
 
 Size estimate_pathman_shmem_size(void);
 void init_shmem_config(void);
-void load_config(void);
+bool load_config(void);
 void unload_config(void);
 Size get_worker_slots_size(void);
 void create_worker_slots(void);

src/pathman.h

@@ -11,15 +11,11 @@
 #ifndef PATHMAN_H
 #define PATHMAN_H
 
-#include "dsm_array.h"
 #include "init.h"
 #include "relation_info.h"
 #include "rangeset.h"
 
 #include "postgres.h"
-#include "utils/date.h"
-#include "utils/snapshot.h"
-#include "utils/typcache.h"
 #include "nodes/makefuncs.h"
 #include "nodes/primnodes.h"
 #include "nodes/execnodes.h"
@@ -28,16 +24,18 @@
 
 
 /* Check PostgreSQL version (9.5.4 contains an important fix for BGW) */
-// #if PG_VERSION_NUM < 90504
-// 	#error "Cannot build pg_pathman with PostgreSQL version lower than 9.5.4"
-// #endif
+#if PG_VERSION_NUM < 90503
+	#error "Cannot build pg_pathman with PostgreSQL version lower than 9.5.3"
+#elif PG_VERSION_NUM < 90504
+	#warning "It is STRONGLY recommended to use pg_pathman with PostgreSQL 9.5.4 since it contains important fixes"
+#endif
 
 /* Get CString representation of Datum (simple wrapper) */
 #ifdef USE_ASSERT_CHECKING
-#include "utils.h"
-#define DebugPrintDatum(datum, typid) ( datum_to_cstring((datum), (typid)) )
+	#include "utils.h"
+	#define DebugPrintDatum(datum, typid) ( datum_to_cstring((datum), (typid)) )
 #else
-#define DebugPrintDatum(datum, typid) ( "[use --enable-cassert]" )
+	#define DebugPrintDatum(datum, typid) ( "[use --enable-cassert]" )
 #endif
 
 

src/pg_pathman.c

@@ -41,6 +41,7 @@
 #include "utils/syscache.h"
 #include "utils/selfuncs.h"
 #include "utils/snapmgr.h"
+#include "utils/typcache.h"
 
 
 PG_MODULE_MAGIC;
@@ -814,9 +815,11 @@ create_partitions_internal(Oid relid, Datum value, Oid value_type)
 		Datum					values[Natts_pathman_config];
 		bool					isnull[Natts_pathman_config];
 
+		prel = get_pathman_relation_info(relid);
+		shout_if_prel_is_invalid(relid, prel, PT_RANGE);
+
 		/* Get both PartRelationInfo & PATHMAN_CONFIG contents for this relation */
-		if ((prel = get_pathman_relation_info(relid)) != NULL &&
-			pathman_config_contains_relation(relid, values, isnull, NULL))
+		if (pathman_config_contains_relation(relid, values, isnull, NULL))
 		{
 			Datum		min_rvalue,
 						max_rvalue;
@@ -828,10 +831,6 @@ create_partitions_internal(Oid relid, Datum value, Oid value_type)
 
 			FmgrInfo	interval_type_cmp;
 
-			if (prel->parttype != PT_RANGE)
-				elog(ERROR, "Relation \"%s\" is not partitioned by RANGE",
-					 get_rel_name_or_relid(relid));
-
 			/* Fill the FmgrInfo struct with a cmp(value, part_attribute) function */
 			fill_type_cmp_fmgr_info(&interval_type_cmp, value_type, prel->atttype);
 
@@ -893,7 +892,7 @@ create_partitions_internal(Oid relid, Datum value, Oid value_type)
 			SPI_finish(); /* close SPI connection */
 		}
 		else
-			elog(ERROR, "Relation \"%s\" is not partitioned by pg_pathman",
+			elog(ERROR, "pg_pathman's config does not contain relation \"%s\"",
 				 get_rel_name_or_relid(relid));
 	}
 	PG_CATCH();
@@ -932,7 +931,8 @@ create_partitions(Oid relid, Datum value, Oid value_type)
 	if (pathman_config_contains_relation(relid, NULL, NULL, &rel_xmin))
 	{
 		/* If table was partitioned in some previous xact, run BGWorker */
-		if (TransactionIdPrecedes(rel_xmin, GetCurrentTransactionId()))
+		if (TransactionIdPrecedes(rel_xmin, GetCurrentTransactionId()) ||
+			TransactionIdEquals(rel_xmin, FrozenTransactionId))
 		{
 			elog(DEBUG2, "create_partitions(): chose BGW [%u]", MyProcPid);
 			last_partition = create_partitions_bg_worker(relid, value, value_type);
@@ -1179,6 +1179,9 @@ handle_binary_opexpr(WalkerContext *context, WrapperNode *result,
 										result);
 				return;
 			}
+
+		default:
+			elog(ERROR, "Unknown partitioning type %u", prel->parttype);
 	}
 
 	result->rangeset = list_make1_irange(make_irange(0,

src/pl_funcs.c

@@ -220,14 +220,12 @@ find_or_create_range_partition(PG_FUNCTION_ARGS)
 	search_rangerel_result	search_state;
 
 	prel = get_pathman_relation_info(parent_oid);
-
-	if (!prel)
-		PG_RETURN_NULL();
+	shout_if_prel_is_invalid(parent_oid, prel, PT_RANGE);
 
 	fill_type_cmp_fmgr_info(&cmp_func, value_type, prel->atttype);
 
-	/* FIXME: does this function even work? */
-	search_state = search_range_partition_eq(value, &cmp_func,prel,
+	/* Use available PartRelationInfo to find partition */
+	search_state = search_range_partition_eq(value, &cmp_func, prel,
 											 &found_rentry);
 
 	/*
@@ -286,9 +284,7 @@ get_range_by_part_oid(PG_FUNCTION_ARGS)
 	const PartRelationInfo *prel;
 
 	prel = get_pathman_relation_info(parent_oid);
-	if (!prel)
-		elog(ERROR, "Relation \"%s\" is not partitioned by pg_pathman",
-			 get_rel_name_or_relid(parent_oid));
+	shout_if_prel_is_invalid(parent_oid, prel, PT_RANGE);
 
 	ranges = PrelGetRangesArray(prel);
 
@@ -306,6 +302,7 @@ get_range_by_part_oid(PG_FUNCTION_ARGS)
 			PG_RETURN_ARRAYTYPE_P(arr);
 		}
 
+	/* No partition found, report error */
 	elog(ERROR, "Relation \"%s\" has no partition \"%s\"",
 		 get_rel_name_or_relid(parent_oid),
 		 get_rel_name_or_relid(child_oid));
@@ -330,9 +327,7 @@ get_range_by_idx(PG_FUNCTION_ARGS)
 	const PartRelationInfo *prel;
 
 	prel = get_pathman_relation_info(parent_oid);
-	if (!prel)
-		elog(ERROR, "Relation \"%s\" is not partitioned by pg_pathman",
-			 get_rel_name_or_relid(parent_oid));
+	shout_if_prel_is_invalid(parent_oid, prel, PT_RANGE);
 
 	if (((uint32) abs(idx)) >= PrelChildrenCount(prel))
 		elog(ERROR, "Partition #%d does not exist (total amount is %u)",
@@ -366,14 +361,7 @@ get_min_range_value(PG_FUNCTION_ARGS)
 	const PartRelationInfo *prel;
 
 	prel = get_pathman_relation_info(parent_oid);
-	if (!prel)
-		elog(ERROR, "Relation \"%s\" is not partitioned by pg_pathman",
-			 get_rel_name_or_relid(parent_oid));
-
-	if (prel->parttype != PT_RANGE)
-		if (!prel)
-			elog(ERROR, "Relation \"%s\" is not partitioned by RANGE",
-				 get_rel_name_or_relid(parent_oid));
+	shout_if_prel_is_invalid(parent_oid, prel, PT_RANGE);
 
 	ranges = PrelGetRangesArray(prel);
 
@@ -391,14 +379,7 @@ get_max_range_value(PG_FUNCTION_ARGS)
 	const PartRelationInfo *prel;
 
 	prel = get_pathman_relation_info(parent_oid);
-	if (!prel)
-		elog(ERROR, "Relation \"%s\" is not partitioned by pg_pathman",
-			 get_rel_name_or_relid(parent_oid));
-
-	if (prel->parttype != PT_RANGE)
-		if (!prel)
-			elog(ERROR, "Relation \"%s\" is not partitioned by RANGE",
-				 get_rel_name_or_relid(parent_oid));
+	shout_if_prel_is_invalid(parent_oid, prel, PT_RANGE);
 
 	ranges = PrelGetRangesArray(prel);
 
@@ -428,14 +409,7 @@ check_overlap(PG_FUNCTION_ARGS)
 	const PartRelationInfo *prel;
 
 	prel = get_pathman_relation_info(parent_oid);
-	if (!prel)
-		elog(ERROR, "Relation \"%s\" is not partitioned by pg_pathman",
-			 get_rel_name_or_relid(parent_oid));
-
-	if (prel->parttype != PT_RANGE)
-		if (!prel)
-			elog(ERROR, "Relation \"%s\" is not partitioned by RANGE",
-				 get_rel_name_or_relid(parent_oid));
+	shout_if_prel_is_invalid(parent_oid, prel, PT_RANGE);
 
 	/* comparison functions */
 	fill_type_cmp_fmgr_info(&cmp_func_1, p1_type, prel->atttype);

src/relation_info.c

@@ -24,6 +24,7 @@
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/snapmgr.h"
+#include "utils/typcache.h"
 
 
 /*

src/relation_info.h

@@ -19,11 +19,12 @@
 
 
 /*
- * Partitioning type
+ * Partitioning type.
  */
 typedef enum
 {
-	PT_HASH = 1,
+	PT_INDIFFERENT = 0, /* for part type traits (virtual type) */
+	PT_HASH,
 	PT_RANGE
 } PartType;