Paul Jungwirth <p...@illuminatedcomputing.com> writes:
> On 7/6/24 05:04, Andrey M. Borodin wrote:>> On 5 Jul 2024, at 23:46, Paul 
> Jungwirth 
> <p...@illuminatedcomputing.com> wrote:
>>> this commit adds support for all combinations of int2/int4/int8 for all 
>>> five btree operators (</<=/=/>=/>).

Perhaps I'm missing something, but how can this possibly work without
any changes to the C code?

For example, gbt_int4_consistent assumes that the comparison value
is always an int4.  Due to the way we represent Datums in-memory,
this will kind of work if it's really an int2 or int8 --- unless the
comparison value doesn't fit in int4, and then you will get a
completely wrong answer based on a value truncated to int4.  (But I
would argue that even the cases where it seems to work are a type
violation, and getting the right answer is accidental if you have not
applied the correct PG_GETARG conversion macro.)  Plus, int4-vs-int8
comparisons will fail in very obvious ways, up to and including core
dumps, on 32-bit machines where int8 is pass-by-reference.

> Here is another patch adding float4/8 and also date/timestamp/timestamptz, in 
> the same combinations 
> as btree.

Similar problems, plus comparing timestamp to timestamptz requires a
timezone conversion that this code isn't doing.

I think to make this work, you'd need to define a batch of new
opclass-specific strategy numbers that convey both the kind of
comparison to perform and the type of the RHS value.  And then
there would need to be a nontrivial amount of new code in the
consistent functions to deal with cross-type cases.

                        regards, tom lane


Reply via email to