I agreed with the changes. Thank you for your work.

I updated patch and added you to the authors.

I specified Ranier Vilela as a reviewer.

On 10.07.2023 06:12, Andrey Lepikhov wrote:
On 7/7/2023 15:20, Alena Rybakina wrote:

because we will provide similar manipulation in this:

foreach(l, gentry->consts)
{
       Node       *rexpr = (Node *) lfirst(l);

       rexpr = coerce_to_common_type(pstate, rexpr,
                                                 scalar_type,
                                                 "IN");
      aexprs = lappend(aexprs, rexpr);
}
I'm not sure that it should be replaced.
In attachment - a bit more corrections to the patch.
The most important change - or_list contains already transformed expression subtree. So, I think we don't need to free it at all.

--
Regards,
Alena Rybakina
Postgres Professional
From 97239cdeac541d3aaeafde3d15a5b6fdc36f86de Mon Sep 17 00:00:00 2001
From: Alena Rybakina <a.rybak...@postgrespro.ru>
Date: Thu, 29 Jun 2023 17:46:58 +0300
Subject: [PATCH] Replace OR clause to ANY expressions. Replace (X=N1) OR
 (X=N2) ... with X = ANY(N1, N2) on the stage of the optimiser when we are
 still working with a tree expression. Firstly, we do not try to make a
 transformation for "non-or" expressions or inequalities and the creation of a
 relation with "or" expressions occurs according to the same scenario.
 Secondly, we do not make transformations if there are less than 500 or
 expressions. Thirdly, it is worth considering that we consider "or"
 expressions only at the current level.

Authors: Alena Rybakina <lena.riback...@yandex.ru>, Andrey Lepikhov <a.lepik...@postgrespro.ru>
Reviewed-by: Ranier Vilela <ranier...@gmail.com>
---
 src/backend/parser/parse_expr.c  | 231 ++++++++++++++++++++++++++++++-
 src/tools/pgindent/typedefs.list |   1 +
 2 files changed, 231 insertions(+), 1 deletion(-)

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 346fd272b6d..0ddcb880ef8 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -95,6 +95,235 @@ static Expr *make_distinct_op(ParseState *pstate, List *opname,
 static Node *make_nulltest_from_distinct(ParseState *pstate,
 										 A_Expr *distincta, Node *arg);
 
+typedef struct OrClauseGroupEntry
+{
+	Node		   *node;
+	List		   *consts;
+	Oid				scalar_type;
+	Oid				opno;
+	Expr 		   *expr;
+} OrClauseGroupEntry;
+
+static int const_transform_or_limit = 500;
+
+static Node *
+transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
+{
+	List		   *or_list = NIL;
+	List		   *groups_list = NIL;
+	ListCell	   *lc;
+
+	/* If this is not an 'OR' expression, skip the transformation */
+	if (expr_orig->boolop != OR_EXPR ||
+		list_length(expr_orig->args) < const_transform_or_limit)
+		return transformBoolExpr(pstate, (BoolExpr *) expr_orig);
+
+	foreach(lc, expr_orig->args)
+	{
+		Node			   *arg = lfirst(lc);
+		Node			   *orqual;
+		Node			   *const_expr;
+		Node			   *nconst_expr;
+		ListCell		   *lc_groups;
+		OrClauseGroupEntry *gentry;
+		bool				const_is_left = true;
+
+		/* 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);
+
+		if (!IsA(orqual, OpExpr))
+		{
+			or_list = lappend(or_list, orqual);
+			continue;
+		}
+
+		/*
+		 * 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))
+			const_is_left = false;
+		else
+		{
+			or_list = lappend(or_list, orqual);
+			continue;
+		}
+
+		/* Get pointers to constant and expression sides of the qual */
+		nconst_expr = (const_is_left) ? get_rightop(orqual) :
+										get_leftop(orqual);
+		const_expr = (const_is_left) ?  get_leftop(orqual) :
+										get_rightop(orqual);
+
+		if (!op_mergejoinable(((OpExpr *) orqual)->opno, exprType(nconst_expr)))
+		{
+			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. 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);
+
+			Assert(v->node != NULL);
+
+			if (equal(v->node, nconst_expr))
+			{
+				v->consts = lappend(v->consts, const_expr);
+				nconst_expr = NULL;
+				break;
+			}
+		}
+
+		if (nconst_expr == NULL)
+			/*
+				* The clause classified successfully and added into existed
+				* clause group.
+				*/
+			continue;
+
+		/* New clause group needed */
+		gentry = palloc(sizeof(OrClauseGroupEntry));
+		gentry->node = nconst_expr;
+		gentry->consts = list_make1(const_expr);
+		gentry->expr = (Expr *) orqual;
+		groups_list = lappend(groups_list,  (void *) gentry);
+	}
+
+	if (groups_list == NIL)
+	{
+		/*
+		* No any transformations possible with this list of arguments. Here we
+		* already made all underlying transformations. Thus, just return the
+		* transformed bool expression.
+		*/
+		return (Node *) makeBoolExpr(OR_EXPR, or_list, expr_orig->location);
+	}
+	else
+	{
+		ListCell	   *lc_args;
+
+		/* Let's convert each group of clauses to an IN operation. */
+
+		/*
+		* Go through the list of groups and convert each, where number of
+		* consts more than 1. trivial groups move to OR-list again
+		*/
+
+		foreach(lc_args, groups_list)
+		{
+			OrClauseGroupEntry *gentry = (OrClauseGroupEntry *) lfirst(lc_args);
+			List			   *allexprs;
+			Oid				    scalar_type;
+			Oid					array_type;
+
+			Assert(list_length(gentry->consts) > 0);
+
+			if (list_length(gentry->consts) == 1)
+			{
+				/*
+				 * Only one element in the class. Return rinfo into the BoolExpr
+				 * args list unchanged.
+				 */
+				list_free(gentry->consts);
+				or_list = lappend(or_list, gentry->expr);
+				continue;
+			}
+
+			/*
+			 * 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).
+			 *
+			 * Note: use list_concat here not lcons, to avoid damaging rnonvars.
+			 *
+			 * As a source of insides, use make_scalar_array_op()
+			 */
+			allexprs = list_concat(list_make1(gentry->node), gentry->consts);
+			scalar_type = select_common_type(NULL, allexprs, NULL, NULL);
+
+			if (OidIsValid(scalar_type) && scalar_type != RECORDOID)
+				array_type = get_array_type(scalar_type);
+			else
+				array_type = InvalidOid;
+
+			if (array_type != InvalidOid)
+			{
+				/*
+				 * OK: coerce all the right-hand non-Var inputs to the common
+				 * type and build an ArrayExpr for them.
+				 */
+				List	   *aexprs;
+				ArrayExpr  *newa;
+				ScalarArrayOpExpr *saopexpr;
+				ListCell *l;
+
+				aexprs = NIL;
+
+				foreach(l, gentry->consts)
+				{
+					Node	   *rexpr = (Node *) lfirst(l);
+
+					rexpr = coerce_to_common_type(pstate, rexpr,
+												scalar_type,
+												"IN");
+					aexprs = lappend(aexprs, rexpr);
+				}
+
+				newa = makeNode(ArrayExpr);
+				/* array_collid will be set by parse_collate.c */
+				newa->element_typeid = scalar_type;
+				newa->array_typeid = get_array_type(scalar_type);
+				newa->multidims = false;
+				newa->elements = aexprs;
+				newa->location = -1;
+
+				saopexpr =
+					(ScalarArrayOpExpr *)
+						make_scalar_array_op(pstate,
+											 list_make1(makeString((char *) "=")),
+											 true,
+											 gentry->node,
+											 (Node *) newa,
+											 -1);
+
+				or_list = lappend(or_list, (void *) saopexpr);
+			}
+			else
+			{
+				list_free(gentry->consts);
+				or_list = lappend(or_list, gentry->expr);
+				continue;
+			}
+		}
+
+		list_free_deep(groups_list);
+	}
+
+	/* One more trick: assemble correct clause */
+	return (Node *) ((list_length(or_list) > 1) ?
+						makeBoolExpr(OR_EXPR, or_list, expr_orig->location) :
+						linitial(or_list));
+}
 
 /*
  * transformExpr -
@@ -208,7 +437,7 @@ transformExprRecurse(ParseState *pstate, Node *expr)
 			}
 
 		case T_BoolExpr:
-			result = transformBoolExpr(pstate, (BoolExpr *) expr);
+			result = (Node *)transformBoolExprOr(pstate, (BoolExpr *) expr);
 			break;
 
 		case T_FuncCall:
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index e941fb6c82f..c3abb725c8c 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1631,6 +1631,7 @@ NumericVar
 OM_uint32
 OP
 OSAPerGroupState
+OrClauseGroupEntry
 OSAPerQueryState
 OSInfo
 OSSLCipher
-- 
2.34.1

Reply via email to