diff-3.diff

text/plain

Filename: diff-3.diff
Type: text/plain
Part: 0
Message: Re: POC, WIP: OR-clause support for indexes

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
File+
src/backend/parser/parse_expr.c 42 80
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 961ca3e482..f0fd63f05c 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -112,9 +112,6 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 	List		   *or_list = NIL;
 	List		   *groups_list = NIL;
 	ListCell	   *lc;
-	Node 		   *result;
-	BoolExpr 	   *expr;
-	bool			change_apply = false;
 
 	/* If this is not an 'OR' expression, skip the transformation */
 	if (expr_orig->boolop != OR_EXPR ||
@@ -131,16 +128,7 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 		OrClauseGroupEntry *gentry;
 		bool				const_is_left = true;
 
-		/*
-		 * The first step: checking that the expression consists only of equality.
-		 * We can only do this here, while arg is still row data type, namely A_Expr.
-		 * After applying transformExprRecurce, we already know that it will be OpExr type,
-		 * but checking the expression for equality is already becoming impossible for us.
-		 * Sometimes we have the chance to devide expression into the groups on
-		 * equality and inequality. This is possible if all list items are not at the
-		 * same level of a single BoolExpr expression, otherwise all of them cannot be converted.
-		 */
-
+		/* At first, transform the arg and evaluate constant expressions. */
 		orqual = transformExprRecurse(pstate, (Node *) arg);
 		orqual = coerce_to_boolean(pstate, orqual, "OR");
 		orqual = eval_const_expressions(NULL, orqual);
@@ -151,7 +139,13 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 			continue;
 		}
 
-		/* Detect constant side of the clause */
+		/*
+		 * Detect the constant side of the clause. Recall non-constant
+		 * expression can be made not only with Vars, but also with Params,
+		 * which is not bonded with any relation. Thus, we detect the const
+		 * side - if another side is constant too, the orqual couldn't be
+		 * an OpExpr.
+		 */
 		if (IsA(get_leftop(orqual), Const))
 			const_is_left = true;
 		else if (IsA(get_rightop(orqual), Const))
@@ -175,26 +169,14 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 		}
 
 		/*
-		 * The next step: transform all the inputs, and detect whether
-		 * any contain Vars.
-		 */
-		if (!contain_vars_of_level(orqual, 0))
-		{
-			/* Again, it's not the expr we can transform */
-			or_list = lappend(or_list, orqual);
-			continue;
-		}
-
-		;
-
-		/*
-			* At this point we definitely have a transformable clause.
-			* Classify it and add into specific group of clauses, or create new
-			* group.
-			* TODO: to manage complexity in the case of many different clauses
-			* (X1=C1) OR (X2=C2 OR) ... (XN = CN) we could invent something
-			* like a hash table (htab key ???).
-			*/
+		* At this point we definitely have a transformable clause.
+		* Classify it and add into specific group of clauses, or create new
+		* group.
+		* TODO: to manage complexity in the case of many different clauses
+		* (X1=C1) OR (X2=C2 OR) ... (XN = CN) we could invent something
+		* like a hash table. But also we believe, that the case of many
+		* different variable sides is very rare.
+		*/
 		foreach(lc_groups, groups_list)
 		{
 			OrClauseGroupEntry *v = (OrClauseGroupEntry *) lfirst(lc_groups);
@@ -220,20 +202,18 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 		gentry = palloc(sizeof(OrClauseGroupEntry));
 		gentry->node = nconst_expr;
 		gentry->consts = list_make1(const_expr);
-		gentry->opno = ((OpExpr *)orqual)->opno;
-		gentry->expr = (Expr *)orqual;
+		gentry->expr = (Expr *) orqual;
 		groups_list = lappend(groups_list,  (void *) gentry);
 	}
 
 	if (groups_list == NIL)
 	{
 		/*
-		* No any transformations possible with this rinfo, just add itself
-		* to the list and go further.
+		* No any transformations possible with this list of arguments. Here we
+		* already made all underlying transformations. Thus, just return the
+		* transformed bool expression.
 		*/
-		list_free(or_list);
-
-		return transformBoolExpr(pstate, (BoolExpr *)expr_orig);
+		return (Node *) makeBoolExpr(OR_EXPR, or_list, expr_orig->location);
 	}
 	else
 	{
@@ -267,11 +247,12 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 			}
 
 			/*
-			 * Do the transformation. It's been a long way ;)
+			 * Do the transformation.
 			 *
-			 * First of all, try to select a common type for the array elements.  Note that
-			 * since the LHS' type is first in the list, it will be preferred when
-			 * there is doubt (eg, when all the RHS items are unknown literals).
+			 * First of all, try to select a common type for the array elements.
+			 * Note that since the LHS' type is first in the list, it will be
+			 * preferred when there is doubt (eg, when all the RHS items are
+			 * unknown literals).
 			 *
 			 * Note: use list_concat here not lcons, to avoid damaging rnonvars.
 			 *
@@ -288,8 +269,8 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 			if (array_type != InvalidOid)
 			{
 				/*
-				 * OK: coerce all the right-hand non-Var inputs to the common type
-				 * and build an ArrayExpr for them.
+				 * OK: coerce all the right-hand non-Var inputs to the common
+				 * type and build an ArrayExpr for them.
 				 */
 				List	   *aexprs;
 				ArrayExpr  *newa;
@@ -314,24 +295,18 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 				newa->array_typeid = get_array_type(scalar_type);
 				newa->multidims = false;
 				newa->elements = aexprs;
-				newa->location = -1; /* Position of the new clause is undefined */
+				newa->location = -1;
 
-				saopexpr = (ScalarArrayOpExpr *)make_scalar_array_op(pstate,
-												   list_make1(makeString((char *) "=")),
-												   true,
-												   gentry->node,
-												   (Node *) newa,
-												   -1); /* Position of the new clause is undefined */
-
-				/*
-				* TODO: here we can try to coerce the array to a Const and find
-				* hash func instead of linear search (see 50e17ad281b).
-				* convert_saop_to_hashed_saop((Node *) saopexpr);
-				* We don't have to do this anymore, do we?
-				*/
+				saopexpr =
+					(ScalarArrayOpExpr *)
+						make_scalar_array_op(pstate,
+											 list_make1(makeString((char *) "=")),
+											 true,
+											 gentry->node,
+											 (Node *) newa,
+											 -1);
 
 				or_list = lappend(or_list, (void *) saopexpr);
-				change_apply = true;
 			}
 			else
 			{
@@ -344,23 +319,10 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 		list_free_deep(groups_list);
 	}
 
-	if (change_apply)
-	{
-		/* One more trick: assemble correct clause */
-		expr = list_length(or_list) > 1 ? makeBoolExpr(OR_EXPR, list_copy(or_list), -1) : linitial(or_list);
-		result = (Node *)expr;
-	}
-	/*
-	 * There was no reasons to create a new expresion, so
-	 * run the original BoolExpr conversion with using
-	 * transformBoolExpr function
-	 */
-	else
-		result = transformBoolExpr(pstate, (BoolExpr *)expr_orig);
-
-	list_free(or_list);
-
-	return result;
+	/* One more trick: assemble correct clause */
+	return (Node *) ((list_length(or_list) > 1) ?
+						makeBoolExpr(OR_EXPR, or_list, expr_orig->location) :
+						linitial(or_list));
 }
 
 /*
@@ -475,7 +437,7 @@ transformExprRecurse(ParseState *pstate, Node *expr)
 			}
 
 		case T_BoolExpr:
-			result = (Node *)transformBoolExprOr(pstate, (BoolExpr *)expr);
+			result = (Node *)transformBoolExprOr(pstate, (BoolExpr *) expr);
 			break;
 
 		case T_FuncCall: