Re: [HACKERS] PATCH: recursive json_populate_record()

2017-05-30 Thread Nikita Glukhov
On 30.05.2017 02:31, Tom Lane wrote: Nikita Glukhov writes: 2. Also I've found a some kind of thinko in JsGetObjectSize() macro, but it seems that this obvious mistake can not lead to incorrect behavior. Hm, I think it actually was wrong for the case of jsonb_cont ==

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-05-29 Thread Tom Lane
Noah Misch writes: > On Mon, May 22, 2017 at 10:19:37PM +0300, Nikita Glukhov wrote: >> Attached two small fixes for the previous committed patch: > [Action required within three days. This is a generic notification.] > The above-described topic is currently a PostgreSQL 10

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-05-29 Thread Noah Misch
On Mon, May 22, 2017 at 10:19:37PM +0300, Nikita Glukhov wrote: > Attached two small fixes for the previous committed patch: > > 1. I've noticed a difference in behavior between json_populate_record() > and jsonb_populate_record() if we are trying to populate record from a > non-JSON-object:

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-05-29 Thread Tom Lane
Nikita Glukhov writes: > Attached two small fixes for the previous committed patch: > 1. I've noticed a difference in behavior between json_populate_record() > and jsonb_populate_record() if we are trying to populate record from a > non-JSON-object: json function throws

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-05-22 Thread Nikita Glukhov
Attached two small fixes for the previous committed patch: 1. I've noticed a difference in behavior between json_populate_record() and jsonb_populate_record() if we are trying to populate record from a non-JSON-object: json function throws an error but jsonb function returns a record with NULL

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-04-06 Thread Andrew Dunstan
On 04/05/2017 07:24 PM, Andrew Dunstan wrote: > > OK, I have been through this and I think it's basically committable. I > am currently doing some cleanup, such as putting all the typedefs in one > place, fixing redundant comments and adding more comments, declaring all > the static functions,

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-04-05 Thread Andrew Dunstan
On 04/04/2017 05:18 PM, Andrew Dunstan wrote: > > On 04/03/2017 05:17 PM, Andres Freund wrote: >> On 2017-03-21 14:31:08 -0400, Andrew Dunstan wrote: >>> On 03/21/2017 01:37 PM, David Steele wrote: On 3/16/17 11:54 AM, David Steele wrote: > On 2/1/17 12:53 AM, Michael Paquier wrote:

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-04-04 Thread Andrew Dunstan
On 04/03/2017 05:17 PM, Andres Freund wrote: > On 2017-03-21 14:31:08 -0400, Andrew Dunstan wrote: >> >> On 03/21/2017 01:37 PM, David Steele wrote: >>> On 3/16/17 11:54 AM, David Steele wrote: On 2/1/17 12:53 AM, Michael Paquier wrote: > On Thu, Jan 26, 2017 at 6:49 AM, Tom Lane

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-04-03 Thread Andres Freund
On 2017-03-21 14:31:08 -0400, Andrew Dunstan wrote: > > > On 03/21/2017 01:37 PM, David Steele wrote: > > On 3/16/17 11:54 AM, David Steele wrote: > >> On 2/1/17 12:53 AM, Michael Paquier wrote: > >>> On Thu, Jan 26, 2017 at 6:49 AM, Tom Lane wrote: > Nikita Glukhov

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-03-22 Thread Nikita Glukhov
On 22.03.2017 00:26, David Steele wrote: On 3/21/17 2:31 PM, Andrew Dunstan wrote: On 03/21/2017 01:37 PM, David Steele wrote: >> This thread has been idle for months since Tom's review. The submission has been marked "Returned with Feedback". Please feel free to resubmit to a future

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-03-21 Thread David Steele
On 3/21/17 2:31 PM, Andrew Dunstan wrote: On 03/21/2017 01:37 PM, David Steele wrote: >> This thread has been idle for months since Tom's review. The submission has been marked "Returned with Feedback". Please feel free to resubmit to a future commitfest. Please revive. I am planning to

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-03-21 Thread Andrew Dunstan
On 03/21/2017 01:37 PM, David Steele wrote: > On 3/16/17 11:54 AM, David Steele wrote: >> On 2/1/17 12:53 AM, Michael Paquier wrote: >>> On Thu, Jan 26, 2017 at 6:49 AM, Tom Lane wrote: Nikita Glukhov writes: > On 25.01.2017 23:58, Tom Lane

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-03-21 Thread David Steele
On 3/16/17 11:54 AM, David Steele wrote: On 2/1/17 12:53 AM, Michael Paquier wrote: On Thu, Jan 26, 2017 at 6:49 AM, Tom Lane wrote: Nikita Glukhov writes: On 25.01.2017 23:58, Tom Lane wrote: I think you need to take a second look at the code

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-03-16 Thread David Steele
On 2/1/17 12:53 AM, Michael Paquier wrote: > On Thu, Jan 26, 2017 at 6:49 AM, Tom Lane wrote: >> Nikita Glukhov writes: >>> On 25.01.2017 23:58, Tom Lane wrote: I think you need to take a second look at the code you're producing and realize

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-31 Thread Michael Paquier
On Thu, Jan 26, 2017 at 6:49 AM, Tom Lane wrote: > Nikita Glukhov writes: >> On 25.01.2017 23:58, Tom Lane wrote: >>> I think you need to take a second look at the code you're producing >>> and realize that it's not so clean either. This extract from

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-25 Thread Tom Lane
Nikita Glukhov writes: > On 25.01.2017 23:58, Tom Lane wrote: >> I think you need to take a second look at the code you're producing >> and realize that it's not so clean either. This extract from >> populate_record_field, for example, is pretty horrid: > But what if we

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-25 Thread Nikita Glukhov
On 25.01.2017 23:58, Tom Lane wrote: I think you need to take a second look at the code you're producing and realize that it's not so clean either. This extract from populate_record_field, for example, is pretty horrid: But what if we introduce some helper macros like this: #define

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-25 Thread Tom Lane
Nikita Glukhov writes: > On 22.01.2017 21:58, Tom Lane wrote: >> 5. The business with having some arguments that are only for json and >> others that are only for jsonb, eg in populate_scalar(), also makes me >> itch. > I've refactored json/jsonb passing using new struct

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-25 Thread Tom Lane
Nikita Glukhov writes: > On 22.01.2017 21:58, Tom Lane wrote: >> If you want such macros I think it would be better to submit a separate >> cosmetic patch that tries to hide such bit-tests behind macros throughout >> the jsonb code. > I've attached that patch, but not

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-24 Thread Nikita Glukhov
On 22.01.2017 21:58, Tom Lane wrote: In the meantime, we are *not* going to have attndims be semantically significant in just this one function. Therefore, please remove this patch's dependence on attndims. Ok, I've removed patch's dependence on attndims. But I still believe that someday

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-22 Thread Tom Lane
Nikita Glukhov writes: > On 01/08/2017 09:52 PM, Tom Lane wrote: >> ... attndims is really syntactic sugar only and doesn't affect anything >> meaningful semantically. > I have fixed the first patch: when the number of dimensionsis unknown > we determine it simply by the

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-11 Thread Nikita Glukhov
On 01/08/2017 09:52 PM, Tom Lane wrote: The example you quoted at the start of the thread doesn't fail for me in HEAD, so I surmise that it's falling foul of some assertion you added in the 0001 patch, but if so I think that assertion is wrong. attndims is really syntactic sugar only and

Re: [HACKERS] PATCH: recursive json_populate_record()

2017-01-08 Thread Tom Lane
Nikita Glukhov writes: > [ 0001_recursive_json_populate_record_v02.patch ] > [ 0002_assign_ndims_to_record_function_result_types_v02.patch ] I do not see the point of the second one of these, and it adds no test case showing why it would be needed. The example you

Re: [HACKERS] PATCH: recursive json_populate_record()

2016-12-27 Thread Nikita Glukhov
> I've noticed that this patch is on CF and needs a reviewer so I decided > to take a look. Code looks good to me in general, it's well covered by > tests and passes `make check-world`. Thanks for your review. > However it would be nice to have a little more comments. In my opinion > every

Re: [HACKERS] PATCH: recursive json_populate_record()

2016-12-27 Thread Aleksander Alekseev
Hello. I've noticed that this patch is on CF and needs a reviewer so I decided to take a look. Code looks good to me in general, it's well covered by tests and passes `make check-world`. However it would be nice to have a little more comments. In my opinion every procedure have to have at least

Re: [HACKERS] PATCH: recursive json_populate_record()

2016-12-12 Thread Michael Paquier
On Tue, Dec 13, 2016 at 9:38 AM, Nikita Glukhov wrote: > It also fixes the following errors/inconsistencies caused by lost quoting of > string json values: > > [master]=# select * from json_to_record('{"js": "a"}') as rec(js json); > ERROR: invalid input syntax for type

[HACKERS] PATCH: recursive json_populate_record()

2016-12-12 Thread Nikita Glukhov
Hi. The first attached patch implements recursive processing of nested objects and arrays in json[b]_populate_record[set](), json[b]_to_record[set](). See regression tests for examples. It also fixes the following errors/inconsistencies caused by lost quoting of string json values: