Re: POC, WIP: OR-clause support for indexes - Mailing list pgsql-hackers

From jian he
Subject Re: POC, WIP: OR-clause support for indexes
Date
Msg-id CACJufxHCJvC3X8nUK-jRvRru-ZEXp16EBPADOwTGaqmOYM1Raw@mail.gmail.com
Whole thread Raw
In response to POC, WIP: OR-clause support for indexes  (Teodor Sigaev <teodor@sigaev.ru>)
List pgsql-hackers
On Fri, Aug 23, 2024 at 8:58 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
based on v37.


+ {
+ /*
+ * We have only Const's.  In this case we can construct an array
+ * directly.
+ */
+ int16 typlen;
+ bool typbyval;
+ char typalign;
+ Datum   *elems;
+ int i = 0;
+ ArrayType  *arrayConst;
+
+ get_typlenbyvalalign(consttype, &typlen, &typbyval, &typalign);
+
+ elems = (Datum *) palloc(sizeof(Datum) * list_length(consts));
+ foreach(lc, consts)
+ elems[i++] = ((Const *) lfirst(lc))->constvalue;
+
+ arrayConst = construct_array(elems, i, consttype,
+ typlen, typbyval, typalign);
+ arrayNode = (Node *) makeConst(arraytype, -1, inputcollid,
+   -1, PointerGetDatum(arrayConst),
+   false, false);
+
+ pfree(elems);
+ list_free(consts);
+ }
List "consts" elements can be NULL?
I didn't find a query to trigger that.
but construct_array comments says
"elems (NULL element values are not supported)."
Do we need to check Const->constisnull for the Const node?


+ /* Construct the list of nested OR arguments */
+ for (j = group_start; j < i; j++)
+ {
+ Node   *arg = list_nth(orargs, matches[j].argindex);
+
+ rargs = lappend(rargs, arg);
+ if (IsA(arg, RestrictInfo))
+ args = lappend(args, ((RestrictInfo *) arg)->clause);
+ else
+ args = lappend(args, arg);
+ }
the ELSE branch never reached?



+ /* Construct the nested OR and wrap it with RestrictInfo */
+ *subrinfo = *rinfo;
+ subrinfo->clause = make_orclause(args);
+ subrinfo->orclause = make_orclause(rargs);
+ result = lappend(result, subrinfo);
should we use memcpy instead of " *subrinfo = *rinfo;"?



+ /* Sort clauses to make similar clauses go together */
+ pg_qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
Should we use qsort?
since comments in pg_qsort:
/*
 * Callers should use the qsort() macro defined below instead of calling
 * pg_qsort() directly.
 */



+/*
+ * Data structure representing information about OR-clause argument and its
+ * matching index key.  Used for grouping of similar OR-clause arguments in
+ * group_similar_or_args().
+ */
+typedef struct
+{
+ int indexnum; /* index of the matching index */
+ int colnum; /* index of the matching column */
+ Oid opno; /* OID of the OpClause operator */
+ Oid inputcollid; /* OID of the OpClause input collation */
+ int argindex; /* index of the clause in the list of
+ * arguments */
+} OrArgIndexMatch;

I am not 100% sure about the comments.
indexnum:  index of the matching index reside in rel->indexlist that
matches (counting from 0)
colnum: the column number of the matched index (counting from 0)



pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Segfault in jit tuple deforming on arm64 due to LLVM issue
Next
From: 王春桂
Date:
Subject: how to log into commitfest.postgresql.org and begin review patch