Обсуждение: SELECT results in "ERROR: index key does not match expected index column"

Hi everyone,

Consider the following test case:

CREATE TABLE t0(c0 boolean UNIQUE);
CREATE INDEX i0 ON t0((nullif(FALSE, TRUE)));
INSERT INTO t0(c0) VALUES(TRUE);
SELECT * FROM t0 WHERE nullif(FALSE, TRUE) OR t0.c0; -- expected: row
is fetched, actual: ERROR:  index key does not match expected index
column

The SELECT above results in an error, which is unexpected. When
removing the UNIQUE keyword from c0, the CREATE INDEX statement, or
the OR t0.c0 in the query, the error does not occur. The error can
also be reproduced with other data types, such as INT.

I'm using the following Postgres version: psql (11.4 (Ubuntu
11.4-1.pgdg19.04+1)).

Best,
Manuel

Manuel Rigger <rigger.manuel@gmail.com> writes:
> Consider the following test case:

> CREATE TABLE t0(c0 boolean UNIQUE);
> CREATE INDEX i0 ON t0((nullif(FALSE, TRUE)));
> INSERT INTO t0(c0) VALUES(TRUE);
> SELECT * FROM t0 WHERE nullif(FALSE, TRUE) OR t0.c0;
> ERROR:  index key does not match expected index column

Huh.  I don't see that in HEAD/v12, but it does reproduce in v11
and quite a large number of branches before that.  Looking ...

(You realize of course that an index on a constant is pretty
useless.  I wonder if we should disallow that.)

            regards, tom lane

On 03/07/2019 10:50, Tom Lane wrote:
> Manuel Rigger <rigger.manuel@gmail.com> writes:
>> Consider the following test case:
>> CREATE TABLE t0(c0 boolean UNIQUE);
>> CREATE INDEX i0 ON t0((nullif(FALSE, TRUE)));
>> INSERT INTO t0(c0) VALUES(TRUE);
>> SELECT * FROM t0 WHERE nullif(FALSE, TRUE) OR t0.c0;
>> ERROR:  index key does not match expected index column
> Huh.  I don't see that in HEAD/v12, but it does reproduce in v11
> and quite a large number of branches before that.  Looking ...
>
> (You realize of course that an index on a constant is pretty
> useless.  I wonder if we should disallow that.)
>
>             regards, tom lane
>
>
I think it should cause an error, most likely an unintentional 
implementation mistake -- so the sooner it is flagged the better to 
remedy it!


Cheers,
Gavin

On Wed, Jul 3, 2019 at 10:50 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Manuel Rigger <rigger.manuel@gmail.com> writes:
> > Consider the following test case:
>
> > CREATE TABLE t0(c0 boolean UNIQUE);
> > CREATE INDEX i0 ON t0((nullif(FALSE, TRUE)));
> > INSERT INTO t0(c0) VALUES(TRUE);
> > SELECT * FROM t0 WHERE nullif(FALSE, TRUE) OR t0.c0;
> > ERROR:  index key does not match expected index column
>
> Huh.  I don't see that in HEAD/v12, but it does reproduce in v11
> and quite a large number of branches before that.  Looking ...

Git bisect points to this commit as the one that fixed the problem:

commit 1a8d5afb0dfc5d0dcc6eda0656a34cb1f0cf0bdf
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Sat Feb 9 17:30:43 2019 -0500

    Refactor the representation of indexable clauses in IndexPaths.

-- 
Thomas Munro
https://enterprisedb.com

Gavin Flower <GavinFlower@archidevsys.co.nz> writes:
> On 03/07/2019 10:50, Tom Lane wrote:
>> Manuel Rigger <rigger.manuel@gmail.com> writes:
>>> Consider the following test case:
>>> CREATE TABLE t0(c0 boolean UNIQUE);
>>> CREATE INDEX i0 ON t0((nullif(FALSE, TRUE)));
>>> INSERT INTO t0(c0) VALUES(TRUE);
>>> SELECT * FROM t0 WHERE nullif(FALSE, TRUE) OR t0.c0;
>>> ERROR:  index key does not match expected index column

>> Huh.  I don't see that in HEAD/v12, but it does reproduce in v11
>> and quite a large number of branches before that.  Looking ...

So the problem here is that fix_indexqual_references hasn't got
enough information to guess whether the derived indexable clause
"nullif(FALSE, TRUE) = TRUE" needs to be commuted or not, and it
guesses wrong:

            /*
             * Check to see if the indexkey is on the right; if so, commute
             * the clause.  The indexkey should be the side that refers to
             * (only) the base relation.
             */
            if (!bms_equal(rinfo->left_relids, index->rel->relids))
                CommuteOpExpr(op);

In this example both left_relids and right_relids are empty (since
both sides of the comparison are constants), so there's no way for it
to do the right thing.  We could invert the way the test is done, but
that would just move the failure to other cases where commutation
*is* needed.

This is fixed somewhat accidentally in HEAD/v12 because I got rid of
the need for fix_indexqual_references to guess anything, by dint of
requiring the index clause to have been generated with indexkey on
the left to begin with:

    Author: Tom Lane <tgl@sss.pgh.pa.us>
    Branch: master [1a8d5afb0] 2019-02-09 17:30:43 -0500

    Refactor the representation of indexable clauses in IndexPaths.
    ...
    We also change the ground rules a bit by mandating that clause commutation,
    if needed, be done up-front so that what is stored in the indexquals list
    is always directly usable as an index condition.  This gets rid of repeated
    re-determination of which side of the clause is the indexkey during costing
    and plan generation, as well as repeated lookups of the commutator
    operator.

There's exactly no chance that we'd back-patch a change as invasive
as that was, of course.

This problem has been there a *long* time --- the given test case
fails back to 8.1, and I can make variants of it fail in 7.4, and
probably further back but I'm disinclined to spin up a machine with
an older server just to check.  Probably the reason it's not been
complained of more is that it's pretty hard to get to --- usually,
an all-constant qual condition wouldn't be a candidate to be matched to
an index in the first place.  The "OR" with another, more obviously
indexable, qual is probably essential to expose the problem.

Given the history and the lack of any easy fix, I'm inclined to write
this off as "won't fix in back branches".

>> (You realize of course that an index on a constant is pretty
>> useless.  I wonder if we should disallow that.)

> I think it should cause an error, most likely an unintentional
> implementation mistake -- so the sooner it is flagged the better to
> remedy it!

On reflection that might be overkill.  I've heard of people using
unique indexes on constants to constrain tables to one row, and
you can make interesting refinements by making the index partial.

(If you want to do something like that in released branches, the
workaround for this problem would be to use a constant value that
doesn't match anything that'd appear in your queries...)

            regards, tom lane

On Tue, Jul 2, 2019 at 4:46 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I think it should cause an error, most likely an unintentional
> > implementation mistake -- so the sooner it is flagged the better to
> > remedy it!
>
> On reflection that might be overkill.  I've heard of people using
> unique indexes on constants to constrain tables to one row, and
> you can make interesting refinements by making the index partial.

I think it's overkill. As you pointed out not too long ago, there are
several ways to define an index that probably or definitely results in
a useless index. There is no reason to draw the line here, or
anywhere.

-- 
Peter Geoghegan

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 Tom> (You realize of course that an index on a constant is pretty
 Tom> useless. I wonder if we should disallow that.)

Unique index on a constant is a standard technique for forcing a table
to be limited to at most one row (or with a partial index, at most one
row matching a condition). Non-unique index on a constant is probably
not useful though.

-- 
Andrew (irc:RhodiumToad)

Just to clarify: I'm developing a tool to find bugs in Postgres, and
this is one of the findings, which could be the reason that some of
the statements might look unusual.

Best,
Manuel

On Wed, Jul 3, 2019 at 4:44 AM Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
>
> >>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>
>  Tom> (You realize of course that an index on a constant is pretty
>  Tom> useless. I wonder if we should disallow that.)
>
> Unique index on a constant is a standard technique for forcing a table
> to be limited to at most one row (or with a partial index, at most one
> row matching a condition). Non-unique index on a constant is probably
> not useful though.
>
> --
> Andrew (irc:RhodiumToad)