[Tom Lane <tgl@sss.pgh.pa.us>]

Apparently, whoever went through indxpath.c to substitute nkeycolumns
for ncolumns was not paying attention.  As far as I can tell, the
*only* place in there where it's correct to reference ncolumns is in
check_index_only, where we determine which columns can be extracted from
an index-only scan.  A lot of the other places are obviously wrong, eg in
relation_has_unique_index_for:

        for (c = 0; c < ind->ncolumns; c++)
        ...
                if (!list_member_oid(rinfo->mergeopfamilies, ind->opfamily[c]))

Even if it were plausible that an INCLUDE column is something to consider
when deciding whether the index enforces uniqueness, this code accesses
beyond the documented end of the opfamily[] array :-(

The fact that the regression tests haven't caught this doesn't give
me a warm feeling about how thoroughly the included-columns logic has
been tested.  It's really easy to make it fall over, for instance

regression=# explain select * from tenk1 where (thousand, tenthous) < (10,100);
                                     QUERY PLAN                                 
     
--------------------------------------------------------------------------------
-----
 Bitmap Heap Scan on tenk1  (cost=5.11..233.86 rows=107 width=244)
   Recheck Cond: (ROW(thousand, tenthous) < ROW(10, 100))
   ->  Bitmap Index Scan on tenk1_thous_tenthous  (cost=0.00..5.09 rows=107 widt
h=0)
         Index Cond: (ROW(thousand, tenthous) < ROW(10, 100))
(4 rows)

regression=# drop index tenk1_thous_tenthous;
DROP INDEX
regression=# create index on tenk1(thousand) include (tenthous);
CREATE INDEX
regression=# explain select * from tenk1 where (thousand, tenthous) < (10,100);
ERROR:  operator 97 is not a member of opfamily 2139062142

I've got mixed feelings about whether to try to fix this before
tomorrow's wraps.  The attached patch seems correct and passes
check-world, but there's sure not a lot of margin for error now.
Thoughts?

			regards, tom lane

Attachments:

  [text/x-diff] ignore-included-columns-in-indxpath-c.patch (2.8K, ../25526.1549847928@sss.pgh.pa.us/2-ignore-included-columns-in-indxpath-c.patch)
  download | inline diff:
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 51d2da5..a55b9e0 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -258,7 +258,7 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
 		IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);
 
 		/* Protect limited-size array in IndexClauseSets */
-		Assert(index->ncolumns <= INDEX_MAX_KEYS);
+		Assert(index->nkeycolumns <= INDEX_MAX_KEYS);
 
 		/*
 		 * Ignore partial indexes that do not match the query.
@@ -473,7 +473,7 @@ consider_index_join_clauses(PlannerInfo *root, RelOptInfo *rel,
 	 * relation itself is also included in the relids set.  considered_relids
 	 * lists all relids sets we've already tried.
 	 */
-	for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
+	for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
 	{
 		/* Consider each applicable simple join clause */
 		considered_clauses += list_length(jclauseset->indexclauses[indexcol]);
@@ -628,7 +628,7 @@ get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
 	/* Identify indexclauses usable with this relids set */
 	MemSet(&clauseset, 0, sizeof(clauseset));
 
-	for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
+	for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
 	{
 		ListCell   *lc;
 
@@ -925,7 +925,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
 	index_clauses = NIL;
 	found_lower_saop_clause = false;
 	outer_relids = bms_copy(rel->lateral_relids);
-	for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
+	for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
 	{
 		ListCell   *lc;
 
@@ -2680,7 +2680,7 @@ match_pathkeys_to_index(IndexOptInfo *index, List *pathkeys,
 			 * amcanorderbyop.  We might need different logic in future for
 			 * other implementations.
 			 */
-			for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
+			for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
 			{
 				Expr	   *expr;
 
@@ -3111,7 +3111,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
 		 * Try to find each index column in the lists of conditions.  This is
 		 * O(N^2) or worse, but we expect all the lists to be short.
 		 */
-		for (c = 0; c < ind->ncolumns; c++)
+		for (c = 0; c < ind->nkeycolumns; c++)
 		{
 			bool		matched = false;
 			ListCell   *lc;
@@ -3187,7 +3187,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
 		}
 
 		/* Matched all columns of this index? */
-		if (c == ind->ncolumns)
+		if (c == ind->nkeycolumns)
 			return true;
 	}
 
@@ -3991,7 +3991,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
 
 				break;
 		}
-		if (i >= index->ncolumns)
+		if (i >= index->nkeycolumns)
 			break;				/* no match found */
 
 		/* Add column number to returned list */