agora inbox for [email protected]  
help / color / mirror / Atom feed
From: Tomas Vondra <[email protected]>
Subject: [PATCH 2/2] Fix processing of OpExpr in extended statistics
Date: Sat, 13 Jul 2019 00:12:16 +0200

---
 src/backend/statistics/extended_stats.c | 64 ++++++++++++++++++++-----
 src/backend/statistics/mcv.c            | 23 +++------
 src/include/statistics/statistics.h     |  1 +
 3 files changed, 60 insertions(+), 28 deletions(-)

diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index 2ed28c054f..1c7ca07661 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -797,21 +797,13 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
 		RangeTblEntry *rte = root->simple_rte_array[relid];
 		OpExpr	   *expr = (OpExpr *) clause;
 		Var		   *var;
-		bool		varonleft = true;
-		bool		ok;
 
 		/* Only expressions with two arguments are considered compatible. */
 		if (list_length(expr->args) != 2)
 			return false;
 
-		/* see if it actually has the right shape (one Var, one Const) */
-		ok = (NumRelids((Node *) expr) == 1) &&
-			(is_pseudo_constant_clause(lsecond(expr->args)) ||
-			 (varonleft = false,
-			  is_pseudo_constant_clause(linitial(expr->args))));
-
-		/* unsupported structure (two variables or so) */
-		if (!ok)
+		/* Check if the expression the right shape (one Var, one Const) */
+		if (!deconstruct_opexpr(expr, &var, NULL, NULL))
 			return false;
 
 		/*
@@ -843,8 +835,6 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
 			!get_func_leakproof(get_opcode(expr->opno)))
 			return false;
 
-		var = (varonleft) ? linitial(expr->args) : lsecond(expr->args);
-
 		return statext_is_compatible_clause_internal(root, (Node *) var,
 													 relid, attnums);
 	}
@@ -1235,3 +1225,53 @@ determine_operator_strategy(Oid opno)
 
 	return strat;
 }
+
+/*
+ * deconstruct_opexpr
+ *		Split expression into Var and Const parts.
+ *
+ * Attempts to match the arguments to either (Var op Const) or (Const op Var),
+ * possibly with a RelabelType on top of the Var node. When successful, returns
+ * true, otherwise returns false.
+ *
+ * Optionally returns pointers to the extracted elements, when passed non-null
+ * pointers (varp, cstp and isgtp).
+ */
+bool
+deconstruct_opexpr(OpExpr *expr, Var **varp, Const **cstp, bool *isgtp)
+{
+	Var	   *var;
+	Const  *cst;
+	bool	isgt;
+
+	/* enforced by statext_is_compatible_clause_internal */
+	Assert(list_length(expr->args) == 2);
+
+	if ((IsA(linitial(expr->args), Var) || IsA(linitial(expr->args), RelabelType)) &&
+		IsA(lsecond(expr->args), Const))
+	{
+		var = linitial(expr->args);
+		cst = lsecond(expr->args);
+		isgt = false;
+	}
+	else if (IsA(linitial(expr->args), Const) &&
+			 (IsA(lsecond(expr->args), Var) || IsA(lsecond(expr->args), RelabelType)))
+	{
+		var = lsecond(expr->args);
+		cst = linitial(expr->args);
+		isgt = true;
+	}
+	else
+		return false;
+
+	if (varp)
+		*varp = var;
+
+	if (cstp)
+		*cstp = cst;
+
+	if (isgtp)
+		*isgtp = isgt;
+
+	return true;
+}
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index 1e919c82f9..0d7f6969ee 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1561,32 +1561,23 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
 		if (is_opclause(clause))
 		{
 			OpExpr	   *expr = (OpExpr *) clause;
-			bool		varonleft = true;
-			bool		ok;
 			FmgrInfo	opproc;
 
-			fmgr_info(get_opcode(expr->opno), &opproc);
+			/* valid only after deconstruct_opexpr returns true  */
+			Var		   *var;
+			Const	   *cst;
+			bool		isgt;
 
-			ok = (NumRelids(clause) == 1) &&
-				(is_pseudo_constant_clause(lsecond(expr->args)) ||
-				 (varonleft = false,
-				  is_pseudo_constant_clause(linitial(expr->args))));
+			fmgr_info(get_opcode(expr->opno), &opproc);
 
-			if (ok)
+			/* extract the var and const from the expression */
+			if (deconstruct_opexpr(expr, &var, &cst, &isgt))
 			{
 				TypeCacheEntry *typecache;
 				FmgrInfo	gtproc;
-				Var		   *var;
-				Const	   *cst;
-				bool		isgt;
 				int			idx;
 				int			strategy;
 
-				/* extract the var and const from the expression */
-				var = (varonleft) ? linitial(expr->args) : lsecond(expr->args);
-				cst = (varonleft) ? lsecond(expr->args) : linitial(expr->args);
-				isgt = (!varonleft);
-
 				/* strip binary-compatible relabeling */
 				if (IsA(var, RelabelType))
 					var = (Var *) ((RelabelType *) var)->arg;
diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h
index 82e2d5c619..f5120d0663 100644
--- a/src/include/statistics/statistics.h
+++ b/src/include/statistics/statistics.h
@@ -119,5 +119,6 @@ extern bool has_stats_of_kind(List *stats, char requiredkind);
 extern StatisticExtInfo *choose_best_statistics(List *stats,
 												Bitmapset *attnums, char requiredkind);
 extern int determine_operator_strategy(Oid opno);
+extern bool deconstruct_opexpr(OpExpr *expr, Var **varp, Const **cstp, bool *isgtp);
 
 #endif							/* STATISTICS_H */
-- 
2.20.1


--omnnplrhcrdpixl2--





view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: [PATCH 2/2] Fix processing of OpExpr in extended statistics
  In-Reply-To: <no-message-id-1882043@localhost>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox