Re: PATCH: Add hstore_to_json() - Mailing list pgsql-hackers

From Robert Haas
Subject Re: PATCH: Add hstore_to_json()
Date
Msg-id 603c8f070912161445y36a34a50r9e13f80e8e2078eb@mail.gmail.com
Whole thread Raw
In response to PATCH: Add hstore_to_json()  ("David E. Wheeler" <david@kineticode.com>)
Responses Re: PATCH: Add hstore_to_json()  ("David E. Wheeler" <david@kineticode.com>)
List pgsql-hackers
On Wed, Dec 16, 2009 at 2:28 PM, David E. Wheeler <david@kineticode.com> wrote:
> I just realized that this was easy to do, and despite my complete lack of C skillz was able to throw this together in
acouple of hours. It might be handy to some, though the possible downsides are: 
>
> * No json_to_hstore().
> * Leads to requests for hstore_to_yaml(), hstore_to_xml(), etc.
> * Andrew Gierth said “no” when I suggested it.
>
> But it's kind of handy, too. Thoughts?

I like it.  The regression tests you've added seem to cover a lot of
cases that aren't really different without covering some that are
probably worth trying, like multiple key/value pairs.  Also, the
comment in the function you've added looks like a cut-and-paste from
somewhere else, which might not be the best way to document.  With
regard to the underlying issue, why can't we just use a StringInfo and
forget about it?

Also, your indentation is not entirely consistent.  If this gets
consensus, that will have to be fixed before it can be committed, so
it would be nice if you could do that rather than leaving it for the
eventual committer.

...Robert


pgsql-hackers by date:

Previous
From: Jeff Davis
Date:
Subject: Re: Range types
Next
From: Tom Lane
Date:
Subject: Re: determine snapshot after obtaining locks for first statement