Re: Cross-type index comparison support in contrib/btree_gin

On Fri, Mar 28, 2025 at 4:22 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> v3 needed to rebase over 55527368b. (I guess "git am" cannot
> tolerate any fuzz at all?) No changes of any significance
> whatsoever.
>
> regards, tom lane
>

Hi!

Thank you for working on this.

I tried the patch and it compiles and works as expected. Several minor
things I noticed:

1) btree_gin.c

case BTEqualStrategyNumber:
if (cmp > 0)
res = -1; /* keep scanning */
else if (cmp == 0)
res = 0;
else
res = 1; /* end scan */
break;

I think the code is correct, but do we need to continue scanning here?
I can't think of an example where we have cmp == 0 after cmp > 0.
Maybe we can use cmp != 0 as a stopping condition?

2) btree_gin.c

switch (data->strategy & 7)
{
case BTLessStrategyNumber:

There are two places where we extract btree_start from the input
strategy. Maybe we can store the extracted value in QueryInfo? Or
create macros to avoid code duplication.

3) float4.sql

-- Check endpoint and out-of-range cases

INSERT INTO test_float4 VALUES ('NaN'), ('Inf'), ('-Inf');

It seems that this test data is in the pending list during the test.
Not sure if we want them to be in the entry tree or not? The same with
date.sql and timestamp.sql.

4)
On 02.02.2025 04:44, Tom Lane wrote:
> ...
>
> * Query value falls between possible values of the index type
> (possible in float8->float4 or timestamp->date cases, for example).
> We can just use our usual conversion rules, though. The critical
> observation here is that it does not matter whether the conversion
> rounds to the next lower or next higher possible value. If we are
> searching for equality, neither of those values will pass the
> cross-type comparison so it doesn't matter. If we are searching for
> inequality, for example "indcol <= value", then only index entries
> strictly less than the query value can match. Rounding down clearly
> doesn't hurt, while rounding up at worst makes the search include
> some index entries just larger than the query value, which will be
> correctly rejected by the cross-type comparison.

I agree with the statements. It's quite a tricky part as for me,
probably it's better to add tests for this special case as it's done
for 'out of range' cases. FWIW while testing the patch I wrote some
tests for these rounding cases, I'm ready to add it to the patchset.

Thank you!

Best regards,
Arseniy Mukhin