WIP refactoring, move functions to reasonable files

Author: funbringer

expected/pathman_calamity.out

@@ -556,7 +556,7 @@ ERROR:  relation "0" does not exist
 SELECT add_to_pathman_config('calamity.part_test', NULL);		/* no column */
 ERROR:  'expression' should not be NULL
 SELECT add_to_pathman_config('calamity.part_test', 'V_A_L');	/* wrong column */
-ERROR:  Cannot find type name for attribute "v_a_l" of relation "part_test"
+ERROR:  cannot find type name for attribute "v_a_l" of relation "part_test"
 SELECT add_to_pathman_config('calamity.part_test', 'val');		/* OK */
  add_to_pathman_config 
 -----------------------

expected/pathman_utility_stmt.out

@@ -159,7 +159,7 @@ SELECT * FROM copy_stmt_hooking.test WHERE val > 20;
 
 /* COPY FROM (partitioned column is not specified) */
 COPY copy_stmt_hooking.test(comment) FROM stdin;
-ERROR:  partition expression's value should not be NULL
+ERROR:  partitioning expression's value should not be NULL
 /* COPY FROM (we don't support FREEZE) */
 COPY copy_stmt_hooking.test FROM stdin WITH (FREEZE);
 ERROR:  freeze is not supported for partitioned tables

src/hooks.c

@@ -800,8 +800,10 @@ pathman_process_utility_hook(Node *parsetree,
 		/* Override standard RENAME statement if needed */
 		else if (is_pathman_related_table_rename(parsetree,
 												 &relation_oid))
+		{
 			PathmanRenameConstraint(relation_oid,
 									(const RenameStmt *) parsetree);
+		}
 
 		/* Override standard ALTER COLUMN TYPE statement if needed */
 		else if (is_pathman_related_alter_column_type(parsetree,
@@ -810,15 +812,15 @@ pathman_process_utility_hook(Node *parsetree,
 													  &part_type))
 		{
 			if (part_type == PT_HASH)
-			{
 				ereport(ERROR,
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 						 errmsg("cannot change type of column \"%s\""
 								" of table \"%s\" partitioned by HASH",
 								get_attname(relation_oid, attr_number),
 								get_rel_name(relation_oid))));
-			}
-			mark_pathman_expression_for_update(relation_oid);
+
+			/* Don't forget to invalidate parsed partitioning expression */
+			pathman_config_invalidate_parsed_expression(relation_oid);
 		}
 	}
 

src/include/init.h

@@ -204,7 +204,15 @@ bool pathman_config_contains_relation(Oid relid,
 									  Datum *values,
 									  bool *isnull,
 									  TransactionId *xmin,
-									  HeapTuple *tuple);
+									  ItemPointerData *iptr);
+
+void pathman_config_invalidate_parsed_expression(Oid relid);
+
+void pathman_config_refresh_parsed_expression(Oid relid,
+											  Datum *values,
+											  bool *isnull,
+											  ItemPointer iptr);
+
 
 bool read_pathman_params(Oid relid,
 						 Datum *values,

src/include/partition_creation.h

@@ -78,23 +78,6 @@ Node * build_raw_hash_check_tree(Node *raw_expression,
 
 void drop_check_constraint(Oid relid);
 
-typedef struct
-{
-	Oid		 expr_type;
-	Datum	 expr_datum;
-	Node	*raw_expr;
-} PartExpressionInfo;
-
-/* Expression parsing functions */
-PartExpressionInfo *get_part_expression_info(Oid relid,
-											 const char *expr_string,
-											 bool check_hash_func,
-											 bool make_plan);
-
-Node *parse_partitioning_expression(Oid relid,
-									const char *expression,
-									char **query_string_out,
-									Node **parsetree_out);
 
 /* Update triggers */
 void create_single_update_trigger_internal(Oid partition_relid,

src/include/partition_filter.h

@@ -25,12 +25,11 @@
 #endif
 
 
-#define ERR_PART_ATTR_NULL		"partition expression's value should not be NULL"
-#define ERR_PART_ATTR_MULTIPLE_RESULTS \
-		"partition expression's value should be single, not set"
-#define ERR_PART_ATTR_NO_PART	"no suitable partition for key '%s'"
-#define ERR_PART_ATTR_MULTIPLE	"PartitionFilter selected more than one partition"
-#define ERR_PART_DESC_CONVERT	"could not convert row type for partition"
+#define ERR_PART_ATTR_NULL				"partitioning expression's value should not be NULL"
+#define ERR_PART_ATTR_MULTIPLE_RESULTS	"partitioning expression should return single value"
+#define ERR_PART_ATTR_NO_PART			"no suitable partition for key '%s'"
+#define ERR_PART_ATTR_MULTIPLE			"PartitionFilter selected more than one partition"
+#define ERR_PART_DESC_CONVERT			"could not convert row type for partition"
 
 
 /*

src/include/pathman.h

@@ -49,8 +49,8 @@
 #define Anum_pathman_config_expression		2	/* partition expression (original) */
 #define Anum_pathman_config_parttype		3	/* partitioning type (1|2) */
 #define Anum_pathman_config_range_interval	4	/* interval for RANGE pt. (text) */
-#define Anum_pathman_config_expression_p	5	/* parsed partition expression (text) */
-#define Anum_pathman_config_atttype			6	/* partitioned atttype */
+#define Anum_pathman_config_expression_p	5	/* parsed partitioning expression (text) */
+#define Anum_pathman_config_atttype			6	/* partitioned atttype (oid) */
 
 /* type modifier (typmod) for 'range_interval' */
 #define PATHMAN_CONFIG_interval_typmod		-1

src/include/relation_info.h

@@ -14,6 +14,7 @@
 
 #include "postgres.h"
 #include "access/attnum.h"
+#include "access/sysattr.h"
 #include "fmgr.h"
 #include "nodes/bitmapset.h"
 #include "nodes/nodes.h"
@@ -86,7 +87,10 @@ FreeBound(Bound *bound, bool byval)
 }
 
 inline static int
-cmp_bounds(FmgrInfo *cmp_func, const Oid collid, const Bound *b1, const Bound *b2)
+cmp_bounds(FmgrInfo *cmp_func,
+		   const Oid collid,
+		   const Bound *b1,
+		   const Bound *b2)
 {
 	if (IsMinusInfinity(b1) || IsPlusInfinity(b2))
 		return -1;
@@ -139,10 +143,10 @@ typedef struct
 	Oid			   *children;		/* Oids of child partitions */
 	RangeEntry	   *ranges;			/* per-partition range entry or NULL */
 
-	const char	   *attname;		/* original expression */
+	const char	   *expr_cstr;		/* original expression */
 	Node		   *expr;			/* planned expression */
 	List		   *expr_vars;		/* vars from expression, lazy */
-	Bitmapset	   *expr_atts;		/* set with attnums from expression */
+	Bitmapset	   *expr_atts;		/* attnums from expression */
 
 	Oid				atttype;		/* expression type */
 	int32			atttypmod;		/* expression type modifier */
@@ -155,6 +159,8 @@ typedef struct
 					hash_proc;		/* hash function for 'atttype' */
 } PartRelationInfo;
 
+#define PART_EXPR_VARNO				( 1 )
+
 /*
  * PartParentInfo
  *		Cached parent of the specified partition.
@@ -166,18 +172,6 @@ typedef struct
 	Oid				parent_rel;
 } PartParentInfo;
 
-/*
- * CustomConst
- *		Modified Const that also stores 'varattno' attribute from some Var
- *		We can check that is CustomConst by checking `location` attrubute.
- *		It should be equal -2
- */
-typedef struct
-{
-	Const		cns;
-	AttrNumber	varattno;
-} CustomConst;
-
 /*
  * PartBoundInfo
  *		Cached bounds of the specified partition.
@@ -242,11 +236,13 @@ static inline List *
 PrelExpressionColumnNames(const PartRelationInfo *prel)
 {
 	List   *columns = NIL;
-	int		j = -1;
+	int		i = -1;
 
-	while ((j = bms_next_member(prel->expr_atts, j)) >= 0)
+	while ((i = bms_next_member(prel->expr_atts, i)) >= 0)
 	{
-		char *attname = get_attname(prel->key, j);
+		AttrNumber	attnum = i + FirstLowInvalidHeapAttributeNumber;
+		char	   *attname = get_attname(PrelParentRelid(prel), attnum);
+
 		columns = lappend(columns, makeString(attname));
 	}
 
@@ -257,15 +253,22 @@ PrelExpressionColumnNames(const PartRelationInfo *prel)
 const PartRelationInfo *refresh_pathman_relation_info(Oid relid,
 													  Datum *values,
 													  bool allow_incomplete);
-PartRelationInfo * invalidate_pathman_relation_info(Oid relid, bool *found);
+PartRelationInfo *invalidate_pathman_relation_info(Oid relid, bool *found);
 void remove_pathman_relation_info(Oid relid);
 const PartRelationInfo *get_pathman_relation_info(Oid relid);
 const PartRelationInfo *get_pathman_relation_info_after_lock(Oid relid,
 															 bool unlock_if_not_found,
 															 LockAcquireResult *lock_result);
 
-/* Expression related routines */
-void mark_pathman_expression_for_update(Oid relid);
+/* Partitioning expression routines */
+Node *parse_partitioning_expression(const Oid relid,
+									const char *expression,
+									char **query_string_out,
+									Node **parsetree_out);
+
+Datum plan_partitioning_expression(const Oid relid,
+								   const char *expr_cstr,
+								   Oid *expr_type);
 
 /* Global invalidation routines */
 void delay_pathman_shutdown(void);
@@ -280,12 +283,12 @@ Oid get_parent_of_partition(Oid partition, PartParentSearch *status);
 
 /* Bounds cache */
 void forget_bounds_of_partition(Oid partition);
-PartBoundInfo * get_bounds_of_partition(Oid partition,
-										const PartRelationInfo *prel);
+PartBoundInfo *get_bounds_of_partition(Oid partition,
+									   const PartRelationInfo *prel);
 
 /* Safe casts for PartType */
 PartType DatumGetPartType(Datum datum);
-char * PartTypeToCString(PartType parttype);
+char *PartTypeToCString(PartType parttype);
 
 /* PartRelationInfo checker */
 void shout_if_prel_is_invalid(const Oid parent_oid,
@@ -367,5 +370,6 @@ extern bool pg_pathman_enable_bounds_cache;
 
 void init_relation_info_static_data(void);
 
+
 #endif /* RELATION_INFO_H */
 

src/include/utils.h

@@ -65,7 +65,5 @@ Datum extract_binary_interval_from_text(Datum interval_text,
 char ** deconstruct_text_array(Datum array, int *array_size);
 RangeVar ** qualified_relnames_to_rangevars(char **relnames, size_t nrelnames);
 
-AttrNumber *get_pathman_attributes_map(const PartRelationInfo *prel,
-										Relation child);
 
 #endif /* PATHMAN_UTILS_H */

src/init.c

@@ -599,12 +599,11 @@ build_update_trigger_func_name_internal(Oid relid)
 
 /*
  * Check that relation 'relid' is partitioned by pg_pathman.
- *
- * Extract tuple into 'values' and 'isnull' if they're provided.
+ * Extract tuple into 'values', 'isnull', 'xmin', 'iptr' if they're provided.
  */
 bool
 pathman_config_contains_relation(Oid relid, Datum *values, bool *isnull,
-								 TransactionId *xmin, HeapTuple *tuple)
+								 TransactionId *xmin, ItemPointerData* iptr)
 {
 	Relation		rel;
 	HeapScanDesc	scan;
@@ -662,8 +661,9 @@ pathman_config_contains_relation(Oid relid, Datum *values, bool *isnull,
 			*xmin = DatumGetTransactionId(value);
 		}
 
-		if (tuple)
-			*tuple = heap_copytuple(htup);
+		/* Set ItemPointer if necessary */
+		if (iptr)
+			*iptr = htup->t_self;
 	}
 
 	/* Clean resources */
@@ -677,9 +677,78 @@ pathman_config_contains_relation(Oid relid, Datum *values, bool *isnull,
 	return contains_rel;
 }
 
+/* Invalidate parsed partitioning expression in PATHMAN_CONFIG */
+void
+pathman_config_invalidate_parsed_expression(Oid relid)
+{
+	ItemPointerData		iptr; /* pointer to tuple */
+	Datum				values[Natts_pathman_config];
+	bool				nulls[Natts_pathman_config];
+
+	/* Check that PATHMAN_CONFIG table contains this relation */
+	if (pathman_config_contains_relation(relid, values, nulls, NULL, &iptr))
+	{
+		Relation	rel;
+		HeapTuple	new_htup;
+
+		/* Reset parsed expression */
+		values[Anum_pathman_config_expression_p - 1] = (Datum) 0;
+		nulls[Anum_pathman_config_expression_p - 1]  = true;
+
+		/* Reset expression type */
+		values[Anum_pathman_config_atttype - 1] = (Datum) 0;
+		nulls[Anum_pathman_config_atttype - 1]  = true;
+
+		rel = heap_open(get_pathman_config_relid(false), RowExclusiveLock);
+
+		/* Form new tuple and perform an update */
+		new_htup = heap_form_tuple(RelationGetDescr(rel), values, nulls);
+		simple_heap_update(rel, &iptr, new_htup);
+		CatalogUpdateIndexes(rel, new_htup);
+
+		heap_close(rel, RowExclusiveLock);
+	}
+}
+
+/* Refresh parsed partitioning expression in PATHMAN_CONFIG */
+void
+pathman_config_refresh_parsed_expression(Oid relid,
+										 Datum *values,
+										 bool *isnull,
+										 ItemPointer iptr)
+{
+	char				   *expr_cstr;
+	Oid						expr_type;
+	Datum					expr_datum;
+
+	Relation				rel;
+	HeapTuple				htup_new;
+
+	/* get and parse expression */
+	expr_cstr = TextDatumGetCString(values[Anum_pathman_config_expression - 1]);
+	expr_datum = plan_partitioning_expression(relid, expr_cstr, &expr_type);
+	pfree(expr_cstr);
+
+	/* prepare tuple values */
+	values[Anum_pathman_config_expression_p - 1] = expr_datum;
+	isnull[Anum_pathman_config_expression_p - 1] = false;
+
+	values[Anum_pathman_config_atttype - 1] = ObjectIdGetDatum(expr_type);
+	isnull[Anum_pathman_config_atttype - 1] = false;
+
+	rel = heap_open(get_pathman_config_relid(false), RowExclusiveLock);
+
+	htup_new = heap_form_tuple(RelationGetDescr(rel), values, isnull);
+	simple_heap_update(rel, iptr, htup_new);
+	CatalogUpdateIndexes(rel, htup_new);
+
+	heap_close(rel, RowExclusiveLock);
+}
+
+
 /*
- * Loads additional pathman parameters like 'enable_parent' or 'auto'
- * from PATHMAN_CONFIG_PARAMS.
+ * Loads additional pathman parameters like 'enable_parent'
+ * or 'auto' from PATHMAN_CONFIG_PARAMS.
  */
 bool
 read_pathman_params(Oid relid, Datum *values, bool *isnull)
@@ -722,6 +791,7 @@ read_pathman_params(Oid relid, Datum *values, bool *isnull)
 	return row_found;
 }
 
+
 /*
  * Go through the PATHMAN_CONFIG table and create PartRelationInfo entries.
  */
@@ -788,10 +858,9 @@ read_pathman_config(void)
 
 /*
  * Validates range constraint. It MUST have one of the following formats:
- *
- *		EXPRESSION >= CONST AND EXPRESSION < CONST
- *		EXPRESSION >= CONST
- *		EXPRESSION < CONST
+ *		1) EXPRESSION >= CONST AND EXPRESSION < CONST
+ *		2) EXPRESSION >= CONST
+ *		3) EXPRESSION < CONST
  *
  * Writes 'lower' & 'upper' and 'lower_null' & 'upper_null' values on success.
  */
@@ -838,7 +907,11 @@ validate_range_constraint(const Expr *expr,
 									  lower, upper, lower_null, upper_null);
 }
 
-/* Validates a single expression of kind EXPRESSION >= CONST | EXPRESSION < CONST */
+/*
+ * Validates a single expression of kind:
+ *		1) EXPRESSION >= CONST
+ *		2) EXPRESSION < CONST
+ */
 static bool
 validate_range_opexpr(const Expr *expr,
 					  const PartRelationInfo *prel,