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 == NULL, wasn't it? Yes, J

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 open item. Andrew,

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: json

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 an error but jsonb functi

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 f

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, an

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 wrote:

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 writes: > > On 25.01.

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 commitf

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 loo

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 wrote: >> I think you need to take a seco

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 you're producing and realize that it's not so c

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 that it's not so clean either. This extract f

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 >>> populate_record_field, for example, is pr

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 introduce some helper mac

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 JsValueIsN

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 JsValue which contains >

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 all the bit-tests were hidde

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 Post

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 number of successive open

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 doesn'

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 quoted at the start of the thre

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 proce

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 a

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 json > DETAIL: Token "a"

[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: [master