Thread: Curious test case added by collation version tracking patch

I am wondering what was the intent of this test case added by commit
257836a75:

CREATE INDEX icuidx16_mood ON collate_test(id) WHERE mood > 'ok' COLLATE "fr-x-icu";

where "mood" is of an enum type, which surely does not respond to
collations.

The reason I ask is that this case started failing after I fixed
a parse_coerce.c bug that allowed a CollateExpr node to survive
in this WHERE expression, which by rights it should not.  I'm
inclined to think that the test case is wrong and should be removed,
but maybe there's some reason to have a variant of it.

            regards, tom lane

On Tue, Apr 13, 2021 at 8:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I am wondering what was the intent of this test case added by commit
> 257836a75:
>
> CREATE INDEX icuidx16_mood ON collate_test(id) WHERE mood > 'ok' COLLATE "fr-x-icu";
>
> where "mood" is of an enum type, which surely does not respond to
> collations.
>
> The reason I ask is that this case started failing after I fixed
> a parse_coerce.c bug that allowed a CollateExpr node to survive
> in this WHERE expression, which by rights it should not.  I'm
> inclined to think that the test case is wrong and should be removed,
> but maybe there's some reason to have a variant of it.

Indeed, this doesn't do anything useful, other than prove that we
record a collation dependency where it is (uselessly) allowed in an
expression.  Since you're not going to allow that anymore, it should
be dropped.

Thomas Munro <thomas.munro@gmail.com> writes:
> On Tue, Apr 13, 2021 at 8:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The reason I ask is that this case started failing after I fixed
>> a parse_coerce.c bug that allowed a CollateExpr node to survive
>> in this WHERE expression, which by rights it should not.  I'm
>> inclined to think that the test case is wrong and should be removed,
>> but maybe there's some reason to have a variant of it.

> Indeed, this doesn't do anything useful, other than prove that we
> record a collation dependency where it is (uselessly) allowed in an
> expression.  Since you're not going to allow that anymore, it should
> be dropped.

OK, I'll go clean it up.  Thanks!

            regards, tom lane