Re: [PG19] eager aggregation gives wrong results because of bpchar_ops

shihao zhong <zhong950419(at)gmail(dot)com> writes:
> I used Opus to go over the new features in PG19, and it found wrong
> results from eager aggregation on a bpchar column with no length.

I went through all the data types that allege equalimage support
carefully, and found another problem: oidvector is claimed to
behave this way, but it does not. In particular, btoidvectorcmp
intentionally pays no attention to the array lower bound field.

While oidvectors would normally have a lower bound of 0, it's
not hard at all to make one that doesn't:

regression=# select array_lower(array[12,34]::oidvector, 1);
array_lower
-------------
1
(1 row)

So we find that these things are equal per btoidvectorcmp:

regression=# select array[12,34]::oidvector = '12 34'::oidvector;
?column?
----------
t
(1 row)

but they are not actually bitwise equal:

regression=# select array[12,34]::oidvector::oid[] = '12 34'::oidvector::oid[];
?column?
----------
f
(1 row)

So my first instinct is that we'd better rescind equalimage support
for oidvector too. But then pg_proc_proname_args_nsp_index and
perhaps some other system catalog indexes will start failing amcheck
in existing installations. That'd likely cause enough trouble to
outweigh the safety argument, especially since I really doubt that
it's possible to get a nonstandard oidvector value into a catalog
without doing superuser-y things.

So I'm not quite sure what to do about this. Maybe rescind in
HEAD/v19, and do nothing in the back branches?

By the by, I'm not especially happy about the errhint that Noah
added in 5f27b5f84:

+ has_interval_ops
+ ? errhint("This is known of \"interval\" indexes last built on a version predating 2023-11.")
+ : 0));

This seems to me to have passed its sell-by date somewhere around
2024. I see that Shihao's patch tries to emulate that, but doing so
for three or so different types seems to me like it will be a complete
mess, and I see no real value in it. The outcome is the same
regardless of cause: you'd better REINDEX.

regards, tom lane