Re: Expanded Objects and Order By

I wrote:
> 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.

> Thanks for finding a test case! I'll check into it shortly.

So the short answer is that the planner assumes, not unreasonably,
that copying a parse node with copyObject() should produce a node
that's still equal() to the original. (The proximate cause of the
"could not find pathkey item to sort" error is failure to match
an ORDER BY expression to an equivalence-class item that was made
from it earlier.)

In the case here, coerce_type() produces a Const node whose constvalue
is pointing at the expanded array returned by array_in(). Nothing
further happens to that node until the planner tries to copy it ---
and copyObject will produce a flattened array value, cf datumCopy().
Even if copying made a new expanded object, that object would not be
equal() to the original node according to datumIsEqual's very
simplistic comparisons.

Now that I've seen this, I'm suspicious that we probably have a lot
of other related bugs --- for example, short-header and non-short-header
varlena values would also not be seen as equal().

What I'm thinking is that we had best apply pg_detoast_datum() to any
varlena value being put into a Const node. That will do nothing in the
normal case where the value already is a simple standard-header varlena
value, but will fix any cases where we might be trying to put a toasted,
expanded, etc value into a Const. Generally it'd be a bad idea to have a
toast reference in a Const anyhow; for example, there is code at the end
of evaluate_expr() that does the equivalent of this when making a Const
from the result of constant-simplifying an expression. That got put in
there as the result of hard experience; but now it seems we did not go
deep enough.

An alternative idea would be to make datumIsEqual smarter; but given the
other ways in which it's used I doubt we want it to be trying to do
detoasting.

regards, tom lane