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

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