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