Обсуждение: Curious test case added by collation version tracking patch

Поиск
Список
Период
Сортировка

Curious test case added by collation version tracking patch

От
Tom Lane
Дата:
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



Re: Curious test case added by collation version tracking patch

От
Thomas Munro
Дата:
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.



Re: Curious test case added by collation version tracking patch

От
Tom Lane
Дата:
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