Paul Jungwirth <pj@illuminatedcomputing.com> writes:
> On 7/6/24 05:04, Andrey M. Borodin wrote:>> On 5 Jul 2024, at 23:46, Paul Jungwirth
> <pj@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
