jsonb and nested hstore

On 01/26/2014 05:42 PM, Andrew Dunstan wrote:

Here is the latest set of patches for nested hstore and jsonb.

Because it's so large I've broken this into two patches and compressed
them. The jsonb patch should work standalone. The nested hstore patch
depends on it.

All the jsonb functions now use the jsonb API - there is no more
turning jsonb into text and reparsing it.

At this stage I'm going to be starting cleanup on the jsonb code
(indentation, error messages, comments etc.) as well get getting up
some jsonb docs.

Here is an update of the jsonb part of this. Charges:

* there is now documentation for jsonb
* most uses of elog() in json_funcs.c are replaced by ereport().
* indentation fixes and other tidying.

No changes in functionality.

cheers

andrew

On Mon, Jan 27, 2014 at 9:43 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 01/26/2014 05:42 PM, Andrew Dunstan wrote:

Here is the latest set of patches for nested hstore and jsonb.

Because it's so large I've broken this into two patches and compressed
them. The jsonb patch should work standalone. The nested hstore patch
depends on it.

All the jsonb functions now use the jsonb API - there is no more turning
jsonb into text and reparsing it.

At this stage I'm going to be starting cleanup on the jsonb code
(indentation, error messages, comments etc.) as well get getting up some
jsonb docs.

Here is an update of the jsonb part of this. Charges:

* there is now documentation for jsonb
* most uses of elog() in json_funcs.c are replaced by ereport().
* indentation fixes and other tidying.

No changes in functionality.

Don't have time to fire it up this morning, but a quick scan of the
patch turned up a few minor things:

* see a comment typo, line 290 'jsonn':
* line 332: 'bogus input' -- is this up to error reporting standards?
How about "value 'x' must be one of array, object, numeric, string,
bool"?
* line 357: "jsonb's key could be only a string" prefer non
possessive: jsonb keys must be a string
* line 374, 389: ditto 332
* line 513: is panic appropriate here?
* line 599: ditto
* line 730: odd phrasing in comment, also commenting on this function
is a little light
* line 807: slightly prefer 'with respect to'
* line 888: another PANIC: these maybe correct, seems odd to halt
server on corrupted datum though*
* line 1150: hm, is the jsonb internal hash structure documented?
Aside: why didn't we use standard hash table (performance maybe)?
* line 1805-6: poor phrasing. How about: "it will order and make
unique the hash keys. Otherwise we believe that pushed keys are
ordered and unique. (Don't like verbed 'unqiue').
* line 1860: "no break here: "

merlin

--
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/28/2014 09:38 AM, Merlin Moncure wrote:

On Mon, Jan 27, 2014 at 9:43 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 01/26/2014 05:42 PM, Andrew Dunstan wrote:

Here is the latest set of patches for nested hstore and jsonb.

Because it's so large I've broken this into two patches and compressed
them. The jsonb patch should work standalone. The nested hstore patch
depends on it.

All the jsonb functions now use the jsonb API - there is no more turning
jsonb into text and reparsing it.

At this stage I'm going to be starting cleanup on the jsonb code
(indentation, error messages, comments etc.) as well get getting up some
jsonb docs.

Here is an update of the jsonb part of this. Charges:

* there is now documentation for jsonb
* most uses of elog() in json_funcs.c are replaced by ereport().
* indentation fixes and other tidying.

No changes in functionality.

Don't have time to fire it up this morning, but a quick scan of the
patch turned up a few minor things:

* see a comment typo, line 290 'jsonn':
* line 332: 'bogus input' -- is this up to error reporting standards?
How about "value 'x' must be one of array, object, numeric, string,
bool"?
* line 357: "jsonb's key could be only a string" prefer non
possessive: jsonb keys must be a string
* line 374, 389: ditto 332
* line 513: is panic appropriate here?
* line 599: ditto
* line 730: odd phrasing in comment, also commenting on this function
is a little light
* line 807: slightly prefer 'with respect to'
* line 888: another PANIC: these maybe correct, seems odd to halt
server on corrupted datum though*
* line 1150: hm, is the jsonb internal hash structure documented?
Aside: why didn't we use standard hash table (performance maybe)?
* line 1805-6: poor phrasing. How about: "it will order and make
unique the hash keys. Otherwise we believe that pushed keys are
ordered and unique. (Don't like verbed 'unqiue').
* line 1860: "no break here: "

Looks like this review is against jsonb-5, not jsonb-6.

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

Looks like this review is against jsonb-5, not jsonb-6.

oh yep -- shoot, sorry for the noise.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andrew Dunstan wrote:

<para>
+    There are two JSON data types: <type>json</type> and <type>jsonb</type>.
+    Both accept identical sets of values as input. The difference is primarily
+    a matter of efficiency. The <type>json</type> data type stores an exact
+    copy of the the input text, and the processing functions have to reparse
+    it to precess it, while the <type>jsonb</type> is stored in a decomposed
+    form that makes it slightly less efficient to input but very much faster
+    to process, since it never needs reparsing.
+   </para>

typo "precess"
duplicated word "of the the input"

+         </indexterm><indexterm>
+          <primary>jsonb_each</primary>
+         </indexterm><para><literal>json_each(json)</literal>
+         </para><para><literal>jsonb_each(jsonb)</literal>
+       </para></entry>

This SGML nesting is odd and hard to read. Please place opening tags in
separate lines (or at least not immediately following a closing tag). I
am not sure whether the mentions of jsonb_each vs. json_each there are
correct or typos. This also occurs in other places.

Expands the object in <replaceable>from_json</replaceable> to a row whose columns match
the record type defined by base. Conversion will be best
effort; columns in base with no corresponding key in <replaceable>from_json</replaceable>
-         will be left null. If a column is specified more than once, the last value is used.
+         will be left null. When processing <type>json</type>, if a column is 
+         specified more than once, the last value is used.

Maybe you also need to specify what happens with jsonb?

diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
new file mode 100644
index 0000000..107ebf0
--- /dev/null
+++ b/src/backend/utils/adt/jsonb.c
@@ -0,0 +1,544 @@
+/*-------------------------------------------------------------------------
+ *
+ * jsonb.c
+ *		I/O for jsonb type
+ *
+ * Portions Copyright (c) 1996-2013, PostgreSQL Global Development Group

2014. Why "Portions", if we don't attribute any portion to UCB?

+ * NOTE. JSONB type is designed to be binary compatible with hstore.
+ *
+ * src/backend/utils/adt/jsonb_support.c

Typo'ed name here.

+#include "postgres.h"

+

+#include "libpq/pqformat.h"
+#include "utils/builtins.h"
+#include "utils/json.h"
+#include "utils/jsonapi.h"
+#include "utils/jsonb.h"

Misplaced prototype?

+static void recvJsonb(StringInfo buf, JsonbValue *v, uint32 level, uint32 header);

Not sure about the jsonb_1.out file. Is that only due to encoding
differences? What happens if you run it in a completely different
encoding than whatever you tested with? (I would assume Latin-9 and
UTF8) If it fails, then I think you'll end up ripping those tests out,
so probably the _1.out file will have no value at all.

I also wonder if it'd be better to have one large .sql file that
produces the same output in all platforms that tests most of the common
stuff, so that tests that changes output in different platforms can have
smaller alternative expected files.

--
�lvaro Herrera 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 01/28/2014 10:50 AM, Alvaro Herrera wrote:

+         </indexterm><indexterm>
+          <primary>jsonb_each</primary>
+         </indexterm><para><literal>json_each(json)</literal>
+         </para><para><literal>jsonb_each(jsonb)</literal>
+       </para></entry>

This SGML nesting is odd and hard to read. Please place opening tags in
separate lines (or at least not immediately following a closing tag). I
am not sure whether the mentions of jsonb_each vs. json_each there are
correct or typos. This also occurs in other places.

As I understand it, an <entry> tag can only contain block-level elements
like <para> if there are no inline elements (including white space).

If that's not correct I'll change it, but that's what I read here:
<http://oreilly.com/openbook/docbook/book/entry.html&gt;

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

Andrew Dunstan <andrew@dunslane.net> writes:

On 01/28/2014 10:50 AM, Alvaro Herrera wrote:
+         </indexterm><indexterm>
+          <primary>jsonb_each</primary>
+         </indexterm><para><literal>json_each(json)</literal>
+         </para><para><literal>jsonb_each(jsonb)</literal>
+       </para></entry>

This SGML nesting is odd and hard to read. Please place opening tags in
separate lines (or at least not immediately following a closing tag). I
am not sure whether the mentions of jsonb_each vs. json_each there are
correct or typos. This also occurs in other places.

As I understand it, an <entry> tag can only contain block-level elements
like <para> if there are no inline elements (including white space).

Practically every existing use of <indexterm> is freer than this in its
use of whitespace. It sounds to me like maybe you are trying to put the
<indexterm> inside something it shouldn't go inside of.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 01/28/2014 10:50 AM, Alvaro Herrera wrote:
+         </indexterm><indexterm>
+          <primary>jsonb_each</primary>
+         </indexterm><para><literal>json_each(json)</literal>
+         </para><para><literal>jsonb_each(jsonb)</literal>
+       </para></entry>

This SGML nesting is odd and hard to read. Please place opening tags in
separate lines (or at least not immediately following a closing tag). I
am not sure whether the mentions of jsonb_each vs. json_each there are
correct or typos. This also occurs in other places.

As I understand it, an <entry> tag can only contain block-level elements
like <para> if there are no inline elements (including white space).

Practically every existing use of <indexterm> is freer than this in its
use of whitespace. It sounds to me like maybe you are trying to put the
<indexterm> inside something it shouldn't go inside of.

FWIW I was just talking about formatting of the SGML source so that it
is easier to read.

--
�lvaro Herrera 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 01/28/2014 11:09 AM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 01/28/2014 10:50 AM, Alvaro Herrera wrote:
+         </indexterm><indexterm>
+          <primary>jsonb_each</primary>
+         </indexterm><para><literal>json_each(json)</literal>
+         </para><para><literal>jsonb_each(jsonb)</literal>
+       </para></entry>

This SGML nesting is odd and hard to read. Please place opening tags in
separate lines (or at least not immediately following a closing tag). I
am not sure whether the mentions of jsonb_each vs. json_each there are
correct or typos. This also occurs in other places.

As I understand it, an <entry> tag can only contain block-level elements
like <para> if there are no inline elements (including white space).

Practically every existing use of <indexterm> is freer than this in its
use of whitespace. It sounds to me like maybe you are trying to put the
<indexterm> inside something it shouldn't go inside of.

The problem is not the indexterm element, it's the space that might
exist outside it. Are we using block level elements like <para> inside
entry elements anywhere else? If not, then your observation is not
relevant. If there are no block level elements then AIUI we can space
things out how we like inside the entry element.

If you can show me how else legally to get a line break inside an entry
element I'm very interested. I tried several things before I found this
way of making it work.

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

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Tom Lane wrote:

Practically every existing use of <indexterm> is freer than this in its
use of whitespace. It sounds to me like maybe you are trying to put the
<indexterm> inside something it shouldn't go inside of.

FWIW I was just talking about formatting of the SGML source so that it
is easier to read.

Yeah, me too. I'm just suggesting that maybe Andrew needs to move the
indexterm so that he can format it more readably.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andrew Dunstan <andrew@dunslane.net> writes:

The problem is not the indexterm element, it's the space that might
exist outside it. Are we using block level elements like <para> inside
entry elements anywhere else?

Probably not, and I wonder why you're trying to. Whole paras inside
a table entry (this is a table no?) don't sound like they are going
to lead to nice-looking results.

regards, tom lane

--
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/28/2014 11:27 AM, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Tom Lane wrote:

Practically every existing use of <indexterm> is freer than this in its
use of whitespace. It sounds to me like maybe you are trying to put the
<indexterm> inside something it shouldn't go inside of.

FWIW I was just talking about formatting of the SGML source so that it
is easier to read.

Yeah, me too. I'm just suggesting that maybe Andrew needs to move the
indexterm so that he can format it more readably.

Hmm. Maybe I could put them inside the para elements. So we'd have:

<entry><para>
<indexterm>
</indexterm>
para text
</para><para>
<indexterm>
</indexterm>
para text
</para></entry>

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/28/2014 11:29 AM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

The problem is not the indexterm element, it's the space that might
exist outside it. Are we using block level elements like <para> inside
entry elements anywhere else?

Probably not, and I wonder why you're trying to. Whole paras inside
a table entry (this is a table no?) don't sound like they are going
to lead to nice-looking results.

See <http://developer.postgresql.org/~adunstan/functions-json.html&gt;

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 Tue, Jan 28, 2014 at 10:46 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 01/28/2014 11:29 AM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

The problem is not the indexterm element, it's the space that might
exist outside it. Are we using block level elements like <para> inside
entry elements anywhere else?

Probably not, and I wonder why you're trying to. Whole paras inside
a table entry (this is a table no?) don't sound like they are going
to lead to nice-looking results.

See <http://developer.postgresql.org/~adunstan/functions-json.html&gt;

yeah. note: I think the json documentation needs *major* overhaul. too
much is going in inside the function listings where there really
should be a big breakout discussing the "big picture" of json/jsonb
with examples of various use cases. I want to give it a shot but
unfortunately can not commit to do that by the end of the 'fest.

merlin

--
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/28/2014 09:58 AM, Merlin Moncure wrote:

yeah. note: I think the json documentation needs *major* overhaul. too
much is going in inside the function listings where there really
should be a big breakout discussing the "big picture" of json/jsonb
with examples of various use cases. I want to give it a shot but
unfortunately can not commit to do that by the end of the 'fest.

FWIW, I've promised Andrew that I'll overhaul this by the end of beta.
Given that we have all of beta for doc refinements.

In addition to this, the JSON vs JSONB datatype page really needs
expansion and clarification.

--
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 Tue, Jan 28, 2014 at 12:09 PM, Josh Berkus <josh@agliodbs.com> wrote:

On 01/28/2014 09:58 AM, Merlin Moncure wrote:

yeah. note: I think the json documentation needs *major* overhaul. too
much is going in inside the function listings where there really
should be a big breakout discussing the "big picture" of json/jsonb
with examples of various use cases. I want to give it a shot but
unfortunately can not commit to do that by the end of the 'fest.

FWIW, I've promised Andrew that I'll overhaul this by the end of beta.
Given that we have all of beta for doc refinements.

In addition to this, the JSON vs JSONB datatype page really needs
expansion and clarification.

right: exactly. I'd be happy to help (such as I can) ...I wanted to
see if jsonb to make it in on this 'fest (doc issues notwithstanding);
it hasn't been formally reviewed yet AFAICT. So my thinking here is
to get docs to minimum acceptable standards in the short term and
focus on the structural code issues for the 'fest (if jsonb slips then
it's moot obviously).

merlin

--
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/28/2014 10:29 AM, Merlin Moncure wrote:

In addition to this, the JSON vs JSONB datatype page really needs
expansion and clarification.

right: exactly. I'd be happy to help (such as I can) ...I wanted to
see if jsonb to make it in on this 'fest (doc issues notwithstanding);
it hasn't been formally reviewed yet AFAICT. So my thinking here is
to get docs to minimum acceptable standards in the short term and
focus on the structural code issues for the 'fest (if jsonb slips then
it's moot obviously).

Well, having reviewed the docs before Andrew sent them in, I felt they
already *were* "minimum acceptable". Certainly they're as complete as
the original JSON docs were.

Or is this just about whitespace and line breaks?

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

Josh Berkus escribi�:

Or is this just about whitespace and line breaks?

If the docs are going to be rehauled, please ignore my whitespace
comments.

--
�lvaro Herrera 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 01/28/2014 10:56 AM, Alvaro Herrera wrote:

Josh Berkus escribió:

Or is this just about whitespace and line breaks?

If the docs are going to be rehauled, please ignore my whitespace
comments.

I'm sure you'll find plenty to criticize in my version. ;-)

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