From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Geoghegan <pg@bowt.ie>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: indxpath.c's references to IndexOptInfo.ncolumns are all wrong, no?
Date: Sun, 10 Feb 2019 21:10:14 -0500
Message-ID: <27778.1549851014@sss.pgh.pa.us> (raw)
In-Reply-To: <CAH2-WzmcfxB-7VNLzrCmvMD-gb-4e5WEDM6jurn04m8ymHFviQ@mail.gmail.com>
References: <25526.1549847928@sss.pgh.pa.us>
<CAH2-WzmcfxB-7VNLzrCmvMD-gb-4e5WEDM6jurn04m8ymHFviQ@mail.gmail.com>
Peter Geoghegan <pg@bowt.ie> writes:
> On Sun, Feb 10, 2019 at 5:18 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.
> Ugh. Yeah, it's rather inconsistent.
Looking closer, it seems like most of these are accidentally protected
by the fact that match_clause_to_index stops at nkeycolumns, so that
the indexclauses lists for later columns can never become nonempty;
they're merely wasting cycles by iterating over later columns, rather
than doing anything seriously wrong. It would be possible to get
match_pathkeys_to_index to trigger the assertion in
match_clause_to_ordering_op if GIST supported included columns,
but it doesn't. And I think relation_has_unique_index_for can be
fooled into reporting that an index doesn't prove uniqueness, when
it does. But the only one of these that's really got obviously bad
consequences is the one in expand_indexqual_rowcompare, which triggers
the failure I showed before. It's also the most obviously wrong code:
/*
* The Var side can match any column of the index.
*/
for (i = 0; i < index->nkeycolumns; i++)
{
if (...)
break;
}
if (i >= index->ncolumns)
break; /* no match found */
Even without understanding the bigger picture, any C programmer ought to
find that loop logic pretty fishy. (I'm a bit surprised Coverity hasn't
whined about it.)
Maybe the right compromise is to fix expand_indexqual_rowcompare
now and leave the rest for post-wrap.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pg@bowt.ie, pgsql-hackers@lists.postgresql.org
Subject: Re: indxpath.c's references to IndexOptInfo.ncolumns are all wrong, no?
In-Reply-To: <27778.1549851014@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox