Simplify the planner's new representation of indexable clauses a little.

In commit 1a8d5afb0, I thought it'd be a good idea to define
IndexClause.indexquals as NIL in the most common case where the given
clause (IndexClause.rinfo) is usable exactly as-is.  It'd be more
consistent to define the indexquals in that case as being a one-element
list containing IndexClause.rinfo, but I thought saving the palloc
overhead for making such a list would be worthwhile.

In hindsight, that was a great example of "premature optimization is the
root of all evil": it's complicated everyplace that needs to deal with
the indexquals, requiring duplicative code to handle both the simple
case and the not-simple case.  I'd initially found that tolerable but
it's getting less so as I mop up some areas that I'd not touched in
1a8d5afb0.  In any case, two more pallocs during a planner run are
surely at the noise level (a conclusion confirmed by a bit of
microbenchmarking).  So let's change this decision before it becomes
set in stone, and insist that IndexClause.indexquals always be a valid
list of the actual index quals for the clause.

Discussion: https://postgr.es/m/24586.1550106354@sss.pgh.pa.us

1 files changed, 10 insertions, 16 deletions

diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 616ec3b3dbf..a008ae07da5 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1185,26 +1185,20 @@ typedef struct IndexPath
  * conditions is done by a planner support function attached to the
  * indexclause's top-level function or operator.
  *
- * If indexquals is NIL, it means that rinfo->clause is directly usable as
- * an indexqual.  Otherwise indexquals contains one or more directly-usable
- * indexqual conditions extracted from the given clause.  The 'lossy' flag
- * indicates whether the indexquals are semantically equivalent to the
- * original clause, or form a weaker condition.
- *
- * Currently, entries in indexquals are RestrictInfos, but they could perhaps
- * be bare clauses instead; the only advantage of making them RestrictInfos
- * is the possibility of caching cost and selectivity information across
- * multiple uses, and it's not clear that that's really worth the price of
- * constructing RestrictInfos for them.  Note however that the extended-stats
- * machinery won't do anything with non-RestrictInfo clauses, so that would
- * have to be fixed.
+ * indexquals is a list of RestrictInfos for the directly-usable index
+ * conditions associated with this IndexClause.  In the simplest case
+ * it's a one-element list whose member is iclause->rinfo.  Otherwise,
+ * it contains one or more directly-usable indexqual conditions extracted
+ * from the given clause.  The 'lossy' flag indicates whether the
+ * indexquals are semantically equivalent to the original clause, or
+ * represent a weaker condition.
  *
  * Normally, indexcol is the index of the single index column the clause
  * works on, and indexcols is NIL.  But if the clause is a RowCompareExpr,
  * indexcol is the index of the leading column, and indexcols is a list of
  * all the affected columns.  (Note that indexcols matches up with the
- * columns of the actual indexable RowCompareExpr, which might be in
- * indexquals rather than rinfo.)
+ * columns of the actual indexable RowCompareExpr in indexquals, which
+ * might be different from the original in rinfo.)
  *
  * An IndexPath's IndexClause list is required to be ordered by index
  * column, i.e. the indexcol values must form a nondecreasing sequence.
@@ -1214,7 +1208,7 @@ typedef struct IndexClause
 {
 	NodeTag		type;
 	struct RestrictInfo *rinfo; /* original restriction or join clause */
-	List	   *indexquals;		/* indexqual(s) derived from it, or NIL */
+	List	   *indexquals;		/* indexqual(s) derived from it */
 	bool		lossy;			/* are indexquals a lossy version of clause? */
 	AttrNumber	indexcol;		/* index column the clause uses (zero-based) */
 	List	   *indexcols;		/* multiple index columns, if RowCompare */