• Tom Lane's avatar
    Clean up after insufficiently-researched optimization of tuple conversions. · 3f902354
    Tom Lane authored
    tupconvert.c's functions formerly considered that an explicit tuple
    conversion was necessary if the input and output tupdescs contained
    different type OIDs.  The point of that was to make sure that a composite
    datum resulting from the conversion would contain the destination rowtype
    OID in its composite-datum header.  However, commit 3838074f entirely
    misunderstood what that check was for, thinking that it had something to do
    with presence or absence of an OID column within the tuple.  Removal of the
    check broke the no-op conversion path in ExecEvalConvertRowtype, as
    reported by Ashutosh Bapat.
    
    It turns out that of the dozen or so call sites for tupconvert.c functions,
    ExecEvalConvertRowtype is the only one that cares about the composite-datum
    header fields in the output tuple.  In all the rest, we'd much rather avoid
    an unnecessary conversion whenever the tuples are physically compatible.
    Moreover, the comments in tupconvert.c only promise physical compatibility
    not a metadata match.  So, let's accept the removal of the guarantee about
    the output tuple's rowtype marking, recognizing that this is a API change
    that could conceivably break third-party callers of tupconvert.c.  (So,
    let's remember to mention it in the v10 release notes.)
    
    However, commit 3838074f did have a bit of a point here, in that two
    tuples mustn't be considered physically compatible if one has HEAP_HASOID
    set and the other doesn't.  (Some of the callers of tupconvert.c might not
    really care about that, but we can't assume it in general.)  The previous
    check accidentally covered that issue, because no RECORD types ever have
    OIDs, while if two tupdescs have the same named composite type OID then,
    a fortiori, they have the same tdhasoid setting.  If we're removing the
    type OID match check then we'd better include tdhasoid match as part of
    the physical compatibility check.
    
    Without that hack in tupconvert.c, we need ExecEvalConvertRowtype to take
    responsibility for inserting the correct rowtype OID label whenever
    tupconvert.c decides it need not do anything.  This is easily done with
    heap_copy_tuple_as_datum, which will be considerably faster than a tuple
    disassembly and reassembly anyway; so from a performance standpoint this
    change is a win all around compared to what happened in earlier branches.
    It just means a couple more lines of code in ExecEvalConvertRowtype.
    
    Ashutosh Bapat and Tom Lane
    
    Discussion: https://postgr.es/m/CAFjFpRfvHABV6+oVvGcshF8rHn+1LfRUhj7Jz1CDZ4gPUwehBg@mail.gmail.com
    3f902354
execExprInterp.c 94.1 KB