indxpath.c's references to IndexOptInfo.ncolumns are all wrong, no?

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

From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2019-02-11T01:18:48Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Clean up planner confusion between ncolumns and nkeycolumns.

  2. Fix indexable-row-comparison logic to account for covering indexes.