On Tue, Jun 24, 2014 at 9:34 AM, Andrew Dunstan <and...@dunslane.net> wrote:
> > On 06/23/2014 07:34 PM, Tom Lane wrote: > >> Andrew Dunstan <and...@dunslane.net> writes: >> >>> On 06/23/2014 11:43 AM, Tom Lane wrote: >>> >>>> However, it seems to me that these functions (json[b]_to_record[set]) >>>> are >>>> handling the nested-json-objects case in a fairly brain-dead fashion to >>>> start with. I would like to propose that we should think about getting >>>> rid of the use_json_as_text flag arguments altogether. What purpose do >>>> they serve? If we're going to the trouble of parsing the nested JSON >>>> objects anyway, why don't we just reconstruct from that data? >>>> >>> Looks like we have some problems in this whole area, not just the new >>> function, so we need to fix 9.3 also :-( >>> IIRC, originally, the intention was to disallow nested json objects, but >>> the use_json_as_text was put in as a possibly less drastic possibility. >>> If we get rid of it our only recourse is to error out if we encounter >>> nested json. I was probably remiss in not considering the likelihood of >>> a json target field. >>> I currently don't have lots of time to devote to this, sadly, but >>> Michael's patch looks like a good minimal fix. >>> >> I can spend some time on it over the next couple of days. I take it you >> don't have a problem with the concept of doing recursive processing, >> as long as it doesn't add much complication? >> >> 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: > > andrew=# create type yyy as (a int, b json, c int, d int); > CREATE TYPE > andrew=# select * from json_populate_recordset(null::yyy, '[ > > {"a":2,"c":3,"b":{"z":4}, "d":6} > ] > ',true) x; > a | b | c | d > ---+---------+---+--- > > | {"z":4} | | 6 > (1 row) > Yeah, the somewhat-backpatchable patch I sent fixes that as well. I wouldn't mind writing a more complete patch with new regression tests and tutti-quanti for 9.3 and 9.4master, but it seems that Tom is already on it. -- Michael