Re: Expanded Objects and Order By

Paul Ramsey <pramsey(at)cleverelephant(dot)ca> writes:
> Thank the Maker, it is reproduceable: returning an expanded header in the _in function is not appreciated in a very narrow number of cases.

BTW, on further poking around: if you'd had RANDOMIZE_ALLOCATED_MEMORY
enabled, returning an expanded object from an input function would have
failed this test in stringTypeDatum():

#ifdef RANDOMIZE_ALLOCATED_MEMORY

/*
* For pass-by-reference data types, repeat the conversion to see if the
* input function leaves any uninitialized bytes in the result. We can
* only detect that reliably if RANDOMIZE_ALLOCATED_MEMORY is enabled, so
* we don't bother testing otherwise. The reason we don't want any
* instability in the input function is that comparison of Const nodes
* relies on bytewise comparison of the datums, so if the input function
* leaves garbage then subexpressions that should be identical may not get
* recognized as such. See pgsql-hackers discussion of 2008-04-04.
*/
if (string && !typform->typbyval)
{
Datum result2;

result2 = OidInputFunctionCall(typinput, string,
typioparam, atttypmod);
if (!datumIsEqual(result, result2, typform->typbyval, typform->typlen))
elog(WARNING, "type %s has unstable input conversion for \"%s\"",
NameStr(typform->typname), string);
}
#endif

The pointer values in the two objects could not be equal so datumIsEqual
would certainly fail. So one way of looking at this is that the case
indeed should be considered unsupported.

However, what I'm realizing from this discussion is that we need to have a
stronger convention on what we put into Const nodes. Repeatable results
from the input function are certainly necessary, but we *also* need to
forbid short-header, compressed, or externally toasted values. Otherwise
we will have Consts that aren't very Const (if the external value goes
away) or fail to compare equal to other Consts that they reasonably should
compare equal to.

Seen in that light, applying pg_detoast_datum() to all varlena values
that are going into Consts is sensible, and it'll take care of the
expanded-object variant of the issue as well.

I've not coded the fix yet, but it looks like the thing to do is to
put "if not-null and typlen==-1 then pg_detoast_datum()" logic into
makeConst() and coerce_type(), which so far as I can find are the only
places that build new Const nodes. Also, the above-quoted
repeatable-results check has to be applied *after* the detoast step,
else it'll still be a hazard for expanded objects. However, coerce_type()
is the only caller of stringTypeDatum(), so what we can do is move the
repeatable-results check over to coerce_type() and have it repeat
pg_detoast_datum() as well as the input-function call proper.

regards, tom lane