Re: PATCH: Add hstore_to_json()

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: "David E(dot) Wheeler" <david(at)kineticode(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: Add hstore_to_json()
Date: 2009-12-16 22:45:57
Message-ID: 603c8f070912161445y36a34a50r9e13f80e8e2078eb@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Dec 16, 2009 at 2:28 PM, David E. Wheeler <david(at)kineticode(dot)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 a couple 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2009-12-16 22:52:03 Re: determine snapshot after obtaining locks for first statement
Previous Message Jeff Davis 2009-12-16 22:31:29 Re: Range types