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
- ignore-included-columns-in-indxpath-c.patch (text/x-diff) patch
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
-
Clean up planner confusion between ncolumns and nkeycolumns.
- 364857f73898 11.3 landed
- 75c46149fc22 12.0 landed
-
Fix indexable-row-comparison logic to account for covering indexes.
- eb68d71f99e9 11.2 landed
- 6bdc3005b54f 12.0 landed