PATCH: Add hstore_to_json()
in [PgSql]
|
From: Robert Haas on 16 Dec 2009 17:45 On Wed, Dec 16, 2009 at 2:28 PM, David E. Wheeler <david(a)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 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "David E. Wheeler" on 16 Dec 2009 17:58 On Dec 16, 2009, at 2:45 PM, Robert Haas wrote: > 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? Dunno. I just duped hstore_out(). I agree there should be more edge cases. > 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. The indentation is also largely copied; wouldn't pg_indent fix it? Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 16 Dec 2009 18:29 On Wed, Dec 16, 2009 at 5:58 PM, David E. Wheeler <david(a)kineticode.com> wrote: > On Dec 16, 2009, at 2:45 PM, Robert Haas wrote: > >> 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? > > Dunno. I just duped hstore_out(). I agree there should be more edge cases. > >> 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. > > The indentation is also largely copied; wouldn't pg_indent fix it? Yeah, eventually, but that's not really a great way of dealing with it. http://archives.postgresql.org/pgsql-hackers/2009-12/msg01208.php ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Peter Eisentraut on 18 Dec 2009 07:49 On ons, 2009-12-16 at 11:28 -0800, David E. Wheeler 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? Should we create a json type before adding all kinds of json formatted data? Or are we content with json as text? -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "David E. Wheeler" on 18 Dec 2009 11:32
On Dec 18, 2009, at 4:49 AM, Peter Eisentraut wrote: > Should we create a json type before adding all kinds of json formatted > data? Or are we content with json as text? json_data_type++ D -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |