Lists: | pgsql-hackers |
---|
From: | Oskari Saarenmaa <os(at)ohmu(dot)fi> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | [PATCH] hstore_to_json: empty hstores must return empty json objects |
Date: | 2013-10-16 16:02:31 |
Message-ID: | 20131016160231.GB2168@saarenmaa.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
hstore_to_json used to return an empty string for empty hstores, but
that is not correct as an empty string is not valid json and calling
row_to_json() on rows which have empty hstores will generate invalid
json for the entire row. The right thing to do is to return an empty
json object.
Signed-off-by: Oskari Saarenmaa <os(at)ohmu(dot)fi>
---
This should probably be applied to 9.3 tree as well as master.
# select row_to_json(r.*) from (select ''::hstore as d) r;
{"d":}
# select hstore_to_json('')::text::json;
ERROR: invalid input syntax for type json
contrib/hstore/expected/hstore.out | 12 ++++++++++++
contrib/hstore/hstore_io.c | 8 ++++----
contrib/hstore/sql/hstore.sql | 2 ++
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/contrib/hstore/expected/hstore.out b/contrib/hstore/expected/hstore.out
index 2114143..3280b5e 100644
--- a/contrib/hstore/expected/hstore.out
+++ b/contrib/hstore/expected/hstore.out
@@ -1472,6 +1472,18 @@ select hstore_to_json_loose('"a key" =>1, b => t, c => null, d=> 12345, e => 012
{"b": true, "c": null, "d": 12345, "e": "012345", "f": 1.234, "g": 2.345e+4, "a key": 1}
(1 row)
+select hstore_to_json('');
+ hstore_to_json
+----------------
+ {}
+(1 row)
+
+select hstore_to_json_loose('');
+ hstore_to_json_loose
+----------------------
+ {}
+(1 row)
+
create table test_json_agg (f1 text, f2 hstore);
insert into test_json_agg values ('rec1','"a key" =>1, b => t, c => null, d=> 12345, e => 012345, f=> 1.234, g=> 2.345e+4'),
('rec2','"a key" =>2, b => f, c => "null", d=> -12345, e => 012345.6, f=> -1.234, g=> 0.345e-4');
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index d3e67dd..3781a71 100644
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
if (count == 0)
{
- out = palloc(1);
- *out = '\0';
+ out = palloc(3);
+ memcpy(out, "{}", 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
@@ -1370,8 +1370,8 @@ hstore_to_json(PG_FUNCTION_ARGS)
if (count == 0)
{
- out = palloc(1);
- *out = '\0';
+ out = palloc(3);
+ memcpy(out, "{}", 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
diff --git a/contrib/hstore/sql/hstore.sql b/contrib/hstore/sql/hstore.sql
index 68b74bf..47cbfad 100644
--- a/contrib/hstore/sql/hstore.sql
+++ b/contrib/hstore/sql/hstore.sql
@@ -335,6 +335,8 @@ select count(*) from testhstore where h = 'pos=>98, line=>371, node=>CBA, indexe
select hstore_to_json('"a key" =>1, b => t, c => null, d=> 12345, e => 012345, f=> 1.234, g=> 2.345e+4');
select cast( hstore '"a key" =>1, b => t, c => null, d=> 12345, e => 012345, f=> 1.234, g=> 2.345e+4' as json);
select hstore_to_json_loose('"a key" =>1, b => t, c => null, d=> 12345, e => 012345, f=> 1.234, g=> 2.345e+4');
+select hstore_to_json('');
+select hstore_to_json_loose('');
create table test_json_agg (f1 text, f2 hstore);
insert into test_json_agg values ('rec1','"a key" =>1, b => t, c => null, d=> 12345, e => 012345, f=> 1.234, g=> 2.345e+4'),
--
1.8.3.1
From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Oskari Saarenmaa <os(at)ohmu(dot)fi> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] hstore_to_json: empty hstores must return empty json objects |
Date: | 2013-10-17 12:20:38 |
Message-ID: | CA+TgmoZjMKZ_k5o6j4cpynwjx2jcoj8TTxtP0yiVcYn5m6U=7g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Wed, Oct 16, 2013 at 12:02 PM, Oskari Saarenmaa <os(at)ohmu(dot)fi> wrote:
> hstore_to_json used to return an empty string for empty hstores, but
> that is not correct as an empty string is not valid json and calling
> row_to_json() on rows which have empty hstores will generate invalid
> json for the entire row. The right thing to do is to return an empty
> json object.
Hmm. This sure looks like a bug to me.
Anyone think otherwise?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From: | Merlin Moncure <mmoncure(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Oskari Saarenmaa <os(at)ohmu(dot)fi>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] hstore_to_json: empty hstores must return empty json objects |
Date: | 2013-10-17 13:19:59 |
Message-ID: | CAHyXU0zWhtu+G0AaY71KOb7VrUeFtoJ+OexmLEGe+3HcVZO=bw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Thu, Oct 17, 2013 at 7:20 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Oct 16, 2013 at 12:02 PM, Oskari Saarenmaa <os(at)ohmu(dot)fi> wrote:
>> hstore_to_json used to return an empty string for empty hstores, but
>> that is not correct as an empty string is not valid json and calling
>> row_to_json() on rows which have empty hstores will generate invalid
>> json for the entire row. The right thing to do is to return an empty
>> json object.
>
> Hmm. This sure looks like a bug to me.
>
> Anyone think otherwise?
It is a bug.
merlin
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Oskari Saarenmaa <os(at)ohmu(dot)fi>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] hstore_to_json: empty hstores must return empty json objects |
Date: | 2013-10-17 13:33:20 |
Message-ID: | 525FE720.50405@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 10/17/2013 08:20 AM, Robert Haas wrote:
> On Wed, Oct 16, 2013 at 12:02 PM, Oskari Saarenmaa <os(at)ohmu(dot)fi> wrote:
>> hstore_to_json used to return an empty string for empty hstores, but
>> that is not correct as an empty string is not valid json and calling
>> row_to_json() on rows which have empty hstores will generate invalid
>> json for the entire row. The right thing to do is to return an empty
>> json object.
> Hmm. This sure looks like a bug to me.
>
> Anyone think otherwise?
>
No, you're right. It was a thinko on my part. :-(
cheers
andrew
From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Oskari Saarenmaa <os(at)ohmu(dot)fi> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] hstore_to_json: empty hstores must return empty json objects |
Date: | 2013-10-17 14:23:20 |
Message-ID: | 20131017142320.GE9746@eldon.alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Oskari Saarenmaa wrote:
> @@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
>
> if (count == 0)
> {
> - out = palloc(1);
> - *out = '\0';
> + out = palloc(3);
> + memcpy(out, "{}", 3);
> PG_RETURN_TEXT_P(cstring_to_text(out));
> }
Can't you just PG_RETURN_TEXT_P(cstring_to_text("{}")) ?
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From: | Oskari Saarenmaa <os(at)ohmu(dot)fi> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] hstore_to_json: empty hstores must return empty json objects |
Date: | 2013-10-17 14:37:12 |
Message-ID: | 525FF618.2070409@ohmu.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 17/10/13 17:23, Alvaro Herrera wrote:
> Oskari Saarenmaa wrote:
>
>> @@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
>>
>> if (count == 0)
>> {
>> - out = palloc(1);
>> - *out = '\0';
>> + out = palloc(3);
>> + memcpy(out, "{}", 3);
>> PG_RETURN_TEXT_P(cstring_to_text(out));
>> }
>
> Can't you just PG_RETURN_TEXT_P(cstring_to_text("{}")) ?
>
I'm not too familiar with PG internals and just modified this to work
like it did before, but it looks like the extra allocation is indeed
unnecessary.
I can post a new patch to make this use cstring_to_text_with_len("{}",
2) (to avoid an unnecessary strlen call) or you can just make the change
and push the modified version.
Thanks,
Oskari
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Oskari Saarenmaa <os(at)ohmu(dot)fi>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] hstore_to_json: empty hstores must return empty json objects |
Date: | 2013-10-17 14:43:32 |
Message-ID: | 525FF794.9010405@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 10/17/2013 10:23 AM, Alvaro Herrera wrote:
> Oskari Saarenmaa wrote:
>
>> @@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
>>
>> if (count == 0)
>> {
>> - out = palloc(1);
>> - *out = '\0';
>> + out = palloc(3);
>> + memcpy(out, "{}", 3);
>> PG_RETURN_TEXT_P(cstring_to_text(out));
>> }
> Can't you just PG_RETURN_TEXT_P(cstring_to_text("{}")) ?
>
Yeah. I'm going to fix this this morning.
cheers
andrew