Re: JSON NULLs

Hackers,

While playing with Andrew’s JSON enhancements, I noticed this:

david=# select * From json_each_as_text('{"baz": null}'::json);
key | value
-----+-------
baz | null

It is returning 'null'::text there, not NULL::text. I had expected the latter, because otherwise it's not possible to tell the difference between '{"foo": null}' and '{"foo": "null"}'.

But then I noticed that this seems to be true for JSON NULLs in general:

david=# select 'null'::json::text IS NULL;
?column?
----------
f

Again, I expected a NULL there. I recognize that JSON NULLs are not the same as SQL NULLs, but if there is no way to tell the difference, well, it’s annoying.

I see that '"null"'::json::text resolves to '"null"'::text, so that’s one way to deal with it. But since json_each_as_text returns values as text, not quoted JSON values, maybe *it* should return JSON NULLs as SQL NULLs?

Thanks,

David

On Wed, Feb 6, 2013 at 1:08 PM, David E. Wheeler <david(at)justatheory(dot)com> wrote:
> Hackers,
>
> While playing with Andrew’s JSON enhancements, I noticed this:
>
> david=# select * From json_each_as_text('{"baz": null}'::json);
> key | value
> -----+-------
> baz | null
>
> It is returning 'null'::text there, not NULL::text. I had expected the latter, because otherwise it's not possible to tell the difference between '{"foo": null}' and '{"foo": "null"}'.

IMO, this is bug in proposed implementation. json unquoted null
should not map to string 'null' but to SQL, casting behavior from
text as implemented looks correct. (only SQL null should produce json
null)

merlin

On 02/06/2013 02:24 PM, Merlin Moncure wrote:
> On Wed, Feb 6, 2013 at 1:08 PM, David E. Wheeler <david(at)justatheory(dot)com> wrote:
>> Hackers,
>>
>> While playing with Andrew’s JSON enhancements, I noticed this:
>>
>> david=# select * From json_each_as_text('{"baz": null}'::json);
>> key | value
>> -----+-------
>> baz | null
>>
>> It is returning 'null'::text there, not NULL::text. I had expected the latter, because otherwise it's not possible to tell the difference between '{"foo": null}' and '{"foo": "null"}'.
> IMO, this is bug in proposed implementation. json unquoted null
> should not map to string 'null' but to SQL, casting behavior from
> text as implemented looks correct. (only SQL null should produce json
> null)
>

Probably. I'm on it.

cheers

andrew

On 02/06/2013 02:36 PM, Andrew Dunstan wrote:
>
> On 02/06/2013 02:24 PM, Merlin Moncure wrote:
>> On Wed, Feb 6, 2013 at 1:08 PM, David E. Wheeler
>> <david(at)justatheory(dot)com> wrote:
>>> Hackers,
>>>
>>> While playing with Andrew’s JSON enhancements, I noticed this:
>>>
>>> david=# select * From json_each_as_text('{"baz": null}'::json);
>>> key | value
>>> -----+-------
>>> baz | null
>>>
>>> It is returning 'null'::text there, not NULL::text. I had expected
>>> the latter, because otherwise it's not possible to tell the
>>> difference between '{"foo": null}' and '{"foo": "null"}'.
>> IMO, this is bug in proposed implementation. json unquoted null
>> should not map to string 'null' but to SQL, casting behavior from
>> text as implemented looks correct. (only SQL null should produce json
>> null)
>>
>
>
> Probably. I'm on it.
>

Revised patch attached. The problem also existed with the get*_as_text
functions (and their operators). Some additional regression tests are
added to test these cases.

cheers

andrew

On 2013-02-08 15:15, Andrew Dunstan wrote:
>
>
>
> Revised patch attached. The problem also existed with the get*_as_text
> functions (and their operators). Some additional regression tests are
> added to test these cases.

Hi,

I did some minor things with the patch today.

1. thanks for the work on the json type, great to see it in Postgres and
also more functions on it!

2.
during compile on

jsonfuncs.c: In function `each_object_field_end':
jsonfuncs.c:1151:13: warning: assignment makes integer from pointer
without a cast

yeb(at)unix:~/ff$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/x86_64-linux-gnu/gcc/x86_64-linux-gnu/4.5.2/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro
4.5.2-8ubuntu4' --with-bugurl=file:///usr/share/doc/gcc-4.5/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.5 --enable-shared --enable-multiarch
--with-multiarch-defaults=x86_64-linux-gnu --enable-linker-build-id
--with-system-zlib --libexecdir=/usr/lib/x86_64-linux-gnu
--without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.5
--libdir=/usr/lib/x86_64-linux-gnu --enable-nls --with-sysroot=/
--enable-clocale=gnu --enable-libstdcxx-debug
--enable-libstdcxx-time=yes --enable-plugin --enable-gold
--enable-ld=default --with-plugin-ld=ld.gold --enable-objc-gc
--disable-werror --with-arch-32=i686 --with-tune=generic
--enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.5.2 (Ubuntu/Linaro 4.5.2-8ubuntu4)

3. I was wondering how to access the first author from this json snippet:

{
"id": "QZr82w_eSi8C",
"etag": "KZ+JsrkCdqw",
"volumeInfo": {
"title": "Heads Up Software Construction",
"authors": [
"Dan Malone",
"Dave Riles"
],

and played a bit with json_get_path_as_text(document, 'volumeInfo',
'authors') that accepts a list of keys as arguments. Have you thought
about an implementation that would accept a single path argument like
'volumeInfo.authors[0]' ? This might be more powerful and easy to use,
since the user does not need to call another function to get the first
element from the author array, and the function call does not need to be
changed when path lenghts change.

My apologies if this has been discussed before - I've gone through
threads from nov 2012 but did not find a previous discussion about this
topic.

regards,
Yeb Havinga

On 02/10/2013 05:43 AM, Yeb Havinga wrote:
> On 2013-02-08 15:15, Andrew Dunstan wrote:
>>
>>
>>
>> Revised patch attached. The problem also existed with the
>> get*_as_text functions (and their operators). Some additional
>> regression tests are added to test these cases.
>
> Hi,
>
> I did some minor things with the patch today.
>
> 1. thanks for the work on the json type, great to see it in Postgres
> and also more functions on it!
>
> 2.
> during compile on
>
> jsonfuncs.c: In function `each_object_field_end':
> jsonfuncs.c:1151:13: warning: assignment makes integer from pointer
> without a cast

Thanks, I have fixed this in my code, and it will be included in the
next patch I post.

>
>
>
> 3. I was wondering how to access the first author from this json snippet:
>
> {
> "id": "QZr82w_eSi8C",
> "etag": "KZ+JsrkCdqw",
> "volumeInfo": {
> "title": "Heads Up Software Construction",
> "authors": [
> "Dan Malone",
> "Dave Riles"
> ],
>
>
> and played a bit with json_get_path_as_text(document, 'volumeInfo',
> 'authors') that accepts a list of keys as arguments. Have you thought
> about an implementation that would accept a single path argument like
> 'volumeInfo.authors[0]' ? This might be more powerful and easy to use,
> since the user does not need to call another function to get the first
> element from the author array, and the function call does not need to
> be changed when path lenghts change.

try:

json_get_path_as_text(document, 'volumeInfo', 'authors', '0')

There are other ways to spell this, too:

json_get_path_as_text(document, variadic
'{volumeInfo,authors,0}'::text[])

or

document->>'{volumeInfo,authors,0}'::text[]

I'm actually wondering if we should use different operator names for the
get_path*op functions so we wouldn't need to type qualify the path
argument. Maybe ?> and ?>> although I'm reluctant to use ? in an
operator given the recent JDBC discussion. Or perhaps #> and #>>.

cheers

andrew

On 2013-02-10 16:03, Andrew Dunstan wrote:
> On 02/10/2013 05:43 AM, Yeb Havinga wrote:
>> 3. I was wondering how to access the first author from this json
>> snippet:
>>
>> {
>> "id": "QZr82w_eSi8C",
>> "etag": "KZ+JsrkCdqw",
>> "volumeInfo": {
>> "title": "Heads Up Software Construction",
>> "authors": [
>> "Dan Malone",
>> "Dave Riles"
>> ],
>>
>>
>
> try:
>
> json_get_path_as_text(document, 'volumeInfo', 'authors', '0')
>
>
> There are other ways to spell this, too:
>
> json_get_path_as_text(document, variadic
> '{volumeInfo,authors,0}'::text[])
>
>
> or
>
> document->>'{volumeInfo,authors,0}'::text[]

That works very nice, thanks!
>
> I'm actually wondering if we should use different operator names for
> the get_path*op functions so we wouldn't need to type qualify the path
> argument. Maybe ?> and ?>> although I'm reluctant to use ? in an
> operator given the recent JDBC discussion. Or perhaps #> and #>>.

different operator name: +1.

thanks
Yeb

On 02/11/2013 03:21 AM, Yeb Havinga wrote:
>
>>
>> I'm actually wondering if we should use different operator names for
>> the get_path*op functions so we wouldn't need to type qualify the
>> path argument. Maybe ?> and ?>> although I'm reluctant to use ? in an
>> operator given the recent JDBC discussion. Or perhaps #> and #>>.
>
> different operator name: +1.
>

OK, updated patch attached. I went with #> and #>>.

cheers

andrew