public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andrei Lepikhov <[email protected]>
To: Alena Rybakina <[email protected]>
To: [email protected]
Cc: Peter Geoghegan <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Finnerty, Jim <[email protected]>
Cc: Marcos Pegoraro <[email protected]>
Cc: [email protected]
Cc: Ranier Vilela <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: Mon, 20 Nov 2023 15:52:08 +0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAPpHfduJtO0s9E=SHUTzrCD88BH0eik0UNog1_q3XBF2wLmH6g@mail.gmail.com>
	<CA+Tgmob=ebuCHFSw327b55DJzE3JtOuZ5owxob+MgErb4me_Ag@mail.gmail.com>
	<[email protected]>
	<CA+TgmoaA+OXewcifo7ypr1szb_JP6m-qJp+K8iSenOPi9mtrbg@mail.gmail.com>
	<CAH2-Wzkrg_nnh-xvnFZhNKEMJf0hZ0putQaqWhQrezEnb8b+XA@mail.gmail.com>
	<CA+Tgmoa+ywixgrgV2x4QHJgLvre31wJDsx-dC1gXDmtJUgv=Ww@mail.gmail.com>
	<CAPpHfdt58O5jzKMjkScNbGecM5JqfaMfdGLjs4rk5XisfjTNBw@mail.gmail.com>
	<[email protected]>
	<[email protected]>

On 10/11/2023 16:20, Alena Rybakina wrote:
>> I added log like that: ERROR:  unrecognized node type: 0.
> I fixed this issue and added some cosmetic refactoring.
> The changes are presented in the or_patch_changes.diff file.

Looking into the patch, I found some trivial improvements (see attachment).
Also, it is not obvious that using a string representation of the clause 
as a hash table key is needed here. Also, by making a copy of the node 
in the get_key_nconst_node(), you replace the location field, but in the 
case of complex expression, you don't do the same with other nodes.
I propose to generate expression hash instead + prove the equality of 
two expressions by calling equal().

-- 
regards,
Andrei Lepikhov
Postgres Professional

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 25a4235dbd..de27d2646c 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -178,6 +178,10 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 	HTAB 		   *or_group_htab = NULL;
 	int 			len_ors = list_length(expr_orig->args);
 
+	/* If this is not an 'OR' expression, skip the transformation */
+	if (!or_transform_limit || expr_orig->boolop != OR_EXPR || len_ors < 2)
+		return transformBoolExpr(pstate, (BoolExpr *) expr_orig);
+
 	MemSet(&info, 0, sizeof(info));
 	info.keysize = sizeof(char *);
 	info.entrysize = sizeof(OrClauseGroupEntry);
@@ -188,10 +192,6 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 									  &info,
 									  HASH_ELEM | HASH_FUNCTION | HASH_COMPARE);
 
-	/* If this is not an 'OR' expression, skip the transformation */
-	if (expr_orig->boolop != OR_EXPR || !or_transform_limit || len_ors == 1 || !or_group_htab)
-		return transformBoolExpr(pstate, (BoolExpr *) expr_orig);
-
 	foreach(lc, expr_orig->args)
 	{
 		Node			   *arg = lfirst(lc);


Attachments:

  [text/plain] changes.txt (1.1K, ../[email protected]/2-changes.txt)
  download | inline diff:
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 25a4235dbd..de27d2646c 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -178,6 +178,10 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 	HTAB 		   *or_group_htab = NULL;
 	int 			len_ors = list_length(expr_orig->args);
 
+	/* If this is not an 'OR' expression, skip the transformation */
+	if (!or_transform_limit || expr_orig->boolop != OR_EXPR || len_ors < 2)
+		return transformBoolExpr(pstate, (BoolExpr *) expr_orig);
+
 	MemSet(&info, 0, sizeof(info));
 	info.keysize = sizeof(char *);
 	info.entrysize = sizeof(OrClauseGroupEntry);
@@ -188,10 +192,6 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
 									  &info,
 									  HASH_ELEM | HASH_FUNCTION | HASH_COMPARE);
 
-	/* If this is not an 'OR' expression, skip the transformation */
-	if (expr_orig->boolop != OR_EXPR || !or_transform_limit || len_ors == 1 || !or_group_htab)
-		return transformBoolExpr(pstate, (BoolExpr *) expr_orig);
-
 	foreach(lc, expr_orig->args)
 	{
 		Node			   *arg = lfirst(lc);


view thread (24+ messages)  latest in thread

reply

Reply instructions:

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

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

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: POC, WIP: OR-clause support for indexes
  In-Reply-To: <[email protected]>

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

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