mogrify and indent features for jsonb
Attached is a patch to provide a number of very useful facilities to
jsonb that people have asked for. These are based on work by Dmitry
Dolgov in his jsonbx extension, but I take responsibility for any bugs.
The facilities are:
new operations:
concatenation: jsonb || jsonb -> jsonb
deletion: jsonb - text -> jsonb
deletion: jsonb - int -> text
new functions:
produce indented text: jsonb_indent(jsonb) -> text
change an element at a path: jsonb_replace(jsonb, text[], jsonb) -> jsonb.
It would be relatively trivial to add:
delete an element at a path: jsonb_delete(jsonb, text[]) -> json
and I think we should do that for the sake of completeness.
The docs might need a little extra work, and the indent code definitely
needs work, which I hope to complete in the next day or two, but I
wanted to put a stake in the ground.
cheers
andrew
Attachments:
jsonbxcore1.patchtext/x-patch; name=jsonbxcore1.patchDownload+1589-13
For jsonb_indent, how about having it match up closer to the
JavaScript JSON.stringify(value, replacer, space)[1]https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify? That way a user
can specify the indentation level and optionally filter the fields
they'd like to output.
In JS, the "replacer" parameter can be either a JS function or an
array of property names. I don't think the former is really possible
(in a SQL callable function) but the latter would be a text[]. The
"space" parameter can be either a string (used directly) or a number
(corresponding number of spaces).
The PG function signatures would be something like:
CREATE OR REPLACE FUNCTION jsonb_stringify(obj jsonb, replacer text[],
space text)
CREATE OR REPLACE FUNCTION jsonb_stringify(obj jsonb, replacer text[],
space int)
For convenience we could also include overloads with replacer removed
(since most people probably want the entire object):
CREATE OR REPLACE FUNCTION jsonb_stringify(obj jsonb, space text)
CREATE OR REPLACE FUNCTION jsonb_stringify(obj jsonb, space int)
Having json_stringify versions of these would be useful as well.
Regards,
-- Sehrope Sarkuni
[1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/15/2015 11:47 AM, Sehrope Sarkuni wrote:
For jsonb_indent, how about having it match up closer to the
JavaScript JSON.stringify(value, replacer, space)[1]? That way a user
can specify the indentation level and optionally filter the fields
they'd like to output.In JS, the "replacer" parameter can be either a JS function or an
array of property names. I don't think the former is really possible
(in a SQL callable function) but the latter would be a text[]. The
"space" parameter can be either a string (used directly) or a number
(corresponding number of spaces).The PG function signatures would be something like:
CREATE OR REPLACE FUNCTION jsonb_stringify(obj jsonb, replacer text[],
space text)
CREATE OR REPLACE FUNCTION jsonb_stringify(obj jsonb, replacer text[],
space int)For convenience we could also include overloads with replacer removed
(since most people probably want the entire object):CREATE OR REPLACE FUNCTION jsonb_stringify(obj jsonb, space text)
CREATE OR REPLACE FUNCTION jsonb_stringify(obj jsonb, space int)Having json_stringify versions of these would be useful as well.
I think if you want these things, especially the filtering, you should
probably load PLV8.
We could probably do the rest, but I'm not sure it's worth doing given
that PLV8 is available for all of it.
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
On 02/14/2015 10:06 PM, Andrew Dunstan wrote:
Attached is a patch to provide a number of very useful facilities to
jsonb that people have asked for. These are based on work by Dmitry
Dolgov in his jsonbx extension, but I take responsibility for any bugs.The facilities are:
new operations:
concatenation: jsonb || jsonb -> jsonb
deletion: jsonb - text -> jsonb
deletion: jsonb - int -> textnew functions:
produce indented text: jsonb_indent(jsonb) -> text
change an element at a path: jsonb_replace(jsonb, text[], jsonb) ->
jsonb.It would be relatively trivial to add:
delete an element at a path: jsonb_delete(jsonb, text[]) -> json
and I think we should do that for the sake of completeness.
The docs might need a little extra work, and the indent code
definitely needs work, which I hope to complete in the next day or
two, but I wanted to put a stake in the ground.
In this version the indent code now works correctly, and there is an
additional delete operator:
jsonb - text[] -> jsonb
Which deletes data at the designated path.
cheers
andrew
Attachments:
jsonbxcore2.patchtext/x-patch; name=jsonbxcore2.patchDownload+1791-14
Hi,
I looked at the patch and have several comments.
First let me say that modifying the individual paths of the json is the
feature I miss the most in the current implementation so I am happy that
this patch was submitted.
I would prefer slightly the patch split into two parts, one for the
indent printing and one with the manipulation functions, but it's not
too big patch so it's not too bad as it is.
There is one compiler warning that I see:
jsonfuncs.c:3533:1: warning: no previous prototype for
‘jsonb_delete_path’ [-Wmissing-prototypes]
jsonb_delete_path(PG_FUNCTION_ARGS)
I think it would be better if the ident printing didn't put the start of
array ([) and start of dictionary ({) on separate line since most
"pretty" printing implementations outside of Postgres (like the
JSON.stringify or python's json module) don't do that. This is purely
about consistency with the rest of the world.
The json_ident might be better named as json_pretty perhaps?
I don't really understand the point of h_atoi() function. What's wrong
with using strtol like pg_atoi does? Also there is no integer overflow
check anywhere in that function.
There is tons of end of line whitespace mess in jsonb_indent docs.
Otherwise everything I tried so far works as expected. The code looks ok
as well except maybe the replacePath could use couple of comments (for
example the line which uses the h_atoi) to make it easier to follow.
About the:
+ /* XXX : why do we need this assertion? The functions is declared to take text[] */ + Assert(ARR_ELEMTYPE(path) == TEXTOID);
I wonder about this also, some functions do that, some don't, I am not
really sure what is the rule there myself.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi, Petr, thanks for the review.
I think it would be better if the ident printing didn't put the start
of array ([) and start of dictionary ({) on separate line
Did you mean this?
[
{
"a": 1,
"b": 2
}
]
I tried to verify this in several ways (http://jsonprettyprint.com/,
"JSON.stringify", "json.too" from python), and I always get this result
(new line after ([) and ({) ).
I don't really understand the point of h_atoi() function.
Initially, this function was to combine the convertion logic and specific
verifications. But I agree, "strtol" is more correct way, I should improve
this.
The code looks ok as well except maybe the replacePath could use couple
of comments
I already added several commentaries (and looks like I should add even more
in the nearest future) for this function in the jsonbx extension, and I
think we can update this patch one more time with that improvement.
About the Assert(ARR_ELEMTYPE(path) == TEXTOID);
I based my work on the hstore extension, which contains such kind of
assertions. But I suppose, it's not required anymore, so I removed this
from the extension. And, I think, we can also remove this from patch.
On 18 February 2015 at 08:32, Petr Jelinek <petr@2ndquadrant.com> wrote:
Show quoted text
Hi,
I looked at the patch and have several comments.
First let me say that modifying the individual paths of the json is the
feature I miss the most in the current implementation so I am happy that
this patch was submitted.I would prefer slightly the patch split into two parts, one for the indent
printing and one with the manipulation functions, but it's not too big
patch so it's not too bad as it is.There is one compiler warning that I see:
jsonfuncs.c:3533:1: warning: no previous prototype for ‘jsonb_delete_path’
[-Wmissing-prototypes]
jsonb_delete_path(PG_FUNCTION_ARGS)I think it would be better if the ident printing didn't put the start of
array ([) and start of dictionary ({) on separate line since most "pretty"
printing implementations outside of Postgres (like the JSON.stringify or
python's json module) don't do that. This is purely about consistency with
the rest of the world.The json_ident might be better named as json_pretty perhaps?
I don't really understand the point of h_atoi() function. What's wrong
with using strtol like pg_atoi does? Also there is no integer overflow
check anywhere in that function.There is tons of end of line whitespace mess in jsonb_indent docs.
Otherwise everything I tried so far works as expected. The code looks ok
as well except maybe the replacePath could use couple of comments (for
example the line which uses the h_atoi) to make it easier to follow.About the:
+ /* XXX : why do we need this assertion? The functions is declared to take text[] */ + Assert(ARR_ELEMTYPE(path) == TEXTOID);I wonder about this also, some functions do that, some don't, I am not
really sure what is the rule there myself.--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 15 February 2015 at 03:06, Andrew Dunstan <andrew@dunslane.net> wrote:
Attached is a patch to provide a number of very useful facilities to jsonb
that people have asked for. These are based on work by Dmitry Dolgov in his
jsonbx extension, but I take responsibility for any bugs.The facilities are:
new operations:
concatenation: jsonb || jsonb -> jsonb
deletion: jsonb - text -> jsonb
deletion: jsonb - int -> textnew functions:
produce indented text: jsonb_indent(jsonb) -> text
change an element at a path: jsonb_replace(jsonb, text[], jsonb) -> jsonb.It would be relatively trivial to add:
delete an element at a path: jsonb_delete(jsonb, text[]) -> json
Would this support deleting "type" and the value 'dd' from the following?:
{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}
and I think we should do that for the sake of completeness.
The docs might need a little extra work, and the indent code definitely
needs work, which I hope to complete in the next day or two, but I wanted
to put a stake in the ground.
This is high on my wanted list, so thanks for working on this.
Seems to work well for me with a few tests.
Is there a way to take the json:
'{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'
and add "ee" to "d" without replacing it? I can think of ways of currently
doing it, but it's very convoluted just for pushing a value to an array.
Also, are there any plans to support the following?:
jsonb - text[] # Provide list of keys to delete in array
jsonb - jsonb # Deduplicate key:value pairs
jsonb && jsonb # Return overlapping jsonb (opposite of jsonb - jsonb)
Thanks
Thom
Is there a way to take the json:
'{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'and add "ee" to "d" without replacing it? I can think of ways of
currently doing it, but it's very convoluted just for pushing a value to
an array.
Can you think of a reasonable syntax for doing that via operators? I
can imagine that as a json_path function, i.e.:
jsonb_add_to_path(jsonb, text[], jsonb)
or where the end of the path is an array:
jsonb_add_to_path(jsonb, text[], text|int|float|bool)
But I simply can't imagine an operator syntax which would make it clear
what the user intended.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: WMd3df425d9e7bba677d80b648119c376c20a6fbbd59c9f889f08e1e7c3a512592ed131928b56e350bf36e099199613f95@asav-2.01.com
On 24 February 2015 at 19:16, Josh Berkus <josh@agliodbs.com> wrote:
Is there a way to take the json:
'{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'and add "ee" to "d" without replacing it? I can think of ways of
currently doing it, but it's very convoluted just for pushing a value to
an array.Can you think of a reasonable syntax for doing that via operators? I
can imagine that as a json_path function, i.e.:jsonb_add_to_path(jsonb, text[], jsonb)
or where the end of the path is an array:
jsonb_add_to_path(jsonb, text[], text|int|float|bool)
But I simply can't imagine an operator syntax which would make it clear
what the user intended.
No, there probably isn't a sane operator syntax for such an operation. A
function would be nice. I'd just want to avoid hacking away at arrays by
exploding them, adding a value then re-arraying them and replacing the
value.
--
Thom
On 02/25/2015 03:13 AM, Thom Brown wrote:
Can you think of a reasonable syntax for doing that via operators? I
can imagine that as a json_path function, i.e.:jsonb_add_to_path(jsonb, text[], jsonb)
or where the end of the path is an array:
jsonb_add_to_path(jsonb, text[], text|int|float|bool)
But I simply can't imagine an operator syntax which would make it clear
what the user intended.No, there probably isn't a sane operator syntax for such an operation.
A function would be nice. I'd just want to avoid hacking away at arrays
by exploding them, adding a value then re-arraying them and replacing
the value.
Well, anyway, that doesn't seem like a reason to block the patch.
Rather, it's a reason to create another one for 9.6 ...
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: WM8f15b83959db2cbb25d68a1f42e95e785536ecd2ecd22e69ef0ec6f7f49e5e3595f697f87dbe204f9a27a7e7ec196acb@asav-2.01.com
Hi, Thom.
Would this support deleting "type" and the value 'dd'
With this patch you can delete them one by one:
select '{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'::jsonb - '{c, type}'::text[] - '{d, -1}'::text[];
?column?
-------------------------------------------------------------------
{"a": 1, "b": 2, "c": {"stuff": "test"}, "d": ["aa", "bb", "cc"]}
(1 row)
Is there a way to take the json:
'{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'
and add "ee" to "d" without replacing it?
No, looks like there is no way to add a new element to array with help of
this patch. I suppose this feature can be implemented easy enough inside
the "jsonb_concat" function:
select '{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'::jsonb || '{"d": ["ee"]}'::jsonb
but I'm not sure, that it will be the best way.
On 26 February 2015 at 01:13, Josh Berkus <josh@agliodbs.com> wrote:
Show quoted text
On 02/25/2015 03:13 AM, Thom Brown wrote:
Can you think of a reasonable syntax for doing that via operators? I
can imagine that as a json_path function, i.e.:jsonb_add_to_path(jsonb, text[], jsonb)
or where the end of the path is an array:
jsonb_add_to_path(jsonb, text[], text|int|float|bool)
But I simply can't imagine an operator syntax which would make it
clear
what the user intended.
No, there probably isn't a sane operator syntax for such an operation.
A function would be nice. I'd just want to avoid hacking away at arrays
by exploding them, adding a value then re-arraying them and replacing
the value.Well, anyway, that doesn't seem like a reason to block the patch.
Rather, it's a reason to create another one for 9.6 ...--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 26 February 2015 at 15:09, Dmitry Dolgov <9erthalion6@gmail.com> wrote:
Hi, Thom.
Would this support deleting "type" and the value 'dd'
With this patch you can delete them one by one:
select '{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'::jsonb - '{c, type}'::text[] - '{d, -1}'::text[];
?column?
-------------------------------------------------------------------
{"a": 1, "b": 2, "c": {"stuff": "test"}, "d": ["aa", "bb", "cc"]}
(1 row)
Doesn't work for me:
# select '{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'::jsonb - '{c, type}'::text[] - '{d, -1}'::text[];
ERROR: operator does not exist: jsonb - text[]
LINE 1: ...ff": "test"}, "d": ["aa","bb","cc","dd"]}'::jsonb - '{c, typ...
^
HINT: No operator matches the given name and argument type(s). You might
need to add explicit type casts.
Is there a way to take the json:
'{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":["aa","bb","cc","dd"]}'
and add "ee" to "d" without replacing it?
No, looks like there is no way to add a new element to array with help of
this patch. I suppose this feature can be implemented easy enough inside
the "jsonb_concat" function:select '{"a": 1, "b": 2, "c": {"type": "json", "stuff": "test"}, "d":
["aa","bb","cc","dd"]}'::jsonb || '{"d": ["ee"]}'::jsonbbut I'm not sure, that it will be the best way.
Yeah, I think that may be problematic. I agree with Josh that there's
probably no sane mix of operators for this, as I would expect your example
to replace "d": ["aa","bb","cc","dd"] with "d": ["ee"] rather than append
to it. Hmm... unless we used a + operator, but then I'm not sure what
should happen in the instance of '{"d": ["aa"]}' + '{"d": "bb"}'.
--
Thom
On 02/26/2015 07:25 AM, Thom Brown wrote:
Yeah, I think that may be problematic. I agree with Josh that there's
probably no sane mix of operators for this, as I would expect your
example to replace "d": ["aa","bb","cc","dd"] with "d": ["ee"] rather
than append to it. Hmm... unless we used a + operator, but then I'm not
sure what should happen in the instance of '{"d": ["aa"]}' + '{"d": "bb"}'.
Yeah, that's what I would expect too. In fact, I could would count on
replacement.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: WM03420583edddfade6d0fa5484f92bc8e035c7cc74dcca9d7f9d6936ad63d6370d5aa66bb9c1226491f16dafb43d19f1f@asav-2.01.com
On 23/02/15 18:15, Dmitry Dolgov wrote:
Hi, Petr, thanks for the review.
I think it would be better if the ident printing didn't put the
start of array ([) and start of dictionary ({) on separate line
Did you mean this?[
{
"a": 1,
"b": 2
}
]I tried to verify this in several ways (http://jsonprettyprint.com/,
"JSON.stringify", "json.too" from python), and I always get this result
(new line after ([) and ({) ).
No, I mean new lines before the ([) and ({) - try pretty printing
something like '{"a":["b", "c"], "d": {"e":"f"}}' using that tool you
pasted and see what your patch outputs to see what I mean.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 03/01/2015 05:03 AM, Petr Jelinek wrote:
On 23/02/15 18:15, Dmitry Dolgov wrote:
Hi, Petr, thanks for the review.
I think it would be better if the ident printing didn't put the
start of array ([) and start of dictionary ({) on separate line
Did you mean this?[
{
"a": 1,
"b": 2
}
]I tried to verify this in several ways (http://jsonprettyprint.com/,
"JSON.stringify", "json.too" from python), and I always get this result
(new line after ([) and ({) ).No, I mean new lines before the ([) and ({) - try pretty printing
something like '{"a":["b", "c"], "d": {"e":"f"}}' using that tool you
pasted and see what your patch outputs to see what I mean.
Please try this patch and see if it's doing what you want.
cheers
andrew
Attachments:
jsonbxcore3.patchtext/x-patch; name=jsonbxcore3.patchDownload+1868-16
On Sat, 14 Feb 2015 22:06:07 -0500
Andrew Dunstan <andrew@dunslane.net> wrote:
Hello.
I have function with recursive merging objects:
# SELECT jsonb_deep_extend('{"a": {"b": 6}}'::jsonb, '{"a": {"c":
7}}'::jsonb) AS new_jsonb;
new_jsonb
-------------------------
{"a": {"b": 6, "c": 7}}
https://github.com/koctep/jsonb-extend
I think this function may be useful for people too.
Could you add it to your patch? I don't have enough time to prepare
patch.
Attached is a patch to provide a number of very useful facilities to
jsonb that people have asked for. These are based on work by Dmitry
Dolgov in his jsonbx extension, but I take responsibility for any
bugs.The facilities are:
new operations:
concatenation: jsonb || jsonb -> jsonb
deletion: jsonb - text -> jsonb
deletion: jsonb - int -> textnew functions:
produce indented text: jsonb_indent(jsonb) -> text
change an element at a path: jsonb_replace(jsonb, text[], jsonb) ->
jsonb.It would be relatively trivial to add:
delete an element at a path: jsonb_delete(jsonb, text[]) -> json
and I think we should do that for the sake of completeness.
The docs might need a little extra work, and the indent code
definitely needs work, which I hope to complete in the next day or
two, but I wanted to put a stake in the ground.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
On 03/11/2015 04:05 AM, Ilya Ashchepkov wrote:
On Sat, 14 Feb 2015 22:06:07 -0500
Andrew Dunstan <andrew@dunslane.net> wrote:Hello.
I have function with recursive merging objects:
# SELECT jsonb_deep_extend('{"a": {"b": 6}}'::jsonb, '{"a": {"c":
7}}'::jsonb) AS new_jsonb;
new_jsonb
-------------------------
{"a": {"b": 6, "c": 7}}https://github.com/koctep/jsonb-extend
I think this function may be useful for people too.
Could you add it to your patch? I don't have enough time to prepare
patch.
It's far too late to be adding new material.
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
On 01/03/15 16:49, Andrew Dunstan wrote:
On 03/01/2015 05:03 AM, Petr Jelinek wrote:
On 23/02/15 18:15, Dmitry Dolgov wrote:
Hi, Petr, thanks for the review.
I think it would be better if the ident printing didn't put the
start of array ([) and start of dictionary ({) on separate line
Did you mean this?[
{
"a": 1,
"b": 2
}
]I tried to verify this in several ways (http://jsonprettyprint.com/,
"JSON.stringify", "json.too" from python), and I always get this result
(new line after ([) and ({) ).No, I mean new lines before the ([) and ({) - try pretty printing
something like '{"a":["b", "c"], "d": {"e":"f"}}' using that tool you
pasted and see what your patch outputs to see what I mean.Please try this patch and see if it's doing what you want.
Yes, this produces output that looks like what javascript/python produce.
(the other stuff I commented about, mainly the h_atoi is still unfixed
though)
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
On 13 March 2015 at 23:30, Petr Jelinek <petr@2ndquadrant.com> wrote:
Show quoted text
On 01/03/15 16:49, Andrew Dunstan wrote:
On 03/01/2015 05:03 AM, Petr Jelinek wrote:
On 23/02/15 18:15, Dmitry Dolgov wrote:
Hi, Petr, thanks for the review.
I think it would be better if the ident printing didn't put the
start of array ([) and start of dictionary ({) on separate line
Did you mean this?[
{
"a": 1,
"b": 2
}
]I tried to verify this in several ways (http://jsonprettyprint.com/,
"JSON.stringify", "json.too" from python), and I always get this result
(new line after ([) and ({) ).No, I mean new lines before the ([) and ({) - try pretty printing
something like '{"a":["b", "c"], "d": {"e":"f"}}' using that tool you
pasted and see what your patch outputs to see what I mean.Please try this patch and see if it's doing what you want.
Yes, this produces output that looks like what javascript/python produce.
(the other stuff I commented about, mainly the h_atoi is still unfixed
though)--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
jsonbxcore4.patchapplication/octet-stream; name=jsonbxcore4.patchDownload+1835-16
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.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers