light refactoring (utils.c etc)

Author: funbringer

src/include/partition_creation.h

@@ -19,7 +19,7 @@
 
 
 /* ACL privilege for partition creation */
-#define ACL_SPAWN_PARTITIONS	 ACL_INSERT
+#define ACL_SPAWN_PARTITIONS	ACL_INSERT
 
 
 /* Create RANGE partitions to store some value */
@@ -87,10 +87,14 @@ typedef struct
 
 /* Expression parsing functions */
 PartExpressionInfo *get_part_expression_info(Oid relid,
-	const char *expr_string, bool check_hash_func, bool make_plan);
-
-Node *get_raw_expression(Oid relid, const char *expr, char **query_string_out,
-		Node **parsetree);
+											 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/relation_info.h

@@ -18,10 +18,11 @@
 #include "nodes/bitmapset.h"
 #include "nodes/nodes.h"
 #include "nodes/primnodes.h"
+#include "nodes/value.h"
 #include "port/atomics.h"
 #include "storage/lock.h"
 #include "utils/datum.h"
-#include "nodes/primnodes.h"
+#include "utils/lsyscache.h"
 
 
 /* Range bound */
@@ -237,6 +238,21 @@ PrelLastChild(const PartRelationInfo *prel)
 	return PrelChildrenCount(prel) - 1; /* last partition */
 }
 
+static inline List *
+PrelExpressionColumnNames(const PartRelationInfo *prel)
+{
+	List   *columns = NIL;
+	int		j = -1;
+
+	while ((j = bms_next_member(prel->expr_atts, j)) >= 0)
+	{
+		char *attname = get_attname(prel->key, j);
+		columns = lappend(columns, makeString(attname));
+	}
+
+	return columns;
+}
+
 
 const PartRelationInfo *refresh_pathman_relation_info(Oid relid,
 													  Datum *values,

src/include/utils.h

@@ -28,7 +28,7 @@
 bool clause_contains_params(Node *clause);
 bool is_date_type_internal(Oid typid);
 bool check_security_policy_internal(Oid relid, Oid role);
-bool match_expr_to_operand(Node *operand, Node *expr);
+bool expr_matches_operand(Node *operand, Node *expr);
 
 /*
  * Misc.
@@ -68,6 +68,4 @@ RangeVar ** qualified_relnames_to_rangevars(char **relnames, size_t nrelnames);
 AttrNumber *get_pathman_attributes_map(const PartRelationInfo *prel,
 										Relation child);
 
-List *get_part_expression_columns(const PartRelationInfo *prel);
-
 #endif /* PATHMAN_UTILS_H */

src/init.c

@@ -1037,7 +1037,7 @@ validate_hash_constraint(const Expr *expr,
 		hash_arg = (Node *) linitial(type_hash_proc_expr->args);
 
 		/* Check arg of TYPE_HASH_PROC() */
-		if (!match_expr_to_operand(prel->expr, hash_arg))
+		if (!expr_matches_operand(prel->expr, hash_arg))
 			return false;
 
 		/* Check that PARTITIONS_COUNT is equal to total amount of partitions */

src/nodes_common.c

@@ -335,10 +335,10 @@ check_clause_for_expression(Node *node, struct check_clause_context *ctx)
 		Node   *left	= linitial(expr->args),
 			   *right	= lsecond(expr->args);
 
-		if (match_expr_to_operand(left, ctx->prel_expr))
+		if (expr_matches_operand(left, ctx->prel_expr))
 			ctx->count += 1;
 
-		if (match_expr_to_operand(right, ctx->prel_expr))
+		if (expr_matches_operand(right, ctx->prel_expr))
 			ctx->count += 1;
 
 		return false;

src/partition_creation.c

@@ -84,11 +84,13 @@ static void postprocess_child_table_and_atts(Oid parent_relid, Oid partition_rel
 static Oid text_to_regprocedure(text *proname_args);
 
 static Constraint *make_constraint_common(char *name, Node *raw_expr);
-static Node *get_constraint_expression(Oid parent_relid,
-									   Oid *expr_type, List **columns);
 static Value make_string_value_struct(char *str);
 static Value make_int_value_struct(int int_val);
 
+static Node *get_partitioning_expression(Oid parent_relid,
+										 Oid *expr_type,
+										 List **columns);
+
 /*
  * ---------------------------------------
  *  Public interface (partition creation)
@@ -123,7 +125,7 @@ create_single_range_partition_internal(Oid parent_relid,
 	}
 
 	/* check pathman config and fill variables */
-	expr = get_constraint_expression(parent_relid, NULL, &trigger_columns);
+	expr = get_partitioning_expression(parent_relid, NULL, &trigger_columns);
 
 	/* Create a partition & get 'partitioning expression' */
 	partition_relid = create_single_partition_internal(parent_relid,
@@ -163,7 +165,7 @@ create_single_hash_partition_internal(Oid parent_relid,
 									  char *tablespace)
 {
 	Oid						partition_relid,
-							value_type;
+							expr_type;
 	Constraint			   *check_constr;
 	Node				   *expr;
 	init_callback_params	callback_params;
@@ -187,14 +189,14 @@ create_single_hash_partition_internal(Oid parent_relid,
 													   tablespace);
 
 	/* check pathman config and fill variables */
-	expr = get_constraint_expression(parent_relid, &value_type, &trigger_columns);
+	expr = get_partitioning_expression(parent_relid, &expr_type, &trigger_columns);
 
 	/* Build check constraint for HASH partition */
 	check_constr = build_hash_check_constraint(partition_relid,
 											   expr,
 											   part_idx,
 											   part_count,
-											   value_type);
+											   expr_type);
 
 	/* Cook args for init_callback */
 	MakeInitCallbackHashParams(&callback_params,
@@ -1710,39 +1712,33 @@ validate_part_expression(Node *node, void *context)
 	return expression_tree_walker(node, validate_part_expression, context);
 }
 
-/* Wraps expression by SELECT query and returns parsed tree */
+/* Wraps expression by SELECT query and returns parse tree */
 Node *
-get_raw_expression(Oid relid, const char *expr, char **query_string_out,
-		Node **parsetree)
+parse_partitioning_expression(Oid relid,
+							  const char *expression,
+							  char **query_string_out,
+							  Node **parsetree_out)
 {
-	Node			*result;
-	SelectStmt		*select_stmt;
-	ResTarget		*target;
+	SelectStmt *select_stmt;
+	List	   *parsetree_list;
 
-	char			*fmt = "SELECT (%s) FROM ONLY %s.\"%s\"";
-	char			*relname = get_rel_name(relid),
-					*namespace_name = get_namespace_name(get_rel_namespace(relid));
-	List			*parsetree_list;
-	char			*query_string = psprintf(fmt, expr, namespace_name, relname);
+	char	   *sql = "SELECT (%s) FROM ONLY %s.\"%s\"";
+	char	   *relname = get_rel_name(relid),
+			   *nspname = get_namespace_name(get_rel_namespace(relid));
+	char	   *query_string = psprintf(sql, expression, nspname, relname);
 
 	parsetree_list = raw_parser(query_string);
 	Assert(list_length(parsetree_list) == 1);
 
+	select_stmt = (SelectStmt *) linitial(parsetree_list);
+
 	if (query_string_out)
-	{
 		*query_string_out = query_string;
-	}
 
-	select_stmt = (SelectStmt *) linitial(parsetree_list);
+	if (parsetree_out)
+		*parsetree_out = (Node *) select_stmt;
 
-	if (parsetree)
-	{
-		*parsetree = (Node *) select_stmt;
-	}
-
-	target = (ResTarget *) linitial(select_stmt->targetList);
-	result = (Node *) target->val;
-	return result;
+	return ((ResTarget *) linitial(select_stmt->targetList))->val;
 }
 
 /*
@@ -1751,7 +1747,7 @@ get_raw_expression(Oid relid, const char *expr, char **query_string_out,
  */
 PartExpressionInfo *
 get_part_expression_info(Oid relid, const char *expr_string,
-		bool check_hash_func, bool make_plan)
+						 bool check_hash_func, bool make_plan)
 {
 	Node				*expr_node,
 						*parsetree;
@@ -1766,12 +1762,12 @@ get_part_expression_info(Oid relid, const char *expr_string,
 	expr_info = palloc(sizeof(PartExpressionInfo));
 
 	pathman_parse_context = AllocSetContextCreate(TopPathmanContext,
-											"pathman parse context",
-											ALLOCSET_DEFAULT_SIZES);
+												  "pathman parse context",
+												  ALLOCSET_DEFAULT_SIZES);
 
 	/* Keep raw expression */
-	expr_info->raw_expr = get_raw_expression(relid, expr_string,
-			&query_string, &parsetree);
+	expr_info->raw_expr = parse_partitioning_expression(relid, expr_string,
+											 &query_string, &parsetree);
 
 	/* If expression is just column we check that is not null */
 	if (IsA(expr_info->raw_expr, ColumnRef))
@@ -1892,12 +1888,11 @@ extract_column_names(Node *node, struct extract_column_names_context *ctx)
 	return raw_expression_tree_walker(node, extract_column_names, ctx);
 }
 
-/*
- * Returns raw partitioning expression, and if specified returns
- * columns from expression and its type
- */
+/* Returns raw partitioning expression + expr_type + columns */
 static Node *
-get_constraint_expression(Oid parent_relid, Oid *expr_type, List **columns)
+get_partitioning_expression(Oid parent_relid,
+							Oid *expr_type,		/* ret val #1 */
+							List **columns)		/* ret val #2 */
 {
 	/* Values extracted from PATHMAN_CONFIG */
 	Datum		 config_values[Natts_pathman_config];
@@ -1906,28 +1901,24 @@ get_constraint_expression(Oid parent_relid, Oid *expr_type, List **columns)
 	char		*expr_string;
 
 	/* Check that table is registered in PATHMAN_CONFIG */
-	if (!pathman_config_contains_relation(parent_relid,
-										  config_values, config_nulls, NULL, NULL))
+	if (!pathman_config_contains_relation(parent_relid, config_values,
+										  config_nulls, NULL, NULL))
 		elog(ERROR, "table \"%s\" is not partitioned",
 			 get_rel_name_or_relid(parent_relid));
 
-	/*
-	 * We need expression type for hash functions. Range functions don't need
-	 * this feature.
-	 */
+	/* We need expression type for hash functions */
 	if (expr_type)
 		*expr_type = DatumGetObjectId(config_values[Anum_pathman_config_atttype - 1]);
 
 	expr_string = TextDatumGetCString(config_values[Anum_pathman_config_expression - 1]);
-	expr = get_raw_expression(parent_relid, expr_string, NULL, NULL);
+	expr = parse_partitioning_expression(parent_relid, expr_string, NULL, NULL);
 	pfree(expr_string);
 
 	if (columns)
 	{
-		struct extract_column_names_context ctx;
-		ctx.columns = NIL;
-		extract_column_names(expr, &ctx);
-		*columns = ctx.columns;
+		struct extract_column_names_context context = { NIL };
+		extract_column_names(expr, &context);
+		*columns = context.columns;
 	}
 
 	return expr;

src/pg_pathman.c

@@ -868,7 +868,7 @@ handle_arrexpr(const ScalarArrayOpExpr *expr, WalkerContext *context)
 
 	Assert(exprnode != NULL);
 
-	if (!match_expr_to_operand(context->prel_expr, exprnode))
+	if (!expr_matches_operand(context->prel_expr, exprnode))
 		goto handle_arrexpr_return;
 
 	if (arraynode && IsA(arraynode, Const) &&
@@ -1146,14 +1146,14 @@ pull_var_param(const WalkerContext *ctx,
 	Node   *left = linitial(expr->args),
 		   *right = lsecond(expr->args);
 
-	if (match_expr_to_operand(left, ctx->prel_expr))
+	if (expr_matches_operand(left, ctx->prel_expr))
 	{
 		*var_ptr = left;
 		*param_ptr = right;
 		return true;
 	}
 
-	if (match_expr_to_operand(right, ctx->prel_expr))
+	if (expr_matches_operand(right, ctx->prel_expr))
 	{
 		*var_ptr = right;
 		*param_ptr = left;

src/pl_funcs.c

@@ -1332,7 +1332,7 @@ create_update_triggers(PG_FUNCTION_ARGS)
 	trigname = build_update_trigger_name_internal(parent);
 
 	/* Create trigger for parent */
-	columns = get_part_expression_columns(prel);
+	columns = PrelExpressionColumnNames(prel);
 	create_single_update_trigger_internal(parent, trigname, columns);
 
 	/* Fetch children array */
@@ -1363,7 +1363,7 @@ create_single_update_trigger(PG_FUNCTION_ARGS)
 	trigname = build_update_trigger_name_internal(parent);
 
 	/* Generate list of columns used in expression */
-	columns = get_part_expression_columns(prel);
+	columns = PrelExpressionColumnNames(prel);
 	create_single_update_trigger_internal(child, trigname, columns);
 
 	PG_RETURN_VOID();

src/pl_range_funcs.c

@@ -60,8 +60,8 @@ static void merge_range_partitions_internal(Oid parent,
 											Oid *parts,
 											uint32 nparts);
 static void modify_range_constraint(Oid child_relid,
-									const char *attname,
-									Oid atttype,
+									const char *expression,
+									Oid expression_type,
 									const Bound *lower,
 									const Bound *upper);
 static char *get_qualified_rel_name(Oid relid);
@@ -554,7 +554,7 @@ build_range_condition(PG_FUNCTION_ARGS)
 				MakeBoundInf(PLUS_INFINITY) :
 				MakeBound(PG_GETARG_DATUM(3));
 
-	expr = get_raw_expression(partition_relid, expression, NULL, NULL);
+	expr = parse_partitioning_expression(partition_relid, expression, NULL, NULL);
 	con = build_range_check_constraint(partition_relid,
 									   expr,
 									   &min, &max,
@@ -1016,8 +1016,8 @@ interval_is_trivial(Oid atttype, Datum interval, Oid interval_type)
  */
 static void
 modify_range_constraint(Oid child_relid,
-						const char *attname,
-						Oid atttype,
+						const char *expression,
+						Oid expression_type,
 						const Bound *lower,
 						const Bound *upper)
 {
@@ -1029,14 +1029,14 @@ modify_range_constraint(Oid child_relid,
 	drop_check_constraint(child_relid);
 
 	/* Parse expression */
-	expr = get_raw_expression(child_relid, attname, NULL, NULL);
+	expr = parse_partitioning_expression(child_relid, expression, NULL, NULL);
 
 	/* Build a new one */
 	constraint = build_range_check_constraint(child_relid,
 											  expr,
 											  lower,
 											  upper,
-											  atttype);
+											  expression_type);
 
 	/* Open the relation and add new check constraint */
 	partition_rel = heap_open(child_relid, AccessExclusiveLock);