Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-12-04 18:48:44 -0500, Robert Haas wrote:
>> And record_image_eq does a rather elaborate dance around here, calling
>> the appropriate GET_x_BYTES macro depending on the type-width. If we
>> can really count on the high-order bits to be zero, that's all
>> completely unnecessary tomfoolery.
> I don't think we can get rid of that dance in record_image_eq - it very
> well could used on records originally generated when those bits haven't
> been guaranteed to be zeroed.
No, you're failing to think about the context here. A Datum is not an
on-disk concept, only an in-memory one. In the case of record_image_eq,
simplifying the comparison of by-value Datums is unconditionally safe
as long as heap_deform_tuple is consistent about using the proper
GetDatum macros, which it is. (So actually, that "elaborate dance"
is a 100% waste of effort today, regardless of any changes we're
discussing here.)
The risk we take by simplifying comparisons in a more general context
is that some function somewhere might've been sloppy about doing a
native-type-to-Datum conversion on its result. In the case of V0
functions that risk is unavoidable except by adding some explicit cleanup
code, but I'm a bit worried that somebody, particularly third-party code,
might've sloppily written "return foo" in a V1 function when "return
Int32GetDatum(foo)" would be correct. In that case, the resultant Datum
might have not-per-spec high-order bits, and if it reaches the fast
comparator without ever having been squeezed into a physical tuple,
we've got a problem.
I would certainly regard this as a bug in that function, but nonetheless
it's a hazard that we need to set against any performance improvement
that we can buy this way.
regards, tom lane