0001-Comments-for-the-0001-patch.patch
text/x-patch
Filename: 0001-Comments-for-the-0001-patch.patch
Type: text/x-patch
Part: 0
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: format-patch
Series: patch 0001
Subject: Comments for the 0001 patch
| File | + | − |
|---|---|---|
| src/backend/optimizer/path/indxpath.c | 31 | 21 |
From ae279c2bef8b0b11739f79b49d68319894c63aa9 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <lepihov@gmail.com>
Date: Wed, 9 Oct 2024 14:05:25 +0700
Subject: [PATCH 1/3] Comments for the 0001 patch
---
src/backend/optimizer/path/indxpath.c | 52 ++++++++++++++++-----------
1 file changed, 31 insertions(+), 21 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 46d576a0c2..602141911d 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -2557,6 +2557,10 @@ match_clause_to_index(PlannerInfo *root,
* It is also possible to match ScalarArrayOpExpr clauses to indexes, when
* the clause is of the form "indexkey op ANY (arrayconst)".
*
+ * It is also possible to match a list of OR clauses if it may be transformed
+ * into single ScalarArrayOpExpr clause. On success, returning index clause
+ * will contain trasformed clause.
+ *
* For boolean indexes, it is also possible to match the clause directly
* to the indexkey; or perhaps the clause is (NOT indexkey).
*
@@ -3157,6 +3161,10 @@ match_rowcompare_to_indexcol(PlannerInfo *root,
* match_orclause_to_indexcol()
* Handles the OR-expr case for match_clause_to_indexcol() in the case
* when it could be transformed to ScalarArrayOpExpr.
+ *
+ * Given a list of an OR-clause args, attempts to transform this BoolExpr
+ * into single SAOP expression. On success, returns IndexClause, containing
+ * transformed expression or NULL, if failed.
*/
static IndexClause *
match_orclause_to_indexcol(PlannerInfo *root,
@@ -3184,11 +3192,12 @@ match_orclause_to_indexcol(PlannerInfo *root,
Assert(orclause->boolop == OR_EXPR);
/*
- * Iterate over OR entries. Check that each OR entry is of the form:
- * (indexkey operator constant) or (constant operator indexkey). Operators
- * of all the entries must match. Constant might be either Const or
- * Param. Exit with NULL on first non-matching entry. Exit is
- * implemented as a break from the loop, which is catched afterwards.
+ * Try to convert list of OR-clauses to single SAOP expression. Each OR
+ * entry must be in the form: (indexkey operator constant) or (constant
+ * operator indexkey). Operators of all the entries must match. Constant
+ * might be either Const or Param. To be effective, give up on first
+ * non-matching entry. Exit is implemented as a break from the loop, which
+ * is catched afterwards.
*/
foreach(lc, orclause->args)
{
@@ -3286,8 +3295,8 @@ match_orclause_to_indexcol(PlannerInfo *root,
break;
/*
- * For the first matching qual, save information about operator, type
- * and collation. For the other quals just check the match with the
+ * Save information about the operator, type, and collation for the
+ * first matching qual. Then, check that subsequent quals match the
* first.
*/
if (firstTime)
@@ -3298,9 +3307,9 @@ match_orclause_to_indexcol(PlannerInfo *root,
inputcollid = subClause->inputcollid;
/*
- * Check operator is present in the opfamily, expression collation
- * matches index collation. Also, there must be an array type in
- * order to construct an array later.
+ * Check operator is presented in the opfamily and expression
+ * collation matches index collation. Also, there must be an array
+ * type to construct an array later.
*/
if (!IndexCollMatchesExprColl(index->indexcollations[indexcol], inputcollid) ||
!op_in_opfamily(matchOpno, index->opfamily[indexcol]) ||
@@ -3332,12 +3341,14 @@ match_orclause_to_indexcol(PlannerInfo *root,
return NULL;
}
+ /*
+ * Assemble an array from the list of constants. It seems more profitable to
+ * build a const array. But in presence of parameters we don't have specific
+ * value here and must employ an ArrayExpr instead.
+ */
+
if (have_param)
{
- /*
- * We need to construct an ArrayExpr given we have Param's not just
- * Const's.
- */
ArrayExpr *arrayExpr = makeNode(ArrayExpr);
/* array_collid will be set by parse_collate.c */
@@ -3351,10 +3362,6 @@ match_orclause_to_indexcol(PlannerInfo *root,
}
else
{
- /*
- * We have only Const's. In this case we can construct an array
- * directly.
- */
int16 typlen;
bool typbyval;
char typalign;
@@ -3383,8 +3390,7 @@ match_orclause_to_indexcol(PlannerInfo *root,
}
/* Lookup for operator to fetch necessary information for the SAOP node */
- opertup = SearchSysCache1(OPEROID,
- ObjectIdGetDatum(matchOpno));
+ opertup = SearchSysCache1(OPEROID, ObjectIdGetDatum(matchOpno));
if (!HeapTupleIsValid(opertup))
elog(ERROR, "cache lookup failed for operator %u", matchOpno);
@@ -3404,7 +3410,11 @@ match_orclause_to_indexcol(PlannerInfo *root,
ReleaseSysCache(opertup);
/*
- * Finally build an IndexClause based on the SAOP node.
+ * Finally build an IndexClause based on the SAOP node. Use
+ * make_simple_restrictinfo to get RestrictInfo with clean selectivity
+ * estimations because it may differ from estimation made for an OR clause.
+ * Although it is not a lossy expression, keep old version of rinfo in
+ * iclause->rinfo to detect duplicates and recheck original clause.
*/
iclause = makeNode(IndexClause);
iclause->rinfo = rinfo;
--
2.39.5