From: | Petr Jelinek <petr(at)2ndquadrant(dot)com> |
---|---|
To: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: mogrify and indent features for jsonb |
Date: | 2015-04-29 22:58:58 |
Message-ID: | 55416232.7050701@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 27/04/15 18:46, Petr Jelinek wrote:
> On 18/04/15 20:35, Dmitry Dolgov wrote:
>> Sorry for late reply. Here is a slightly improved version of the patch
>> with the new `h_atoi` function, I hope this implementation will be more
>> appropriate.
>>
>
> It's better, but a) I don't like the name of the function b) I don't see
> why we need the function at all given that it's called from only one
> place and is effectively couple lines of code.
>
> In general I wonder if it wouldn't be better to split the replacePath
> into 3 functions, one replacePath, one replacePathObject,
> replacePathArray and call those Object/Array ones from the replacePath
> and inlining the h_atoi code into the Array one. If this was done then
> it would make also sense to change the main if/else in replacePath into
> a switch.
>
> Another thing I noticed now when reading the code again - you should not
> be using braces when you only have one command in the code-block.
>
Hi,
I worked this over a bit (I hope Dmitry won't mind) and I am now more or
less happy with the patch. Here is list of changes I made:
- rebased to todays master (Oid conflicts, transforms patch conflicts)
- changed the main if/else if/else if/else to switch in replacePath
- split the replacePath into 3 functions (main one plus 2 helpers for
Object and Array)
- removed the h_atoi and use the strtol directly
- renamed jsonb_indent to jsonb_pretty because we use "pretty" for
similar use-case everywhere else
- fixed whitespace/brackets where needed
- added/reworded some comments and couple of lines in docs
I think it's ready for Andrew now.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachment | Content-Type | Size |
---|---|---|
0001-jsonbxcore5.patch | application/x-patch | 60.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | John Gorman | 2015-04-29 23:07:26 | Incompatible trig error handling |
Previous Message | Tomas Vondra | 2015-04-29 22:55:06 | Re: alternative compression algorithms? |