Re: [HACKERS] BUG #10728: json_to_recordset with nested json objects NULLs columns

On 06/23/2014 09:43 PM, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> On 06/23/2014 07:34 PM, Tom Lane wrote:
>>> I'm not following your comment about 9.3. The json[b]_to_record[set]
>>> functions are new in 9.4, which is what makes me feel it's not too
>>> late to redefine their behavior. But changing behavior of stuff that
>>> was in 9.3 seems a lot more debatable.
>> This problem is also manifest in json_populate_recordset, which also
>> uses the function in question, and is in 9.3:
> Ah, I see the problem.
>
> Here is a first cut suggestion:
>
> * Get rid of the use_json_as_text flag argument for the new functions.
> In json_populate_record(set), ignore its value and deprecate using it.
> (The fact that it already had a default makes that easier.) The
> behavior should always be as below.
>
> * For nested json objects, we'll spit those out in json textual format,
> which means they'll successfully convert to either text or json/jsonb.
> Compared to the old behavior of json_populate_recordset, this just means
> that we don't throw an error anymore regardless of the flag value,
> which seems ok (though maybe not something to backpatch into 9.3).
>
> * Nested json arrays are a bit more problematic. What I'd ideally like
> is to spit them out in a form that would be successfully parsable as a SQL
> array of the appropriate element type. Unfortunately, I think that that
> ship has sailed because json_populate_recordset failed to do that in 9.3.
> What we should probably do is define this the same as the nested object
> case, ie, we spit it out in *json* array format, meaning you can insert it
> into a text or json/jsonb field of the result record. Maybe sometime in
> the future we can add a json-array-to-SQL-array converter function, but
> these functions won't do that.
>
> >From a user's standpoint this just boils down to (a) fix the bug with
> mishandling of the hash tables, and (b) get rid of the gratuitous
> error report.
>
>

The big problem is that we have been ignoring the result type when
constructing the hash, even though the info is available. There is some
sense in this in that the field might not even be present in the result
type. And it works except for structured types like records, arrays and
json. Even if we don't have a nested value, the functions will do the
wrong thing for a scalar string destined for a json field (it will be
de-escaped, when it should not be).

w.r.t. json arrays, I think you're chasing a chimera, since they are
heterogenous, unlike SQL arrays.

w.r.t. the use_json_as_text argument, yes, it has a default, but the
default is false. Ignoring it seems to be more than just deprecating it.
I agree it's a mess, though :-(

cheers

andrew