Andrew Dunstan <andrew@dunslane.net> writes:
> On 11/08/2014 09:26 AM, Robert Haas wrote:
>> I'm not sure whether this is safe enough to back-patch, but it seems
>> like we should probably plan to back-patch *something*, because the
>> status quo isn't great either.
> I confirm that Tom's patch does indeed fix my test case that produces
> empty field names.
> We should probably not backpatch it, as it is a behaviour change.
> However, I do think we should add checks in composite_to_json and
> hstore_from_record for empty field names, and error out if they are
> found.
That seems like a pretty silly move: it wouldn't actually fix anything,
and it would break cases that perhaps are acceptable to users today.
We could reduce the risks involved by narrowing the cases in which
ExecEvalWholeRowVar will replace field names it got from the input.
I'd be inclined to propose:
1. If Var is of a named composite type, use *exactly* the field names
associated with that type. (This avoids the need to possibly produce
RECORD outputs from a named-type Var, thus removing the Assert-weakening
issue.)
2. If Var is of type RECORD, replace only empty field names with aliases
from the RTE. (This might sound inconsistent --- could you end up with
some names coming from point A and some from point B? --- but in practice
I think it would always be all-or-nothing, because the issue is whether
or not the planner bothered to attach column names to a lower-level
targetlist.)
regards, tom lane
