Subject: Re: POC, WIP: OR-clause support for indexes

From: Alexander Korotkov <[email-masked]>
Date: Mon, 17 Jun 2024 15:11:09 +0300

On Mon, Jun 17, 2024 at 1:33 PM Alena Rybakina
<[email-masked]> wrote:
> I noticed that 7 libraries have been added to src/backend/optimizer/plan/initsplan.c, and as far as I remember, Tom Lane has already expressed doubts about the approach that requires adding a large number of libraries [0], but I'm afraid I'm out of ideas about alternative approach.

Thank you for pointing.  Right, the number of extra headers included
was one of points for criticism on this patch.  I'll look to move this
functionality elsewhere, while the stage of transformation could
probably be the same.

> In addition, I checked the fix in the previous cases that you wrote earlier [1] and noticed that SeqScan continues to generate, unfortunately, without converting expressions:

I've rechecked and see I made wrong conclusion about this.  The plan
regression is still here.  But I'm still looking to workaround this
without extra GUC.

I think we need to additionally do something like [1], but take
further steps to avoid planning overhead when not necessary.  In
particular, I think we should only consider splitting SAOP for bitmap
OR in the following cases:
1. There are partial indexes with predicates over target column.
2. There are multiple indexes covering target column and different
subsets of other columns presented in restrictions.
3. There are indexes covreing target column without support of SAOP
(amsearcharray == false).
Hopefully this should skip generation of useless bitmap paths in
majority cases. Thoughts?

Links.
1. https://www.postgresql.org/message-id/67bd918d-285e-44d2-a207-f52d9a4c35e6%40postgrespro.ru

------
Regards,
Alexander Korotkov
Supabase

---

From: Alena Rybakina <[email-masked]>
Date: Mon, 17 Jun 2024 18:21:09 +0300

> functionality elsewhere, while the stage of transformation could
Yes, I thing so.
> further steps to avoid planning overhead when not necessary.

Iagreewithyoutoreconsiderthisplacein detailonceagain,becauseotherwiseit 
lookslike we're likelyto runinto aperformanceissue.
> In
Frankly, I see that we will need to split SAOP anyway to check it, right?
> subsets of other columns presented in restrictions.
I see two cases in one. First, we need to check whether there is an 
index for the columns specified in the restrictlist, and secondly, the 
index ranges for which the conditions fall into the "OR" expressions.
> majority cases. Thoughts?
I'm notsureIfullyunderstandhowusefulthiscanbe.Couldyouexplainit to mein 
more detail?
> Links.
> 1.https://www.postgresql.org/message-id/67bd918d-285e-44d2-a207-f52d9a4c35e6%40postgrespro.ru

-- 
Regards,
Alena Rybakina
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

---

From: Nikolay Shaplov <[email-masked]>
Date: Mon, 24 Jun 2024 23:33:29 +0300

Hi!

Let me join the review process.

I am no expert in execution plans, so there would not be much help in doing 
even better optimization. But I can read the code, as a person who is not familiar 
with this area and help making it clear even to a person like me.

So, I am reading v25-0001-Transform-OR-clauses-to-ANY-expression.patch that 
have been posted some time ago, and especially transform_or_to_any function.

> @@ -38,7 +45,6 @@
>  int			from_collapse_limit;
>  int			join_collapse_limit;
>
> - 
>  /*
>   * deconstruct_jointree requires multiple passes over the join tree,  because we 
>   * need to finish computing JoinDomains before we start  distributing quals.

Do not think that removing empty line should be part of the patch

> +			/*
> +			 * If the const node's (right side of operator expression) type
> +			 * don't have â€œtrueâ€ array type, then we cannnot do the
> +			 * transformation. We simply concatenate the expression node.
> +			 */
Guess using unicode double quotes is not the best idea here... 
   
Now to the first part of  `transform_or_to_any`

---

From: Nikolay Shaplov <[email-masked]>
Date: Mon, 24 Jun 2024 23:51:56 +0300

Hi!

I am no expert in execution plans, so there would not be much help in doing 
even better optimization. But I can read the code, as a person who is not 
familiar 
with this area and help making it clear even to a person like me.

   
Now to the first part of  `transform_or_to_any`:

> +	List	   *entries = NIL;
I guess the idea of entries should be explained from the start. What kind of 
entries are accomulated there... I see they are added there all around the 
code, but what is the purpose of that is not quite clear when you read it.

At the first part of `transform_or_to_any` function, you costanly repeat two 
lines, like a mantra:

> +			entries = lappend(entries, rinfo);
> +			continue;

"If something is wrong -- do that mantra"

From my perspective, if you have to repeat same code again and again, then 
most probably you have some issues with architecture of the code. If you 
repeat some code again and again, you need to try to rewrite the code, the 
way, that part is repeated only once.

In that case I would try to move the most of the first loop of 
`transform_or_to_any`  to a separate function (let's say its name is 
prepare_single_or), that will do all the checks, if this or is good for us; 
return NULL if it does not suits our purposes (and in this case we do "entries 
= lappend(entries, rinfo); continue" in the main code, but only once) or 
return pointer to some useful data if this or clause is good for our purposes. 

This, I guess will make that part more clear and easy to read, without 
repeating same "lappend mantra" again and again.

Will continue digging into the code tomorrow.

P.S. Sorry for sending partly finished email. Pressed Ctrl+Enter 
accidentally... With no way to make it back :-(((

---

From: Nikolay Shaplov <[email-masked]>
Date: Wed, 26 Jun 2024 20:41:13 +0300

В письме от понедельник, 24 июня 2024 г. 23:51:56 MSK пользователь Nikolay 
Shaplov написал:

So,  I continue reading the patch.

I see there is `entries` variable in the code, that is the list of 
`RestrictInfo` objects and `entry` that is `OrClauseGroup` object.

This naming is quite misguiding (at least for me).

 `entries` variable name can be used, as we deal only with RestrictInfo 
entries here. It is kind of "generic" type. Though naming it 
`restric_info_entry` might make te code more readable.

But when we come to an `entry` variable, it is very specific entry, it should 
be `OrClauseGroup` entry, not just any entry. So I would suggest to name this 
variable `or_clause_group_entry`, or even `or_clause_group` , so when we meet 
this variable in the middle of the code, we can get the idea what we are 
dealing with, without scrolling code up.

Variable naming is very important thing. It can drastically improve (or ruin) 
code readability.

========

Also I see some changes in the tests int this patch. There are should be tests 
that check that this new feature works well. And there are test whose behavior 
have been just accidentally affected.

I whould suggest to split these tests into two patches, as they should be 
reviewed in different ways. Functionality tests should be thoroughly checked 
that all stuff we added is properly tested, and affected tests should be checked 
that nothing important is not broken. It would be more easy to check if these 
are two different patches.

I would also suggest to add to the commit message of affected tests changes 
some explanation why this changes does not really breaks anything. This will 
do the checking more simple.

To be continued.

-- 
Nikolay Shaplov aka Nataraj
Fuzzing Engineer at Postgres Professional
Matrix IM: @dhyan:nataraj.su

---

From: Alena Rybakina <[email-masked]>
Date: Tue, 9 Jul 2024 04:57:15 +0300

Hi! Thank you for your review! Sorryforthe delayin responding.

Irewrotethe patchasyourequested,butnowI'm facedwiththe problemof 
processingthe elementsof the or_entries list.For somereason, 
thepointerto thelistis cleared and I couldn't find the place where it 
happened.MaybeI'mmissingsomethingsimpleinviewof the heavyworkloadright 
now,butmaybeyou'll seea problem?Ihave displayedpart of stackbelow.

#5 0x00005b0f6d9f6a6a in ExceptionalCondition 
(conditionName=0x5b0f6dbb74f7 "IsPointerList(list)", 
fileName=0x5b0f6dbb7418 "list.c", lineNumber=341) at assert.c:66 #6 
0x00005b0f6d5dc3ba in lappend (list=0x5b0f6eec5ca0, 
datum=0x5b0f6eec0d90) at list.c:341 #7 0x00005b0f6d69230c in 
transform_or_to_any (root=0x5b0f6eeb13c8, orlist=0x5b0f6eec57c0) at 
initsplan.c:2818 #8 0x00005b0f6d692958 in add_base_clause_to_rel 
(root=0x5b0f6eeb13c8, relid=1, restrictinfo=0x5b0f6eec5990) at 
initsplan.c:2982 #9 0x00005b0f6d692e5f in 
distribute_restrictinfo_to_rels (root=0x5b0f6eeb13c8, 
restrictinfo=0x5b0f6eec5990) at initsplan.c:3175 #10 0x00005b0f6d691bf2 
in distribute_qual_to_rels (root=0x5b0f6eeb13c8, clause=0x5b0f6eec0fc0, 
jtitem=0x5b0f6eec4330, sjinfo=0x0, security_level=0, 
qualscope=0x5b0f6eec4730, ojscope=0x0, outerjoin_nonnullable=0x0, 
incompatible_relids=0x0, allow_equivalence=true, has_clone=false, 
is_clone=false, postponed_oj_qual_list=0x0) at initsplan.c:2576 #11 
0x00005b0f6d69146f in distribute_quals_to_rels (root=0x5b0f6eeb13c8, 
clauses=0x5b0f6eec0bb0, jtitem=0x5b0f6eec4330, sjinfo=0x0, 
security_level=0, qualscope=0x5b0f6eec4730, ojscope=0x0, 
outerjoin_nonnullable=0x0, incompatible_relids=0x0, 
allow_equivalence=true, has_clone=false, is_clone=false, 
postponed_oj_qual_list=0x0) at initsplan.c:2144

Thisis stillthe firstiterationof the fixesyouhave proposed,soI have 
attachedthe patchindiffformat.I rewroteit,asyousuggestedinthe 
firstletter[0].Icreateda separatefunctionthattriesto forman 
OrClauseGroup node,butifit failsinthis, it returnsfalse,otherwiseit 
processesthe generatedelementaccordingtowhat it found-eitheraddsit to 
thelistasnew,oraddsa constantto anexistingone.

Ialsodividedonegenerallistof 
suitableforconversionandunsuitableintotwodifferentones:appropriate_entriesandor_entries.Nowweare 
onlylookinginthe listof suitableelementstoformANYexpr.

Thishelpsusto get ridofrepetitionsinthe codeyoumentioned. 
Pleasewriteifthisis notthelogicthatyouhave seenbefore.

[0] 
https://www.postgresql.org/message-id/3381819.e9J7NaK4W3%40thinkpad-pgpro

-- 
Regards,
Alena Rybakina
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index e2c68fe6f99..1fdada54e23 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -14,9 +14,13 @@
  */
 #include "postgres.h"
 
+#include "catalog/namespace.h"
+#include "catalog/pg_operator.h"
 #include "catalog/pg_type.h"
+#include "common/hashfn.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
+#include "nodes/queryjumble.h"
 #include "optimizer/clauses.h"
 #include "optimizer/cost.h"
 #include "optimizer/inherit.h"
@@ -29,8 +33,11 @@
 #include "optimizer/planner.h"
 #include "optimizer/restrictinfo.h"
 #include "parser/analyze.h"
+#include "parser/parse_coerce.h"
+#include "parser/parse_oper.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/lsyscache.h"
+#include "utils/syscache.h"
 #include "utils/rel.h"
 #include "utils/typcache.h"
 
@@ -38,7 +45,6 @@
 int			from_collapse_limit;
 int			join_collapse_limit;
 
-
 /*
  * deconstruct_jointree requires multiple passes over the join tree, because we
  * need to finish computing JoinDomains before we start distributing quals.
@@ -2617,6 +2623,287 @@ check_redundant_nullability_qual(PlannerInfo *root, Node *clause)
 	return false;
 }
 
+static bool
+try_get_appropriable_group(RestrictInfo *rinfo, List **appropriate_group)
+{
+	OpExpr	   *orqual;
+	Node	   *const_expr;
+	Node	   *nconst_expr;
+	Oid			opno;
+	Oid			consttype;
+	Node	   *leftop,
+				*rightop;
+	ListCell   *lc2;
+	bool found = false;
+	OrClauseGroup *entry;
+
+	if (!IsA(rinfo, RestrictInfo) || !IsA(rinfo->clause, OpExpr))
+	{
+		return false;
+	}
+
+	orqual = (OpExpr *) rinfo->clause;
+	opno = orqual->opno;
+	if (get_op_rettype(opno) != BOOLOID)
+	{
+		/* Only operator returning boolean suits OR -> ANY transformation */
+		return false;
+	}
+
+	/*
+		* 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.  Get pointers to constant and expression sides of the qual.
+		*/
+	leftop = get_leftop(orqual);
+	if (IsA(leftop, RelabelType))
+		leftop = (Node *) ((RelabelType *) leftop)->arg;
+
+	rightop = get_rightop(orqual);
+	if (IsA(rightop, RelabelType))
+		rightop = (Node *) ((RelabelType *) rightop)->arg;
+
+	if (IsA(leftop, Const) || IsA(leftop, Param))
+	{
+		opno = get_commutator(opno);
+
+		if (!OidIsValid(opno))
+		{/* commutator doesn't exist, we can't reverse the order */
+			return false;
+		}
+
+		nconst_expr = get_rightop(orqual);
+		const_expr = get_leftop(orqual);
+	}
+	else if (IsA(rightop, Const) || IsA(rightop, Param))
+	{
+		const_expr = get_rightop(orqual);
+		nconst_expr = get_leftop(orqual);
+	}
+	else
+	{
+		return false;
+	}
+
+	/*
+		* Forbid transformation for composite types, records, and volatile
+		* expressions.
+		*/
+	consttype = exprType(const_expr);
+	if (type_is_rowtype(exprType(const_expr)) ||
+		type_is_rowtype(consttype) ||
+		contain_volatile_functions((Node *) nconst_expr))
+	{
+		return false;
+	}
+
+	entry = makeNode(OrClauseGroup);
+
+	foreach(lc2, *appropriate_group)
+	{
+
+		if (!IsA(lfirst(lc2), OrClauseGroup))
+			Assert(0);
+
+		entry = (OrClauseGroup *) lfirst(lc2);
+
+		if (entry->opno == opno &&
+			entry->consttype == consttype &&
+			entry->inputcollid == exprCollation(const_expr) &&
+			equal(entry->expr, nconst_expr))
+		{
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+	{
+		entry->expr = (Expr *) nconst_expr;
+		entry->exprs = list_make1((void *) orqual);
+		entry->opno = opno;
+		entry->inputcollid = exprCollation(const_expr);
+		entry->consttype = consttype;
+		entry->consts = list_make1(const_expr);
+		*appropriate_group = lappend(*appropriate_group, entry);
+	}
+	else
+	{
+		entry->consts = lappend(entry->consts, const_expr);
+		entry->exprs = lappend(entry->exprs, (void *) orqual);
+	}
+
+	return true;
+}
+
+/*
+ * transform_or_to_any -
+ *	  Discover the args of an OR expression and try to group similar OR
+ *	  expressions to SAOP expressions.
+ *
+ * This transformation groups two-sided equality expression.  One side of
+ * such an expression must be a plain constant or constant expression.  The
+ * other side must be a variable expression without volatile functions.
+ * To group quals, opno, inputcollid of variable expression, and type of
+ * constant expression must be equal too.
+ *
+ * The grouping technique is based on the equivalence of variable sides of
+ * the expression: using exprId and equal() routine, it groups constant sides
+ * of similar clauses into an array.  After the grouping procedure, each
+ * couple ('variable expression' and 'constant array') forms a new SAOP
+ * operation, which is added to the args list of the returning expression.
+ */
+static List *
+transform_or_to_any(PlannerInfo *root, List *orlist)
+{
+	List	   *appropriate_entries = NIL;
+	List	   *or_entries = NIL;
+	ListCell   *lc;
+	int			len_ors = list_length(orlist);
+	OrClauseGroup *entry = NULL;
+	bool found;
+
+	Assert(len_ors >= 2);
+
+	foreach(lc, orlist)
+	{
+		RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
+
+		/*
+		 * Add the entry to the list.  It is needed exclusively to manage
+		 * the problem with the order of transformed clauses in explain.
+		 * Hash value can depend on the platform and version.  Hence,
+		 * sequental scan of the hash table would prone to change the
+		 * order of clauses in lists and, as a result, break regression
+		 * tests accidentially.
+		 */
+		found = try_get_appropriable_group(rinfo, &appropriate_entries);
+
+		if (!found)
+			or_entries = lappend(or_entries, (void *) rinfo->clause);
+	}
+
+	/* Let's convert each group of clauses to an ANY expression. */
+
+	/*
+	 * 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, appropriate_entries)
+	{
+		Oid			scalar_type;
+		Oid			array_type;
+
+		if (!IsA(lfirst(lc), OrClauseGroup))
+		{
+			Assert(0);
+		}
+
+		entry = (OrClauseGroup *) lfirst(lc);
+
+		Assert(list_length(entry->consts) > 0);
+		Assert(list_length(entry->exprs) == list_length(entry->consts));
+
+		if (list_length(entry->consts) == 1)
+		{
+			/*
+			 * Only one element returns origin expression into the BoolExpr
+			 * args list unchanged.
+			 */
+			list_free(entry->consts);
+			or_entries = lappend(or_entries, linitial(entry->exprs));
+			continue;
+		}
+
+		/*
+		 * Do the transformation.
+		 */
+		scalar_type = entry->consttype;
+		array_type = OidIsValid(scalar_type) ? get_array_type(scalar_type) :
+			InvalidOid;
+
+		if (OidIsValid(array_type))
+		{
+			/*
+			 * OK: coerce all the right-hand non-Var inputs to the common type
+			 * and build an ArrayExpr for them.
+			 */
+			List	   *aexprs = NIL;
+			ArrayExpr  *newa = NULL;
+			ScalarArrayOpExpr *saopexpr = NULL;
+			HeapTuple	opertup;
+			Form_pg_operator operform;
+			List	   *namelist = NIL;
+			ListCell   *lc2;
+
+			foreach(lc2, entry->consts)
+			{
+				Node	   *node = (Node *) lfirst(lc2);
+
+				node = coerce_to_common_type(NULL, node, scalar_type,
+											 "OR ANY Transformation");
+				aexprs = lappend(aexprs, node);
+			}
+
+			newa = makeNode(ArrayExpr);
+			/* array_collid will be set by parse_collate.c */
+			newa->element_typeid = scalar_type;
+			newa->array_typeid = array_type;
+			newa->multidims = false;
+			newa->elements = aexprs;
+			newa->location = -1;
+
+			/*
+			 * Try to cast this expression to Const. Due to current strict
+			 * transformation rules it should be done [almost] every time.
+			 */
+			newa = (ArrayExpr *) eval_const_expressions(NULL, (Node *) newa);
+
+			opertup = SearchSysCache1(OPEROID,
+									  ObjectIdGetDatum(entry->opno));
+			if (!HeapTupleIsValid(opertup))
+				elog(ERROR, "cache lookup failed for operator %u",
+					 entry->opno);
+
+			operform = (Form_pg_operator) GETSTRUCT(opertup);
+			if (!OperatorIsVisible(entry->opno))
+				namelist = lappend(namelist, makeString(get_namespace_name(operform->oprnamespace)));
+
+			namelist = lappend(namelist, makeString(pstrdup(NameStr(operform->oprname))));
+			ReleaseSysCache(opertup);
+
+			saopexpr =
+				(ScalarArrayOpExpr *)
+				make_scalar_array_op(NULL,
+									 namelist,
+									 true,
+									 (Node *) entry->expr,
+									 (Node *) newa,
+									 -1);
+			saopexpr->inputcollid = entry->inputcollid;
+
+			or_entries = lappend(or_entries, (void *) saopexpr);
+		}
+		else
+		{
+			/*
+			 * If the const node's (right side of operator expression) type
+			 * don't have “true” array type, then we cannnot do the
+			 * transformation. We simply concatenate the expression node.
+			 */
+			list_free(entry->consts);
+			or_entries = list_concat(or_entries, entry->exprs);
+		}
+	}
+	list_free(appropriate_entries);
+
+	/* One more trick: assemble correct clause */
+	return or_entries;
+}
+
 /*
  * add_base_clause_to_rel
  *		Add 'restrictinfo' as a baserestrictinfo to the base relation denoted
@@ -2677,6 +2964,36 @@ add_base_clause_to_rel(PlannerInfo *root, Index relid,
 		}
 	}
 
+	if (restriction_is_or_clause(restrictinfo))
+	{
+		BoolExpr   *orExpr = (BoolExpr *) restrictinfo->orclause;
+
+		Assert(is_orclause(restrictinfo->orclause));
+
+		if (list_length(orExpr->args) >= 2)
+		{
+			List *or_list = transform_or_to_any(root, orExpr->args);
+			Expr *clause = list_length(or_list) > 1 ?
+								makeBoolExpr(OR_EXPR, or_list,
+											 ((BoolExpr *) restrictinfo->clause)->location) :
+								linitial(or_list);
+
+			RestrictInfo *rinfo;
+			rinfo = make_restrictinfo(root,
+										clause,
+										restrictinfo->is_pushed_down,
+										restrictinfo->has_clone,
+										restrictinfo->is_clone,
+										restrictinfo->pseudoconstant,
+										restrictinfo->security_level,
+										restrictinfo->required_relids,
+										restrictinfo->incompatible_relids,
+										restrictinfo->outer_relids);
+
+			restrictinfo = rinfo;
+		}
+	}
+
 	/* Add clause to rel's restriction list */
 	rel->baserestrictinfo = lappend(rel->baserestrictinfo, restrictinfo);
 
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 14ccfc1ac1c..f48ddb8c385 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2708,6 +2708,37 @@ typedef struct RestrictInfo
 	Oid			right_hasheqoperator pg_node_attr(equal_ignore);
 } RestrictInfo;
 
+/*
+ * The group of similar operator expressions in transform_or_to_any().
+ */
+typedef struct OrClauseGroup
+{
+	pg_node_attr(nodetag_only)
+
+	NodeTag		type;
+
+	/* The expression of the variable side of operator */
+	Expr	   *expr;
+	/* The operator of the operator expression */
+	Oid			opno;
+	/* The collation of the operator expression */
+	Oid			inputcollid;
+	/* The type of constant side of operator */
+	Oid			consttype;
+
+	/* The list of constant sides of operators */
+	List	   *consts;
+
+	/*
+	 * List of source expressions.  We need this for convenience in case we
+	 * will give up on transformation.
+	 */
+	List	   *exprs;
+
+	Node	   *const_expr;
+} OrClauseGroup;
+
+
 /*
  * This macro embodies the correct way to test whether a RestrictInfo is
  * "pushed down" to a given outer join, that is, should be treated as a filter
diff --git a/src/include/nodes/queryjumble.h b/src/include/nodes/queryjumble.h
index f1c55c8067f..5643ee8f651 100644
--- a/src/include/nodes/queryjumble.h
+++ b/src/include/nodes/queryjumble.h
@@ -65,6 +65,7 @@ extern PGDLLIMPORT int compute_query_id;
 
 extern const char *CleanQuerytext(const char *query, int *location, int *len);
 extern JumbleState *JumbleQuery(Query *query);
+extern JumbleState *JumbleExpr(Expr *expr, uint64 *exprId);
 extern void EnableQueryId(void);
 
 extern PGDLLIMPORT bool query_id_enabled;
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index cf6eac57349..421a645fae0 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1844,18 +1844,73 @@ DROP TABLE onek_with_null;
 EXPLAIN (COSTS OFF)
 SELECT * FROM tenk1
   WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
-                                                               QUERY PLAN                                                                
------------------------------------------------------------------------------------------------------------------------------------------
+                                     QUERY PLAN                                     
+------------------------------------------------------------------------------------
  Bitmap Heap Scan on tenk1
-   Recheck Cond: (((thousand = 42) AND (tenthous = 1)) OR ((thousand = 42) AND (tenthous = 3)) OR ((thousand = 42) AND (tenthous = 42)))
-   ->  BitmapOr
-         ->  Bitmap Index Scan on tenk1_thous_tenthous
-               Index Cond: ((thousand = 42) AND (tenthous = 1))
-         ->  Bitmap Index Scan on tenk1_thous_tenthous
-               Index Cond: ((thousand = 42) AND (tenthous = 3))
-         ->  Bitmap Index Scan on tenk1_thous_tenthous
-               Index Cond: ((thousand = 42) AND (tenthous = 42))
-(9 rows)
+   Recheck Cond: ((thousand = 42) AND (tenthous = ANY ('{1,3,42}'::integer[])))
+   ->  Bitmap Index Scan on tenk1_thous_tenthous
+         Index Cond: ((thousand = 42) AND (tenthous = ANY ('{1,3,42}'::integer[])))
+(4 rows)
+
+SELECT * FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
+ unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4 
+---------+---------+-----+------+-----+--------+---------+----------+-------------+-----------+----------+-----+------+----------+----------+---------
+      42 |    5530 |   0 |    2 |   2 |      2 |      42 |       42 |          42 |        42 |       42 |  84 |   85 | QBAAAA   | SEIAAA   | OOOOxx
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = (SELECT 1 + 2) OR tenthous = 42);
+                                          QUERY PLAN                                          
+----------------------------------------------------------------------------------------------
+ Bitmap Heap Scan on tenk1
+   Recheck Cond: ((thousand = 42) AND (tenthous = ANY (ARRAY[1, (InitPlan 1).col1, 42])))
+   InitPlan 1
+     ->  Result
+   ->  Bitmap Index Scan on tenk1_thous_tenthous
+         Index Cond: ((thousand = 42) AND (tenthous = ANY (ARRAY[1, (InitPlan 1).col1, 42])))
+(6 rows)
+
+SELECT * FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = (SELECT 1 + 2) OR tenthous = 42);
+ unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4 
+---------+---------+-----+------+-----+--------+---------+----------+-------------+-----------+----------+-----+------+----------+----------+---------
+      42 |    5530 |   0 |    2 |   2 |      2 |      42 |       42 |          42 |        42 |       42 |  84 |   85 | QBAAAA   | SEIAAA   | OOOOxx
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
+                                     QUERY PLAN                                     
+------------------------------------------------------------------------------------
+ Aggregate
+   ->  Bitmap Heap Scan on tenk1
+         Recheck Cond: ((hundred = 42) AND (thousand = ANY ('{42,99}'::integer[])))
+         ->  BitmapAnd
+               ->  Bitmap Index Scan on tenk1_hundred
+                     Index Cond: (hundred = 42)
+               ->  Bitmap Index Scan on tenk1_thous_tenthous
+                     Index Cond: (thousand = ANY ('{42,99}'::integer[]))
+(8 rows)
+
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
+ count 
+-------
+    10
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
+                                     QUERY PLAN                                     
+------------------------------------------------------------------------------------
+ Bitmap Heap Scan on tenk1
+   Recheck Cond: ((thousand = 42) AND (tenthous = ANY ('{1,3,42}'::integer[])))
+   ->  Bitmap Index Scan on tenk1_thous_tenthous
+         Index Cond: ((thousand = 42) AND (tenthous = ANY ('{1,3,42}'::integer[])))
+(4 rows)
 
 SELECT * FROM tenk1
   WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
@@ -1867,23 +1922,123 @@ SELECT * FROM tenk1
 EXPLAIN (COSTS OFF)
 SELECT count(*) FROM tenk1
   WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
-                                   QUERY PLAN                                    
----------------------------------------------------------------------------------
+                                     QUERY PLAN                                     
+------------------------------------------------------------------------------------
  Aggregate
    ->  Bitmap Heap Scan on tenk1
-         Recheck Cond: ((hundred = 42) AND ((thousand = 42) OR (thousand = 99)))
+         Recheck Cond: ((hundred = 42) AND (thousand = ANY ('{42,99}'::integer[])))
          ->  BitmapAnd
                ->  Bitmap Index Scan on tenk1_hundred
                      Index Cond: (hundred = 42)
+               ->  Bitmap Index Scan on tenk1_thous_tenthous
+                     Index Cond: (thousand = ANY ('{42,99}'::integer[]))
+(8 rows)
+
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
+ count 
+-------
+    10
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand < 42 OR thousand < 99 OR 43 > thousand OR 42 > thousand);
+                                        QUERY PLAN                                        
+------------------------------------------------------------------------------------------
+ Aggregate
+   ->  Bitmap Heap Scan on tenk1
+         Recheck Cond: ((hundred = 42) AND (thousand < ANY ('{42,99,43,42}'::integer[])))
+         ->  BitmapAnd
+               ->  Bitmap Index Scan on tenk1_hundred
+                     Index Cond: (hundred = 42)
+               ->  Bitmap Index Scan on tenk1_thous_tenthous
+                     Index Cond: (thousand < ANY ('{42,99,43,42}'::integer[]))
+(8 rows)
+
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand < 42 OR thousand < 99 OR 43 > thousand OR 42 > thousand);
+ count 
+-------
+    10
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3) OR thousand = 41;
+                                                        QUERY PLAN                                                         
+---------------------------------------------------------------------------------------------------------------------------
+ Aggregate
+   ->  Bitmap Heap Scan on tenk1
+         Recheck Cond: ((((thousand = 42) AND (tenthous = 1)) OR ((thousand = 42) AND (tenthous = 3))) OR (thousand = 41))
+         ->  BitmapOr
                ->  BitmapOr
                      ->  Bitmap Index Scan on tenk1_thous_tenthous
-                           Index Cond: (thousand = 42)
+                           Index Cond: ((thousand = 42) AND (tenthous = 1))
                      ->  Bitmap Index Scan on tenk1_thous_tenthous
-                           Index Cond: (thousand = 99)
+                           Index Cond: ((thousand = 42) AND (tenthous = 3))
+               ->  Bitmap Index Scan on tenk1_thous_tenthous
+                     Index Cond: (thousand = 41)
 (11 rows)
 
 SELECT count(*) FROM tenk1
-  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3) OR thousand = 41;
+ count 
+-------
+    10
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99 OR tenthous < 2) OR thousand = 41;
+                                                       QUERY PLAN                                                       
+------------------------------------------------------------------------------------------------------------------------
+ Aggregate
+   ->  Bitmap Heap Scan on tenk1
+         Recheck Cond: (((hundred = 42) AND ((thousand = 42) OR (thousand = 99) OR (tenthous < 2))) OR (thousand = 41))
+         ->  BitmapOr
+               ->  BitmapAnd
+                     ->  Bitmap Index Scan on tenk1_hundred
+                           Index Cond: (hundred = 42)
+                     ->  BitmapOr
+                           ->  Bitmap Index Scan on tenk1_thous_tenthous
+                                 Index Cond: (thousand = 42)
+                           ->  Bitmap Index Scan on tenk1_thous_tenthous
+                                 Index Cond: (thousand = 99)
+                           ->  Bitmap Index Scan on tenk1_thous_tenthous
+                                 Index Cond: (tenthous < 2)
+               ->  Bitmap Index Scan on tenk1_thous_tenthous
+                     Index Cond: (thousand = 41)
+(16 rows)
+
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99 OR tenthous < 2) OR thousand = 41;
+ count 
+-------
+    20
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 41 OR thousand = 99 AND tenthous = 2);
+                                                          QUERY PLAN                                                          
+------------------------------------------------------------------------------------------------------------------------------
+ Aggregate
+   ->  Bitmap Heap Scan on tenk1
+         Recheck Cond: ((hundred = 42) AND ((thousand = ANY ('{42,41}'::integer[])) OR ((thousand = 99) AND (tenthous = 2))))
+         Filter: ((thousand = 42) OR (thousand = 41) OR ((thousand = 99) AND (tenthous = 2)))
+         ->  BitmapAnd
+               ->  Bitmap Index Scan on tenk1_hundred
+                     Index Cond: (hundred = 42)
+               ->  BitmapOr
+                     ->  Bitmap Index Scan on tenk1_thous_tenthous
+                           Index Cond: (thousand = ANY ('{42,41}'::integer[]))
+                     ->  Bitmap Index Scan on tenk1_thous_tenthous
+                           Index Cond: ((thousand = 99) AND (tenthous = 2))
+(12 rows)
+
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 41 OR thousand = 99 AND tenthous = 2);
  count 
 -------
     10
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 9142dab171f..59d2617295f 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4234,6 +4234,58 @@ select * from tenk1 a join tenk1 b on
                            Index Cond: (unique2 = 7)
 (19 rows)
 
+explain (costs off)
+select * from tenk1 a join tenk1 b on
+  (a.unique1 = 1 and b.unique1 = 2) or
+  ((a.unique2 = 3 or a.unique2 = 7) and b.hundred = 4);
+                                                      QUERY PLAN                                                      
+----------------------------------------------------------------------------------------------------------------------
+ Nested Loop
+   Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4)))
+   ->  Bitmap Heap Scan on tenk1 b
+         Recheck Cond: ((unique1 = 2) OR (hundred = 4))
+         ->  BitmapOr
+               ->  Bitmap Index Scan on tenk1_unique1
+                     Index Cond: (unique1 = 2)
+               ->  Bitmap Index Scan on tenk1_hundred
+                     Index Cond: (hundred = 4)
+   ->  Materialize
+         ->  Bitmap Heap Scan on tenk1 a
+               Recheck Cond: ((unique1 = 1) OR (unique2 = 3) OR (unique2 = 7))
+               ->  BitmapOr
+                     ->  Bitmap Index Scan on tenk1_unique1
+                           Index Cond: (unique1 = 1)
+                     ->  Bitmap Index Scan on tenk1_unique2
+                           Index Cond: (unique2 = 3)
+                     ->  Bitmap Index Scan on tenk1_unique2
+                           Index Cond: (unique2 = 7)
+(19 rows)
+
+explain (costs off)
+select * from tenk1 a join tenk1 b on
+  (a.unique1 < 20 or a.unique1 = 3 or a.unique1 = 1 and b.unique1 = 2) or
+  ((a.unique2 = 3 or a.unique2 = 7) and b.hundred = 4);
+                                                                         QUERY PLAN                                                                          
+-------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Nested Loop
+   Join Filter: ((a.unique1 < 20) OR (a.unique1 = 3) OR ((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4)))
+   ->  Seq Scan on tenk1 b
+   ->  Materialize
+         ->  Bitmap Heap Scan on tenk1 a
+               Recheck Cond: ((unique1 < 20) OR (unique1 = 3) OR (unique1 = 1) OR (unique2 = 3) OR (unique2 = 7))
+               ->  BitmapOr
+                     ->  Bitmap Index Scan on tenk1_unique1
+                           Index Cond: (unique1 < 20)
+                     ->  Bitmap Index Scan on tenk1_unique1
+                           Index Cond: (unique1 = 3)
+                     ->  Bitmap Index Scan on tenk1_unique1
+                           Index Cond: (unique1 = 1)
+                     ->  Bitmap Index Scan on tenk1_unique2
+                           Index Cond: (unique2 = 3)
+                     ->  Bitmap Index Scan on tenk1_unique2
+                           Index Cond: (unique2 = 7)
+(17 rows)
+
 --
 -- test placement of movable quals in a parameterized join tree
 --
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 8c4da955084..7678744181c 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -1322,19 +1322,19 @@ SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE
 SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE (a = 1 OR a = 51) AND b = ''1''');
  estimated | actual 
 -----------+--------
-        99 |    100
+       100 |    100
 (1 row)
 
 SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE (a = 1 OR a = 51) AND (b = ''1'' OR b = ''2'')');
  estimated | actual 
 -----------+--------
-        99 |    100
+       100 |    100
 (1 row)
 
 SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE (a = 1 OR a = 2 OR a = 51 OR a = 52) AND (b = ''1'' OR b = ''2'')');
  estimated | actual 
 -----------+--------
-       197 |    200
+       200 |    200
 (1 row)
 
 -- OR clauses referencing different attributes are incompatible
@@ -1664,19 +1664,19 @@ SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE
 SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE ((a * 2) = 2 OR (a * 2) = 102) AND upper(b) = ''1''');
  estimated | actual 
 -----------+--------
-        99 |    100
+       100 |    100
 (1 row)
 
 SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE ((a * 2) = 2 OR (a * 2) = 102) AND (upper(b) = ''1'' OR upper(b) = ''2'')');
  estimated | actual 
 -----------+--------
-        99 |    100
+       100 |    100
 (1 row)
 
 SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE ((a * 2) = 2 OR (a * 2) = 4 OR (a * 2) = 102 OR (a * 2) = 104) AND (upper(b) = ''1'' OR upper(b) = ''2'')');
  estimated | actual 
 -----------+--------
-       197 |    200
+       200 |    200
 (1 row)
 
 -- OR clauses referencing different attributes
diff --git a/src/test/regress/expected/tidscan.out b/src/test/regress/expected/tidscan.out
index f133b5a4ac7..0f6d3d323d4 100644
--- a/src/test/regress/expected/tidscan.out
+++ b/src/test/regress/expected/tidscan.out
@@ -43,11 +43,12 @@ SELECT ctid, * FROM tidscan WHERE '(0,1)' = ctid;
 -- OR'd clauses
 EXPLAIN (COSTS OFF)
 SELECT ctid, * FROM tidscan WHERE ctid = '(0,2)' OR '(0,1)' = ctid;
-                          QUERY PLAN                          
---------------------------------------------------------------
+                         QUERY PLAN                         
+------------------------------------------------------------
  Tid Scan on tidscan
-   TID Cond: ((ctid = '(0,2)'::tid) OR ('(0,1)'::tid = ctid))
-(2 rows)
+   TID Cond: (ctid = ANY ('{"(0,2)","(0,1)"}'::tid[]))
+   Filter: ((ctid = '(0,2)'::tid) OR ('(0,1)'::tid = ctid))
+(3 rows)
 
 SELECT ctid, * FROM tidscan WHERE ctid = '(0,2)' OR '(0,1)' = ctid;
  ctid  | id 
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index e296891cab8..f74ad415fbf 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -726,6 +726,24 @@ DROP TABLE onek_with_null;
 -- Check bitmap index path planning
 --
 
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
+SELECT * FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = (SELECT 1 + 2) OR tenthous = 42);
+SELECT * FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = (SELECT 1 + 2) OR tenthous = 42);
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
+
 EXPLAIN (COSTS OFF)
 SELECT * FROM tenk1
   WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
@@ -738,6 +756,30 @@ SELECT count(*) FROM tenk1
 SELECT count(*) FROM tenk1
   WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
 
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand < 42 OR thousand < 99 OR 43 > thousand OR 42 > thousand);
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand < 42 OR thousand < 99 OR 43 > thousand OR 42 > thousand);
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3) OR thousand = 41;
+SELECT count(*) FROM tenk1
+  WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3) OR thousand = 41;
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99 OR tenthous < 2) OR thousand = 41;
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 99 OR tenthous < 2) OR thousand = 41;
+
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 41 OR thousand = 99 AND tenthous = 2);
+SELECT count(*) FROM tenk1
+  WHERE hundred = 42 AND (thousand = 42 OR thousand = 41 OR thousand = 99 AND tenthous = 2);
+
 --
 -- Check behavior with duplicate index column contents
 --
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index e3d26520832..cf02cc34ed0 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1409,6 +1409,15 @@ select * from tenk1 a join tenk1 b on
   (a.unique1 = 1 and b.unique1 = 2) or
   ((a.unique2 = 3 or a.unique2 = 7) and b.hundred = 4);
 
+explain (costs off)
+select * from tenk1 a join tenk1 b on
+  (a.unique1 = 1 and b.unique1 = 2) or
+  ((a.unique2 = 3 or a.unique2 = 7) and b.hundred = 4);
+explain (costs off)
+select * from tenk1 a join tenk1 b on
+  (a.unique1 < 20 or a.unique1 = 3 or a.unique1 = 1 and b.unique1 = 2) or
+  ((a.unique2 = 3 or a.unique2 = 7) and b.hundred = 4);
+
 --
 -- test placement of movable quals in a parameterized join tree
 --
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 442428d937c..3d2ce04f9e2 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -40,6 +40,7 @@ explain (costs off) select * from lp where a is not null;
 explain (costs off) select * from lp where a is null;
 explain (costs off) select * from lp where a = 'a' or a = 'c';
 explain (costs off) select * from lp where a is not null and (a = 'a' or a = 'c');
+
 explain (costs off) select * from lp where a <> 'g';
 explain (costs off) select * from lp where a <> 'a' and a <> 'd';
 explain (costs off) select * from lp where a not in ('a', 'd');
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 9320e4d8080..4ec9e2ce32f 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1715,6 +1715,7 @@ NumericVar
 OM_uint32
 OP
 OSAPerGroupState
+OrClauseGroup
 OSAPerQueryState
 OSInfo
 OSSLCipher

---

From: Alena Rybakina <[email-masked]>
Date: Thu, 11 Jul 2024 19:17:42 +0300

Hi, again!

I have finished patch and processed almost your suggestions (from [0], 
[1], [2]). It remainsonlyto addtestswherethe conversionshouldwork,butI 
willaddthis inthe nextversion.

[0] 

[1] 
https://www.postgresql.org/message-id/9736220.CDJkKcVGEf%40thinkpad-pgpro

[2] 
https://www.postgresql.org/message-id/2193851.QkHrqEjB74%40thinkpad-pgpro

On 09.07.2024 04:57, Alena Rybakina wrote:
> restrictinfo=0x5b0f6eec5990) at initsplan.c:3175 #10 
> 0x00005b0f6d691bf2 in distribute_qual_to_rels (root=0x5b0f6eeb13c8, 
> clause=0x5b0f6eec0fc0, jtitem=0x5b0f6eec4330, sjinfo=0x0, 
> postponed_oj_qual_list=0x0) at initsplan.c:2576 #11 0x00005b0f6d69146f 
> in distribute_quals_to_rels (root=0x5b0f6eeb13c8, 
The errorwascausedby the specificsof storingthe "OR"clausesinthe 
RestrictInfostructure.Scanning the orclauses list of the RestrictInfo 
variable, wecouldfacenotonlytheitem with RestrictInfo 
type,butalsotheBoolExpr type.

For example, when we have both or clauses and "AND" clauses together, 
like x = 1 and (y =1 or y=2 or y=3 and z = 1). The structure looks like:

RestrictInfo->orclauses = [RestrictInfo [x=1],
RestrictInfo->orclauses = [RestrictInfo[y=1],
                     RestrictInfo [y=2],
                     BoolExpr = [Restrictinfo [y=3], RestrictInfo [z=1]
                    ]
                                          ]

It'sworkingfinenow.

-- 
Regards,
Alena Rybakina
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
From 26eca98229749b20ad0c82a9fa55f4f37fd34d29 Mon Sep 17 00:00:00 2001
From: Alena Rybakina <[email-masked]>
Date: Thu, 11 Jul 2024 19:01:10 +0300
Subject: [PATCH 1/2] Transform OR clauses to ANY expression

Replace (expr op C1) OR (expr op C2) ... with expr op ANY(ARRAY[C1, C2, ...])
during adding restrictinfo's to the base relation.

Here Cn is a n-th constant or parameters expression, 'expr' is non-constant
expression, 'op' is an operator which returns boolean result and has a commuter
(for the case of reverse order of constant and non-constant parts of the
expression, like 'Cn op expr').

Discussion: https://postgr.es/m/567ED6CA.2040504%40sigaev.ru
Author: Alena Rybakina <[email-masked]>
Author: Andrey Lepikhov <[email-masked]>
Reviewed-by: Peter Geoghegan <[email-masked]>
Reviewed-by: Ranier Vilela <[email-masked]>
Reviewed-by: Alexander Korotkov <[email-masked]>
Reviewed-by: Robert Haas <[email-masked]>
Reviewed-by: Jian He <[email-masked]>
Reviewed-by: Tom Lane <[email-masked]>
Reviewed-by: Nikolay Shaplov <[email-masked]>
---
 src/backend/optimizer/plan/initsplan.c        | 335 ++++++++++++++++++
 src/include/nodes/pathnodes.h                 |  31 ++
 src/test/regress/expected/create_index.out    |  32 +-
 src/test/regress/expected/partition_prune.out |  46 +--
 src/test/regress/expected/stats_ext.out       |  12 +-
 src/test/regress/expected/tidscan.out         |   6 +-
 src/tools/pgindent/typedefs.list              |   1 +
 7 files changed, 410 insertions(+), 53 deletions(-)

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index e2c68fe6f99..572312be748 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -14,9 +14,13 @@
  */
 #include "postgres.h"

@@ -2617,6 +2624,308 @@ check_redundant_nullability_qual(PlannerInfo *root, Node *clause)
 	return false;
 }
 
+static bool
+try_prepare_single_or(RestrictInfo *rinfo, List **appropriate_group)
+{
+	OpExpr	   *orqual;
+	Node	   *const_expr;
+	Node	   *nconst_expr;
+	Oid			opno;
+	Oid			consttype;
+	Node	   *leftop,
+			   *rightop;
+	ListCell   *lc2;
+	bool found = false;
+	OrClauseGroup *or_clause_group;
+
+	if (!IsA(rinfo, RestrictInfo) || !IsA(rinfo->clause, OpExpr))
+	{
+		return false;
+	}
+
+	orqual = (OpExpr *) rinfo->clause;
+	opno = orqual->opno;
+	if (get_op_rettype(opno) != BOOLOID)
+	{
+		/* Only operator returning boolean suits OR -> ANY transformation */
+		return false;
+	}
+
+	/*
+		* 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.  Get pointers to constant and expression sides of the qual.
+		*/
+	leftop = get_leftop(orqual);
+	if (IsA(leftop, RelabelType))
+		leftop = (Node *) ((RelabelType *) leftop)->arg;
+
+	rightop = get_rightop(orqual);
+	if (IsA(rightop, RelabelType))
+		rightop = (Node *) ((RelabelType *) rightop)->arg;
+
+	if (IsA(leftop, Const) || IsA(leftop, Param))
+	{
+		opno = get_commutator(opno);
+
+		if (!OidIsValid(opno))
+		{/* commutator doesn't exist, we can't reverse the order */
+			return false;
+		}
+
+		nconst_expr = get_rightop(orqual);
+		const_expr = get_leftop(orqual);
+	}
+	else if (IsA(rightop, Const) || IsA(rightop, Param))
+	{
+		const_expr = get_rightop(orqual);
+		nconst_expr = get_leftop(orqual);
+	}
+	else
+	{
+		return false;
+	}
+
+	/*
+		* Forbid transformation for composite types, records, and volatile
+		* expressions.
+		*/
+	consttype = exprType(const_expr);
+	if (type_is_rowtype(exprType(const_expr)) ||
+		type_is_rowtype(consttype) ||
+		contain_volatile_functions((Node *) nconst_expr))
+	{
+		return false;
+	}
+
+	or_clause_group = makeNode(OrClauseGroup);
+
+	foreach(lc2, *appropriate_group)
+	{
+
+		if (!IsA(lfirst(lc2), OrClauseGroup))
+			Assert(0);
+
+		or_clause_group = (OrClauseGroup *) lfirst(lc2);
+
+		if (or_clause_group->opno == opno &&
+			or_clause_group->consttype == consttype &&
+			or_clause_group->inputcollid == exprCollation(const_expr) &&
+			equal(or_clause_group->expr, nconst_expr))
+		{
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+	{
+		or_clause_group->expr = (Expr *) nconst_expr;
+		or_clause_group->exprs = list_make1((void *) orqual);
+		or_clause_group->opno = opno;
+		or_clause_group->inputcollid = exprCollation(const_expr);
+		or_clause_group->consttype = consttype;
+		or_clause_group->consts = list_make1(const_expr);
+		Assert(list_length(or_clause_group->exprs) == list_length(or_clause_group->consts));
+		*appropriate_group = lappend(*appropriate_group, or_clause_group);
+	}
+	else
+	{
+		or_clause_group->consts = lappend(or_clause_group->consts, const_expr);
+		or_clause_group->exprs = lappend(or_clause_group->exprs, (void *) orqual);
+		Assert(list_length(or_clause_group->exprs) == list_length(or_clause_group->consts));
+	}
+
+	return true;
+}
+
+/*
+ * transform_or_to_any -
+ *	  Discover the args of an OR expression and try to group similar OR
+ *	  expressions to SAOP expressions.
+ *
+ * This transformation groups two-sided equality expression.  One side of
+ * such an expression must be a plain constant or constant expression.  The
+ * other side must be a variable expression without volatile functions.
+ * To group quals, opno, inputcollid of variable expression, and type of
+ * constant expression must be equal too.
+ *
+ * The grouping technique is based on the equivalence of variable sides of
+ * the expression: using exprId and equal() routine, it groups constant sides
+ * of similar clauses into an array.  After the grouping procedure, each
+ * couple ('variable expression' and 'constant array') forms a new SAOP
+ * operation, which is added to the args list of the returning expression.
+ */
+static List *
+transform_or_to_any(PlannerInfo *root, RestrictInfo * rinfo)
+{
+	List	   *appropriate_entries = NIL;
+	List	   *or_entries = NIL;
+	int			len_ors = ((BoolExpr *) rinfo->clause)->args ? list_length(((BoolExpr *) rinfo->clause)->args) : 0;
+	OrClauseGroup *restrict_info_entry = NULL;
+	ListCell	   *lc;
+	bool found;
+
+	if(len_ors < 2)
+		return NIL;
+
+	foreach(lc, ((BoolExpr *) rinfo->orclause)->args)
+	{
+		RestrictInfo *sub_rinfo;
+		Expr		 *or_qual = (Expr *) lfirst(lc);
+
+		if(!IsA(lfirst(lc), RestrictInfo))
+			or_entries = lappend(or_entries, (void *) or_qual);
+		else
+		{
+			sub_rinfo = (RestrictInfo *) lfirst(lc);
+
+			/*
+			* Add the restrict_info_entry to the list.  It is needed exclusively to manage
+			* the problem with the order of transformed clauses in explain.
+			* Hash value can depend on the platform and version.  Hence,
+			* sequental scan of the hash table would prone to change the
+			* order of clauses in lists and, as a result, break regression
+			* tests accidentially.
+			*/
+			found = try_prepare_single_or(sub_rinfo, &appropriate_entries);
+
+			if (!found)
+			{
+				or_entries = lappend(or_entries, (void *) sub_rinfo->clause);
+			}
+		}
+	}
+
+	if (list_length(or_entries) == len_ors || (appropriate_entries == NIL && list_length(appropriate_entries) < 1))
+		return NIL;
+
+	found = false;
+
+	/* Let's convert each group of clauses to an ANY expression. */
+
+	/*
+	 * 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, appropriate_entries)
+	{
+		Oid			scalar_type;
+		Oid			array_type;
+
+		if (!IsA(lfirst(lc), OrClauseGroup))
+		{
+			Assert(0);
+		}
+
+		restrict_info_entry = (OrClauseGroup *) lfirst(lc);
+
+		Assert(list_length(restrict_info_entry->exprs) == list_length(restrict_info_entry->consts));
+
+		if (list_length(restrict_info_entry->consts) == 1)
+		{
+			/*
+			 * Only one element returns origin expression into the BoolExpr
+			 * args list unchanged.
+			 */
+			or_entries = list_concat(or_entries, (void *) restrict_info_entry->exprs);
+			continue;
+		}
+
+		/*
+		 * Do the transformation.
+		 */
+		scalar_type = restrict_info_entry->consttype;
+		array_type = OidIsValid(scalar_type) ? get_array_type(scalar_type) :
+			InvalidOid;
+
+		if (OidIsValid(array_type))
+		{
+			/*
+			 * OK: coerce all the right-hand non-Var inputs to the common type
+			 * and build an ArrayExpr for them.
+			 */
+			List	   *aexprs = NIL;
+			ArrayExpr  *newa = NULL;
+			ScalarArrayOpExpr *saopexpr = NULL;
+			HeapTuple	opertup;
+			Form_pg_operator operform;
+			List	   *namelist = NIL;
+			ListCell   *lc2;
+
+			foreach(lc2, restrict_info_entry->consts)
+			{
+				Node	   *node = (Node *) lfirst(lc2);
+
+				node = coerce_to_common_type(NULL, node, scalar_type,
+											 "OR ANY Transformation");
+				aexprs = lappend(aexprs, node);
+			}
+
+			newa = makeNode(ArrayExpr);
+			/* array_collid will be set by parse_collate.c */
+			newa->element_typeid = scalar_type;
+			newa->array_typeid = array_type;
+			newa->multidims = false;
+			newa->elements = aexprs;
+			newa->location = -1;
+
+			/*
+			 * Try to cast this expression to Const. Due to current strict
+			 * transformation rules it should be done [almost] every time.
+			 */
+			newa = (ArrayExpr *) eval_const_expressions(NULL, (Node *) newa);
+
+			opertup = SearchSysCache1(OPEROID,
+									  ObjectIdGetDatum(restrict_info_entry->opno));
+			if (!HeapTupleIsValid(opertup))
+				elog(ERROR, "cache lookup failed for operator %u",
+					 restrict_info_entry->opno);
+
+			operform = (Form_pg_operator) GETSTRUCT(opertup);
+			if (!OperatorIsVisible(restrict_info_entry->opno))
+				namelist = lappend(namelist, makeString(get_namespace_name(operform->oprnamespace)));
+
+			namelist = lappend(namelist, makeString(pstrdup(NameStr(operform->oprname))));
+			ReleaseSysCache(opertup);
+
+			saopexpr =
+				(ScalarArrayOpExpr *)
+				make_scalar_array_op(NULL,
+									 namelist,
+									 true,
+									 (Node *) restrict_info_entry->expr,
+									 (Node *) newa,
+									 -1);
+			saopexpr->inputcollid = restrict_info_entry->inputcollid;
+
+			or_entries = lappend(or_entries, (Expr *) saopexpr);
+
+			found = true;
+		}
+		else
+		{
+			/*
+			 * If the const node's (right side of operator expression) type
+			 * don't have “true” array type, then we cannnot do the
+			 * transformation. We simply concatenate the expression node.
+			 */
+			list_free(restrict_info_entry->consts);
+			or_entries = list_concat(or_entries, (void *) restrict_info_entry->exprs);
+		}
+	}
+	list_free(appropriate_entries);
+
+	/* One more trick: assemble correct clause */
+	if (found)
+		return or_entries;
+	else
+		return NIL;
+}
+
 /*
  * add_base_clause_to_rel
  *		Add 'restrictinfo' as a baserestrictinfo to the base relation denoted
@@ -2677,6 +2986,32 @@ add_base_clause_to_rel(PlannerInfo *root, Index relid,
 		}
 	}
 
+		List *or_list = transform_or_to_any(root, restrictinfo);
+		if (or_list != NIL)

+/*
index cf6eac57349..bcae073143a 100644
@@ -1844,18 +1844,11 @@ DROP TABLE onek_with_null;
- Bitmap Heap Scan on tenk1
+------------------------------------------------------------------------------
+ Index Scan using tenk1_thous_tenthous on tenk1
+(2 rows)
 
 SELECT * FROM tenk1
@@ -1867,20 +1860,17 @@ SELECT * FROM tenk1
-(11 rows)
 
 SELECT count(*) FROM tenk1
   WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 7a03b4e3607..dc0876b4bb3 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -100,23 +100,23 @@ explain (costs off) select * from lp where a is null;
 (2 rows)
 
 explain (costs off) select * from lp where a = 'a' or a = 'c';
-                        QUERY PLAN                        
-----------------------------------------------------------
+                  QUERY PLAN                   
+-----------------------------------------------
  Append
    ->  Seq Scan on lp_ad lp_1
-         Filter: ((a = 'a'::bpchar) OR (a = 'c'::bpchar))
+         Filter: (a = ANY ('{a,c}'::bpchar[]))
    ->  Seq Scan on lp_bc lp_2
-         Filter: ((a = 'a'::bpchar) OR (a = 'c'::bpchar))
+         Filter: (a = ANY ('{a,c}'::bpchar[]))
 (5 rows)
 
 explain (costs off) select * from lp where a is not null and (a = 'a' or a = 'c');
-                                   QUERY PLAN                                   
---------------------------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Append
    ->  Seq Scan on lp_ad lp_1
-         Filter: ((a IS NOT NULL) AND ((a = 'a'::bpchar) OR (a = 'c'::bpchar)))
+         Filter: ((a IS NOT NULL) AND (a = ANY ('{a,c}'::bpchar[])))
    ->  Seq Scan on lp_bc lp_2
-         Filter: ((a IS NOT NULL) AND ((a = 'a'::bpchar) OR (a = 'c'::bpchar)))
+         Filter: ((a IS NOT NULL) AND (a = ANY ('{a,c}'::bpchar[])))
 (5 rows)
 
 explain (costs off) select * from lp where a <> 'g';
@@ -533,10 +533,10 @@ explain (costs off) select * from rlp where a <= 31;
 (27 rows)
 
 explain (costs off) select * from rlp where a = 1 or a = 7;
-           QUERY PLAN           
---------------------------------
+                QUERY PLAN                
+------------------------------------------
  Seq Scan on rlp2 rlp
-   Filter: ((a = 1) OR (a = 7))
+   Filter: (a = ANY ('{1,7}'::integer[]))
 (2 rows)
 
 explain (costs off) select * from rlp where a = 1 or b = 'ab';
@@ -614,13 +614,13 @@ explain (costs off) select * from rlp where a < 1 or (a > 20 and a < 25);
 
 -- where clause contradicts sub-partition's constraint
 explain (costs off) select * from rlp where a = 20 or a = 40;
-               QUERY PLAN               
-----------------------------------------
+                    QUERY PLAN                    
+--------------------------------------------------
  Append
    ->  Seq Scan on rlp4_1 rlp_1
-         Filter: ((a = 20) OR (a = 40))
+         Filter: (a = ANY ('{20,40}'::integer[]))
    ->  Seq Scan on rlp5_default rlp_2
-         Filter: ((a = 20) OR (a = 40))
+         Filter: (a = ANY ('{20,40}'::integer[]))
 (5 rows)
 
 explain (costs off) select * from rlp3 where a = 20;   /* empty */
@@ -2090,10 +2090,10 @@ explain (costs off) select * from hp where a = 1 and b = 'abcde';
 
 explain (costs off) select * from hp where a = 1 and b = 'abcde' and
   (c = 2 or c = 3);
-                              QUERY PLAN                              
-----------------------------------------------------------------------
+                                   QUERY PLAN                                   
+--------------------------------------------------------------------------------
  Seq Scan on hp2 hp
-   Filter: ((a = 1) AND (b = 'abcde'::text) AND ((c = 2) OR (c = 3)))
+   Filter: ((c = ANY ('{2,3}'::integer[])) AND (a = 1) AND (b = 'abcde'::text))
 (2 rows)
 
 drop table hp2;
@@ -2429,8 +2429,8 @@ select explain_parallel_append('execute ab_q5 (33, 44, 55)');
 
 -- Test Parallel Append with PARAM_EXEC Params
 select explain_parallel_append('select count(*) from ab where (a = (select 1) or a = (select 3)) and b = 2');
-                                    explain_parallel_append                                     
-------------------------------------------------------------------------------------------------
+                                      explain_parallel_append                                      
+---------------------------------------------------------------------------------------------------
  Aggregate (actual rows=N loops=N)
    InitPlan 1
      ->  Result (actual rows=N loops=N)
@@ -2441,11 +2441,11 @@ select explain_parallel_append('select count(*) from ab where (a = (select 1) or
          Workers Launched: N
          ->  Parallel Append (actual rows=N loops=N)
                ->  Parallel Seq Scan on ab_a1_b2 ab_1 (actual rows=N loops=N)
-                     Filter: ((b = 2) AND ((a = (InitPlan 1).col1) OR (a = (InitPlan 2).col1)))
+                     Filter: ((a = ANY (ARRAY[(InitPlan 1).col1, (InitPlan 2).col1])) AND (b = 2))
                ->  Parallel Seq Scan on ab_a2_b2 ab_2 (never executed)
-                     Filter: ((b = 2) AND ((a = (InitPlan 1).col1) OR (a = (InitPlan 2).col1)))
+                     Filter: ((a = ANY (ARRAY[(InitPlan 1).col1, (InitPlan 2).col1])) AND (b = 2))
                ->  Parallel Seq Scan on ab_a3_b2 ab_3 (actual rows=N loops=N)
-                     Filter: ((b = 2) AND ((a = (InitPlan 1).col1) OR (a = (InitPlan 2).col1)))
+                     Filter: ((a = ANY (ARRAY[(InitPlan 1).col1, (InitPlan 2).col1])) AND (b = 2))
 (15 rows)
 
 -- Test pruning during parallel nested loop query

 

 
 -- OR clauses referencing different attributes
diff --git a/src/test/regress/expected/tidscan.out b/src/test/regress/expected/tidscan.out
index f133b5a4ac7..09553f19524 100644
--- a/src/test/regress/expected/tidscan.out
+++ b/src/test/regress/expected/tidscan.out
@@ -43,10 +43,10 @@ SELECT ctid, * FROM tidscan WHERE '(0,1)' = ctid;
 -- OR'd clauses
 EXPLAIN (COSTS OFF)
 SELECT ctid, * FROM tidscan WHERE ctid = '(0,2)' OR '(0,1)' = ctid;
-                          QUERY PLAN                          
---------------------------------------------------------------
+                      QUERY PLAN                       
+-------------------------------------------------------
  Tid Scan on tidscan
-   TID Cond: ((ctid = '(0,2)'::tid) OR ('(0,1)'::tid = ctid))
+   TID Cond: (ctid = ANY ('{"(0,2)","(0,1)"}'::tid[]))
 (2 rows)
 
2.34.1

From 8afa37e0843b2ce8cc8acb23e435a6bb1d92cc44 Mon Sep 17 00:00:00 2001
From: Alena Rybakina <[email-masked]>
Date: Thu, 11 Jul 2024 19:08:17 +0300
Subject: [PATCH 2/2] Add some new tests to check the functionality of
 transformation of OR expressions to Any expression.

---
 src/test/regress/sql/create_index.sql | 42 +++++++++++++++++++++++++++
 src/test/regress/sql/join.sql         |  9 ++++++
 2 files changed, 51 insertions(+)

+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1
 
+EXPLAIN (COSTS OFF)
 
+explain (costs off)
2.34.1

---

From: Alena Rybakina <[email-masked]>
Date: Thu, 11 Jul 2024 19:29:02 +0300

Sorry for repeating, but I have noticed that this message displays 
incorrectly and just in case I'll duplicate it.

>                     BoolExpr = [Restrictinfo [y=3], RestrictInfo [z=1]
The error was caused by the specifics of storing the "OR" clauses in the 
RestrictInfo structure. When viewing the list of or offers, we could 
encounter not only the RestrictInfo type, but also the BoolExpr type. 
It's working fine now.

It's working fine now.

---

From: Alexander Korotkov <[email-masked]>
Date: Wed, 17 Jul 2024 03:03:03 +0300

Hi, Alena!

On Thu, Jul 11, 2024 at 7:17 PM Alena Rybakina
<[email-masked]> wrote:
> I have finished patch and processed almost your suggestions (from [0], [1], [2]). It remains only to add tests where the conversion should work, but I will add this in the next version.
>
> [0] https://www.postgresql.org/message-id/3381819.e9J7NaK4W3%40thinkpad-pgpro
>
> [1] https://www.postgresql.org/message-id/9736220.CDJkKcVGEf%40thinkpad-pgpro
>
> [2] https://www.postgresql.org/message-id/2193851.QkHrqEjB74%40thinkpad-pgpro

I dare making another revision of this patch.  In this version I moved
the transformation to match_clause_to_indexcol().  Therefore, this
allows to successfully construct index scans with SAOP, but has no
degradation in generation of bitmap scans which I observed in [1] and
[2].  BTW, I found that my description in [2] lacks of t_b_c_idx index
definition. Sorry for that.

Given that now we're doing OR-to-ANY transformation solely to match an
index we don't need complex analysis of OR-list, which potentially
could take quadratic time.  Instead, we're trying to match every OR
element to an index and quit immediately on failure.

I'd like to head a feedback on the new place to apply the
transformation.  It looks like significant simplification for me and
the way to go.

Also, I have addressed some of notes by Robert Haas [3].  In v27 we
don't use expression evaluation, but directly construct an array
constant when possible.  Also we don't transform operator id to string
and back, but directly construct SAOP instead.

Links.
1. https://www.postgresql.org/message-id/CAPpHfduJtO0s9E%3DSHUTzrCD88BH0eik0UNog1_q3XBF2wLmH6g%40mail.gmail.com
2. https://www.postgresql.org/message-id/CAPpHfdtSXxhdv3mLOLjEewGeXJ%2BFtfhjqodn1WWuq5JLsKx48g%40mail.gmail.com
3. https://www.postgresql.org/message-id/CA%2BTgmobu0DUFCTF28DuAi975mEc4xYqX3xyt8RA0WbnyrYg%2BFw%40mail.gmail.com

------
Regards,
Alexander Korotkov
Supabase

---

From: Alena Rybakina <[email-masked]>
Date: Wed, 17 Jul 2024 15:53:24 +0300

Hi! Thanks for your contribution to this topic!

> element to an index and quit immediately on failure.

Yes I see that. I will look at this in detail, but so far I have not 
found any unpleasant side effects indicating that the patch should be 
moved to another place and this is very good)

The only thing that worries me so far is that most likely we will need 
to analyze the changes in rinfo and distribute them to others places 
where links about them are used.
But I need to look at this in more detail separately before discussing it.

Yes, I am ready to agree that there was no degradation in tests [1] and 
[2]. But just in case, I will do a review to rule out any other problems.
> 3. https://www.postgresql.org/message-id/CA%2BTgmobu0DUFCTF28DuAi975mEc4xYqX3xyt8RA0WbnyrYg%2BFw%40mail.gmail.com

Thanks for your effort and any help is welcome)

Yesterday I finished a big project in my work and now I'm ready to 
continue working on this thread. I'll write the results one of these days.

-- 
Regards,
Alena Rybakina
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

---

From: Alexander Korotkov <[email-masked]>
Date: Wed, 17 Jul 2024 22:36:19 +0300

Hi, Alena!

On Wed, Jul 17, 2024 at 3:53 PM Alena Rybakina <[email-masked]>
wrote:

> [1], [2]). It remains only to add tests where the conversion should work,
> but I will add this in the next version.
> >>
> >> [0]

I'm not sure if would need to distribute changes of RestrictInfo's, because
we're modifying anything in-place.  Instead we create a new RestrictInfo
for IndexOptInfo.  I think this is what Robert proposed at [1].  The side
effect of this I yet see is redundancy of clauses in [2] test case.

                                                               QUERY PLAN

----------------------------------------------------------------------------------------------------------------------------------------
 Bitmap Heap Scan on t  (cost=19.70..26.93 rows=5001 width=12)
   Recheck Cond: (((b = 1) AND (b = ANY ('{1,2}'::integer[])) AND (c = 2))
OR ((a = 1) AND (b = 2) AND (b = ANY ('{1,2}'::integer[]))))
   Filter: ((a = 1) AND (c = 2))
   ->  BitmapOr  (cost=19.70..19.70 rows=2 width=0)
         ->  Bitmap Index Scan on t_b_c_idx  (cost=0.00..8.60 rows=1
width=0)
               Index Cond: ((b = 1) AND (b = ANY ('{1,2}'::integer[])) AND
(c = 2))
         ->  Bitmap Index Scan on t_a_b_idx  (cost=0.00..8.60 rows=1
width=0)
               Index Cond: ((a = 1) AND (b = 2) AND (b = ANY
('{1,2}'::integer[])))
(8 rows)

You can see that index conds and recheck conds contain both SAOP clauses
and equality clauses.  I this this happens because bitmap scan planning
code doesn't understands equivalency of original and transformed
RestrictInfo's.  I'm not yet sure what to do about this.  We probably need
to teach bitmap scan planning code to understand this equivalency.  Or,
otherwise, just allow this redundancy given that this is quite rare case I
believe.

> > 1.
> https://www.postgresql.org/message-id/CAPpHfduJtO0s9E%3DSHUTzrCD88BH0eik0UNog1_q3XBF2wLmH6g%40mail.gmail.com
> > 2.
> https://www.postgresql.org/message-id/CAPpHfdtSXxhdv3mLOLjEewGeXJ%2BFtfhjqodn1WWuq5JLsKx48g%40mail.gmail.com
> > 3.
> https://www.postgresql.org/message-id/CA%2BTgmobu0DUFCTF28DuAi975mEc4xYqX3xyt8RA0WbnyrYg%2BFw%40mail.gmail.com
>

Great, thank you. I would appreciate your further work on this patch.
Apart from general feedback on approach, the last patch requires comments,
code beautification etc.

Links.
1.
https://www.postgresql.org/message-id/CA%2BTgmoarYLO6PL%2BFEnXJ6A-57KsVsotpvHnB771M-wXQOGNy9w%40mail.gmail.com
2.
https://www.postgresql.org/message-id/CAPpHfdtSXxhdv3mLOLjEewGeXJ%2BFtfhjqodn1WWuq5JLsKx48g%40mail.gmail.com

------
Regards,
Alexander Korotkov
Supabase

---

From: Nikolay Shaplov <[email-masked]>
Date: Sun, 21 Jul 2024 11:17:01 +0300

В письме от среда, 17 июля 2024 г. 22:36:19 MSK пользователь Alexander 
Korotkov написал:

Hi All!

I am continue reading the patch, now it's newer version

First main question:

As far a I can get, the entry point for OR->ANY convertation have been moved 
to match_clause_to_indexcol funtion, that checks if some restriction can use 
index for performance.

The thing I do not understand what match_clause_to_indexcol actually received 
as arguments. Should this be set of expressions  with OR in between grouped by 
one of the expression argument?

If not I do not understand how this ever should work.

The rest is about code readability

> +	if (bms_is_member(index->rel->relid, rinfo->right_relids))
> +		return NULL;

This check it totally not obvious for person who is not deep into postgres 
code. There should go comment explaining what are we checking for, and why it 
does not suit our purposes

> +	foreach(lc, orclause->args)
> +	{
Being no great expert in postgres code, I am confused what are we iterating on 
here? Two arguments of OR statement? (a>1) OR (b>2) those in brackets? Or 
what? Comment explaining that would be a great help here.

> +if (sub_rinfo->is_pushed_down != rinfo->is_pushed_down ||
> +	sub_rinfo->is_clone != rinfo->is_clone ||
> +	sub_rinfo->security_level != rinfo->security_level ||
> +	!bms_equal(sub_rinfo->required_relids, rinfo->required_relids) ||
> +	!bms_equal(sub_rinfo->incompatible_relids, rinfo- 
incompatible_relids) ||
> +	!bms_equal(sub_rinfo->outer_relids, rinfo->outer_relids))
> +	{ 

This check it totally mind-blowing... What in the name of existence is going 
on here?

I would suggest  to split these checks into parts (compiler optimizer should 
take care about overhead)  and give each part a sane explanation.