Rework examine_opclause_expression to use varonleft

The examine_opclause_expression function needs to return information on
which side of the operator we found the Var, but the variable was called
"isgt" which is rather misleading (it assumes the operator is either
less-than or greater-than, but it may be equality or something else).
Other places in the planner use a variable called "varonleft" for this
purpose, so just adopt the same convention here.

The code also assumed we don't care about this flag for equality, as
(Var = Const) and (Const = Var) should be the same thing. But that does
not work for cross-type operators, in which case we need to pass the
parameters to the procedure in the right order. So just use the same
code for all types of expressions.

This means we don't need to care about the selectivity estimation
function anymore, at least not in this code. We should only get the
supported cases here (thanks to statext_is_compatible_clause).

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/8736jdhbhc.fsf%40ansel.ydns.eu
Backpatch-to: 12

src/backend/statistics/extended_stats.c

@@ -1196,15 +1196,15 @@ statext_clauselist_selectivity(PlannerInfo *root, List *clauses, int varRelid,
  * returns true, otherwise returns false.
  *
  * Optionally returns pointers to the extracted Var/Const nodes, when passed
- * non-null pointers (varp, cstp and isgtp). The isgt flag specifies whether
+ * non-null pointers (varp, cstp and varonleftp). The varonleftp flag specifies
- * the Var node is on the left (false) or right (true) side of the operator.
+ * on which side of the operator we found the Var node.
  */
 bool
-examine_opclause_expression(OpExpr *expr, Var **varp, Const **cstp, bool *isgtp)
+examine_opclause_expression(OpExpr *expr, Var **varp, Const **cstp, bool *varonleftp)
 {
 	Var	   *var;
 	Const  *cst;
-	bool	isgt;
+	bool	varonleft;
 	Node   *leftop,
 		   *rightop;
 
@@ -1225,13 +1225,13 @@ examine_opclause_expression(OpExpr *expr, Var **varp, Const **cstp, bool *isgtp)
 	{
 		var = (Var *) leftop;
 		cst = (Const *) rightop;
-		isgt = false;
+		varonleft = true;
 	}
 	else if (IsA(leftop, Const) && IsA(rightop, Var))
 	{
 		var = (Var *) rightop;
 		cst = (Const *) leftop;
-		isgt = true;
+		varonleft = false;
 	}
 	else
 		return false;
@@ -1243,8 +1243,8 @@ examine_opclause_expression(OpExpr *expr, Var **varp, Const **cstp, bool *isgtp)
 	if (cstp)
 		*cstp = cst;
 
-	if (isgtp)
+	if (varonleftp)
-		*isgtp = isgt;
+		*varonleftp = varonleft;
 
 	return true;
 }

src/backend/statistics/mcv.c

@@ -1581,18 +1581,15 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
 			OpExpr	   *expr = (OpExpr *) clause;
 			FmgrInfo	opproc;
 
-			/* get procedure computing operator selectivity */
-			RegProcedure oprrest = get_oprrest(expr->opno);
-
 			/* valid only after examine_opclause_expression returns true */
 			Var		   *var;
 			Const	   *cst;
-			bool		isgt;
+			bool		varonleft;
 
 			fmgr_info(get_opcode(expr->opno), &opproc);
 
 			/* extract the var and const from the expression */
-			if (examine_opclause_expression(expr, &var, &cst, &isgt))
+			if (examine_opclause_expression(expr, &var, &cst, &varonleft))
 			{
 				int			idx;
 
@@ -1629,46 +1626,21 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
 					if (RESULT_IS_FINAL(matches[i], is_or))
 						continue;
 
-					switch (oprrest)
+					/*
-					{
+					 * First check whether the constant is below the lower
-						case F_EQSEL:
+					 * boundary (in that case we can skip the bucket, because
-						case F_NEQSEL:
+					 * there's no overlap).
-
+					 */
-							/*
+					if (varonleft)
-							 * We don't care about isgt in equality, because
+						match = DatumGetBool(FunctionCall2Coll(&opproc,
-							 * it does not matter whether it's (var op const)
+															   DEFAULT_COLLATION_OID,
-							 * or (const op var).
+															   item->values[idx],
-							 */
+															   cst->constvalue));
-							match = DatumGetBool(FunctionCall2Coll(&opproc,
+					else
-																   DEFAULT_COLLATION_OID,
+						match = DatumGetBool(FunctionCall2Coll(&opproc,
-																   cst->constvalue,
+															   DEFAULT_COLLATION_OID,
-																   item->values[idx]));
+															   cst->constvalue,
-
+															   item->values[idx]));
-							break;
-
-						case F_SCALARLTSEL: /* column < constant */
-						case F_SCALARLESEL: /* column <= constant */
-						case F_SCALARGTSEL: /* column > constant */
-						case F_SCALARGESEL: /* column >= constant */
-
-							/*
-							 * First check whether the constant is below the
-							 * lower boundary (in that case we can skip the
-							 * bucket, because there's no overlap).
-							 */
-							if (isgt)
-								match = DatumGetBool(FunctionCall2Coll(&opproc,
-																	   DEFAULT_COLLATION_OID,
-																	   cst->constvalue,
-																	   item->values[idx]));
-							else
-								match = DatumGetBool(FunctionCall2Coll(&opproc,
-																	   DEFAULT_COLLATION_OID,
-																	   item->values[idx],
-																	   cst->constvalue));
-
-							break;
-					}
 
 					/* update the match bitmap with the result */
 					matches[i] = RESULT_MERGE(matches[i], is_or, match);

src/include/statistics/extended_stats_internal.h

@@ -98,7 +98,7 @@ extern SortItem *build_sorted_items(int numrows, int *nitems, HeapTuple *rows,
 									int numattrs, AttrNumber *attnums);
 
 extern bool examine_opclause_expression(OpExpr *expr, Var **varp,
-										Const **cstp, bool *isgtp);
+										Const **cstp, bool *varonleftp);
 
 extern Selectivity mcv_clauselist_selectivity(PlannerInfo *root,
 											  StatisticExtInfo *stat,