On 03/04/2018 02:37 AM, Tom Lane wrote: > Tomas Vondra <tomas.von...@2ndquadrant.com> writes: >> Here is v2 of the fix. It does handle all the convert_to_scalar calls >> for various data types, just like the numeric one did in v1 with the >> exception of bytea. > > Pushed with some adjustments. >
Thanks. I see I forgot to tweak a call in btree_gist - sorry about that. >> The bytea case is fixed by checking that the boundary values are >> varlenas. This seems better than checking for BYTEAOID explicitly, which >> would fail for custom varlena-based types. At first I've been thinking >> there might be issues when the data types has mismatching ordering, but >> I don't think the patch makes it any worse. > > I didn't like this one bit. It's unlike all the other cases, which accept > only specific type OIDs, and there's no good reason to assume that a > bytea-vs-something-else comparison operator would have bytea-like > semantics. So I think it's better to punt, pending the invention of an > API to let the operator supply its own convert_to_scalar logic. > OK, understood. That's the "mismatching ordering" I was wondering about. >> I've also added a bunch of regression tests, checking each case. The >> bytea test it should cause segfault on master, of course. > > I was kind of underwhelmed with these test cases, too, so I didn't > commit them. But they were good for proving that the bytea bug > wasn't hypothetical :-) Underwhelmed in what sense? Should the tests be constructed in some other way, or do you think it's something that doesn't need the tests? regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
