fix function get_pathman_relation_info(), refactoring for function validate_range_constraint()

funbringer · funbringer · commit 609813b906f3 · 2017-02-09T14:57:27.000+03:00
diff --git a/src/init.c b/src/init.c
@@ -80,6 +80,12 @@ static bool validate_range_constraint(const Expr *expr,
 									  const AttrNumber part_attno,
 									  Datum *lower, Datum *upper,
 									  bool *lower_null, bool *upper_null);
+static bool validate_range_opexpr(const Expr *expr,
+								  const PartRelationInfo *prel,
+								  const TypeCacheEntry *tce,
+								  const AttrNumber part_attno,
+								  Datum *lower, Datum *upper,
+								  bool *lower_null, bool *upper_null);
 
 static bool validate_hash_constraint(const Expr *expr,
 									 const PartRelationInfo *prel,
@@ -920,6 +926,65 @@ cmp_range_entries(const void *p1, const void *p2, void *arg)
 	return cmp_bounds(flinfo, &v1->min, &v2->min);
 }
 
+/* Validates a single expression of kind VAR >= CONST or VAR < CONST */
+static bool
+validate_range_opexpr(const Expr *expr,
+					  const PartRelationInfo *prel,
+					  const TypeCacheEntry *tce,
+					  const AttrNumber part_attno,
+					  Datum *lower, Datum *upper,
+					  bool *lower_null, bool *upper_null)
+{
+	const OpExpr   *opexpr;
+	Datum			val;
+
+	if (!expr)
+		return false;
+
+	/* Fail fast if it's not an OpExpr node */
+	if(!IsA(expr, OpExpr))
+		return false;
+
+	/* Perform cast */
+	opexpr = (const OpExpr *) expr;
+
+	/* Try reading Const value */
+	if (!read_opexpr_const(opexpr, prel, part_attno, &val))
+		return false;
+
+	/* Examine the strategy (expect '>=' OR '<') */
+	switch (get_op_opfamily_strategy(opexpr->opno, tce->btree_opf))
+	{
+		case BTGreaterEqualStrategyNumber:
+			{
+				/* Bound already exists */
+				if (*lower_null == false)
+					return false;
+
+				*lower_null = false;
+				*lower = val;
+
+				return true;
+			}
+
+		case BTLessStrategyNumber:
+			{
+				/* Bound already exists */
+				if (*upper_null == false)
+					return false;
+
+				*upper_null = false;
+				*upper = val;
+
+				return true;
+			}
+
+		default:
+			return false;
+	}
+}
+
+
 /*
  * Validates range constraint. It MUST have one of the following formats:
  *
@@ -936,66 +1001,41 @@ validate_range_constraint(const Expr *expr,
 						  Datum *lower, Datum *upper,
 						  bool *lower_null, bool *upper_null)
 {
-	const TypeCacheEntry   *tce;
-	const OpExpr		   *opexpr;
-	int						strategy;
-
-/* Validates a single expression of kind VAR >= CONST or VAR < CONST */
-#define validate_range_expr(expr, part_attno)								\
-	{																		\
-		Datum val;															\
-		opexpr = (OpExpr *) (expr);											\
-		strategy = get_op_opfamily_strategy(opexpr->opno, tce->btree_opf);	\
-																			\
-		/* Get const value */												\
-		if (!read_opexpr_const(opexpr, prel, part_attno, &val))				\
-			return false;													\
-																			\
-		/* Set min or max depending on operator */							\
-		switch (strategy)													\
-		{																	\
-			case BTGreaterEqualStrategyNumber:								\
-				*lower_null = false;										\
-				*lower = val;												\
-				break;														\
-			case BTLessStrategyNumber:										\
-				*upper_null = false;										\
-				*upper = val;												\
-				break;														\
-			default:														\
-				return false;												\
-		}																	\
-	}
+	const TypeCacheEntry *tce;
 
 	if (!expr)
 		return false;
+
+	/* Set default values */
 	*lower_null = *upper_null = true;
+
+	/* Find type cache entry for partitioned column's type */
 	tce = lookup_type_cache(prel->atttype, TYPECACHE_BTREE_OPFAMILY);
 
-	/* It could be either AND operator on top or just an OpExpr */
+	/* Is it an AND clause? */
 	if (and_clause((Node *) expr))
 	{
-		const BoolExpr	*boolexpr = (const BoolExpr *) expr;
-		ListCell		*lc;
+		const BoolExpr *boolexpr = (const BoolExpr *) expr;
+		ListCell	   *lc;
 
+		/* Walk through boolexpr's args */
 		foreach (lc, boolexpr->args)
 		{
-			Node	   *arg = lfirst(lc);
+			const OpExpr *opexpr = (const OpExpr *) lfirst(lc);
 
-			if(!IsA(arg, OpExpr))
+			/* Exit immediately if something is wrong */
+			if (!validate_range_opexpr((const Expr *) opexpr, prel, tce, part_attno,
+									   lower, upper, lower_null, upper_null))
 				return false;
-
-			validate_range_expr(arg, part_attno);
 		}
-		return true;
-	}
-	else if(IsA(expr, OpExpr))
-	{
-		validate_range_expr(expr, part_attno);
+
+		/* Everything seems to be fine */
 		return true;
 	}
 
-	return false;
+	/* It might be just an OpExpr clause */
+	else return validate_range_opexpr(expr, prel, tce, part_attno,
+									  lower, upper, lower_null, upper_null);
 }
 
 /*
diff --git a/src/relation_info.c b/src/relation_info.c
@@ -296,8 +296,13 @@ get_pathman_relation_info(Oid relid)
 			/* TODO: possible refactoring, pass found 'prel' instead of searching */
 			prel = refresh_pathman_relation_info(relid, part_type, attname, false);
 		}
+
 		/* Else clear remaining cache entry */
-		else remove_pathman_relation_info(relid);
+		else
+		{
+			remove_pathman_relation_info(relid);
+			prel = NULL; /* don't forget to reset 'prel' */
+		}
 	}
 
 	elog(DEBUG2,

Original file line number	Diff line number	Diff line change
`@@ -296,8 +296,13 @@ get_pathman_relation_info(Oid relid)`
`296`	`296`	`/* TODO: possible refactoring, pass found 'prel' instead of searching */`
`297`	`297`	`prel = refresh_pathman_relation_info(relid, part_type, attname, false);`
`298`	`298`	`}`
	`299`	`+`
`299`	`300`	`/* Else clear remaining cache entry */`
`300`		`- else remove_pathman_relation_info(relid);`
	`301`	`+ else`
	`302`	`+ {`
	`303`	`+ remove_pathman_relation_info(relid);`
	`304`	`+ prel = NULL; /* don't forget to reset 'prel' */`
	`305`	`+ }`
`301`	`306`	`}`
`302`	`307`
`303`	`308`	`elog(DEBUG2,`