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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: matti(dot)hameister(at)technologygroup(dot)de, pgsql-bugs(at)postgresql(dot)org, pgsql-hackers(at)postgresql(dot)org
Subject: Re: BUG #10728: json_to_recordset with nested json objects NULLs columns
Date: 2014-06-23 15:43:00
Message-ID: 16858.1403538180@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Michael Paquier <michael(dot)paquier(at)gmail(dot)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.

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Merlin Moncure 2014-06-23 16:19:05 Re: BUG #10728: json_to_recordset with nested json objects NULLs columns
Previous Message Tomas Vondra 2014-06-23 14:31:50 Re: BUG #10734: PostgreSQL 9.3.4 shutdown forever in zfsonlinux 0.6.3-1 filesystem

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-06-23 15:52:49 Re: /proc/self/oom_adj is deprecated in newer Linux kernels
Previous Message Andres Freund 2014-06-23 15:42:28 Re: Wait free LW_SHARED acquisition - v0.2