On 06/23/2014 11:43 AM, Tom Lane wrote:
Michael Paquier <michael.paqu...@gmail.com> writes:
Digging more into that, I have found the issue and a fix for it. It happens
that populate_recordset_object_start, which is used to initialize the
process for the population of the record, is taken *each* time a json
object is found, re-creating every time the hash table for the parsing
process, hence removing from PopulateRecordsetState all the entries already
parsed and creating the problem reported by Matti. The fix I am proposing
to fix this issue is rather simple: simply bypass the creation of the hash
table if lex_level > 1 as we are in presence of a nested object and rely on
the existing hash table.
Yes, this code is clearly not handling the nested-objects case correctly.
I had written a fix more or less equivalent to yours last night.
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?
(IOW, we probably actually should have nested hash tables in this case.
I suspect that the current bug arose from incompletely-written logic
to do it like that.)
Since we've already decided to force an initdb for 9.4beta2, it's not
quite too late to revisit this API, and I think it needs revisiting.
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.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
