latest hstore patch

Started by Andrew Gierthover 16 years ago29 messageshackers
Jump to latest
#1Andrew Gierth
andrew@tao11.riddles.org.uk

Hstore patch incorporating changes as previously discussed.

In addition the requested new features of conversions to and from
array formats have been added (with docs).

--
Andrew (irc:RhodiumToad)

Attachments:

hstore-20090923.patch.gzapplication/octet-streamDownload
#2David E. Wheeler
david@kineticode.com
In reply to: Andrew Gierth (#1)
Re: latest hstore patch

On Sep 22, 2009, at 7:18 PM, Andrew Gierth wrote:

Hstore patch incorporating changes as previously discussed.

In addition the requested new features of conversions to and from
array formats have been added (with docs).

Thanks Andrew.

Just a few thoughts for discussion:

* From my previous posts: Is it time to kill off `@` and `~`,? Not
necessarily for your patch to handle, just wondering what others think.

* I like the %% operator for converting to arrays. Though I think
maybe I would have liked %@ better, but maybe that's just the Perl
hacker in me.

* I also like the new %# operator to convert to two-dimensional
arrays. But if you adopted %@ for arrays, maybe %@@ better indicates a
2-dimensional array? I'm just thinking out lout here, I'm happy to
have them no matter what they're called.

* More name stuff: Why `hstore_to_list` rather than `hstore_to_array`?
And I'm not sure about `hstore_to_matrix` for the 2-dimensional array.
I guess that's better than `hstore_to_multidimensional_array` would
be. ;-)

For those following along at home, here's what these guys look like:

       SELECT %% 'a=>foo, b=>bar'::hstore as array_op,
              hstore_to_list('a=>foo, b=>bar'::hstore),
              %#  'a=>foo, b=>bar'::hstore as matrix_op,
              hstore_to_matrix('a=>foo, b=>bar'::hstore);
          array_op    | hstore_to_list |     matrix_op     |  
hstore_to_matrix
       ---------------+----------------+------------------- 
+-------------------
        {a,foo,b,bar} | {a,foo,b,bar}  | {{a,foo},{b,bar}} | {{a,foo}, 
{b,bar}}
       (1 row)

Pretty cool!

* Thanks for updating the docs with:
   + BTREE and HASH index support
   + A fix for the populate_hash() pasto
   + A link to a discussion of backslashing and SQL standard strings
   + A note on the overhead of reading the old binary format
   + Notes on how to update from the old binary format

In the attached patch, I made a few tweaks to the hstore docs, after
applying your patch. I would have created a new patch with everything,
but ran out of time trying to convince Git to create a context diff.
This is a unified diff, but short, with just these changes:

* Fixed doc pasto for %#.
* Noted in docs that the format is new in 8.5, rather than "this
version".
* Eliminated a redundant "However, ".
* Added an example for creating a HASH index.

In sum: Modulo a discussion of the names of the array casting
operators and functions, I think this patch is ready for committer
review.

Thanks,

David

Attachments:

hstore-doc.patchapplication/octet-stream; name=hstore-doc.patch; x-unix-mode=0644Download+8-5
#3Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: David E. Wheeler (#2)
Re: latest hstore patch

"David" == "David E Wheeler" <david@kineticode.com> writes:

David> Just a few thoughts for discussion:

David> * From my previous posts: Is it time to kill off `@` and `~`,?
David> Not necessarily for your patch to handle, just wondering what
David> others think.

I'll take them out if people think that is appropriate.

David> * I like the %% operator for converting to arrays. Though I
David> think maybe I would have liked %@ better, but maybe that's
David> just the Perl hacker in me.

I originally tried just % but something in the grammar stops you using
that for a unary op.

David> * I also like the new %# operator to convert to
David> two-dimensional arrays. But if you adopted %@ for arrays,
David> maybe %@@ better indicates a 2-dimensional array? I'm just
David> thinking out lout here, I'm happy to have them no matter what
David> they're called.

%@@ is a bit on the ugly side for an operator I think.

David> * More name stuff: Why `hstore_to_list` rather than
David> `hstore_to_array`? And I'm not sure about `hstore_to_matrix`
David> for the 2-dimensional array. I guess that's better than
David> `hstore_to_multidimensional_array` would be. ;-)

I intentionally avoided hstore_to_array because it would be unclear
which one it meant (the 1-d or 2-d result).

--
Andrew (irc:RhodiumToad)

#4David E. Wheeler
david@kineticode.com
In reply to: Andrew Gierth (#3)
Re: latest hstore patch

On Sep 23, 2009, at 5:27 PM, Andrew Gierth wrote:

I intentionally avoided hstore_to_array because it would be unclear
which one it meant (the 1-d or 2-d result).

Thanks Andrew.

Given these replies, unless anyone else wants to weigh in on the array
conversion operator and function names, this patch is ready for
committer review (along with my tiny doc patch). I'll update the
commitfest site to that effect.

Thanks,

David

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Gierth (#3)
Re: latest hstore patch

[ starting to look at this now... ]

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

"David" == "David E Wheeler" <david@kineticode.com> writes:
David> * More name stuff: Why `hstore_to_list` rather than
David> `hstore_to_array`? And I'm not sure about `hstore_to_matrix`
David> for the 2-dimensional array. I guess that's better than
David> `hstore_to_multidimensional_array` would be. ;-)

I intentionally avoided hstore_to_array because it would be unclear
which one it meant (the 1-d or 2-d result).

hstore_to_list seems like a pretty horrible name though for something
that produces an array. I also note that "array" means "1-D array"
according to no less an authority than the SQL standard ;-). I think
we could live with hstore_to_array and hstore_to_matrix. Thoughts,
other ideas?

regards, tom lane

#6Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: Tom Lane (#5)
Re: latest hstore patch

"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

David> * More name stuff: Why `hstore_to_list` rather than
David> `hstore_to_array`? And I'm not sure about `hstore_to_matrix`
David> for the 2-dimensional array. I guess that's better than
David> `hstore_to_multidimensional_array` would be. ;-)

I intentionally avoided hstore_to_array because it would be
unclear which one it meant (the 1-d or 2-d result).

Tom> hstore_to_list seems like a pretty horrible name though for
Tom> something that produces an array. I also note that "array"
Tom> means "1-D array" according to no less an authority than the SQL
Tom> standard ;-). I think we could live with hstore_to_array and
Tom> hstore_to_matrix. Thoughts, other ideas?

I don't feel particularly strongly about the name (I've also
intentionally held off on updating the pgfoundry version of the code
until this is settled so no-one else should care either).

My own expectation is that the operator should normally be used in
preference (though obviously people's tastes will vary in this
respect).

--
Andrew (irc:RhodiumToad)

#7Josh Berkus
josh@agliodbs.com
In reply to: Andrew Gierth (#6)
Re: latest hstore patch

I don't feel particularly strongly about the name (I've also
intentionally held off on updating the pgfoundry version of the code
until this is settled so no-one else should care either).

Well, since we already have string_to_array, hstore_to_array would be
consistent.

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

#8David E. Wheeler
david@kineticode.com
In reply to: Andrew Gierth (#6)
Re: latest hstore patch

On Sep 29, 2009, at 4:11 PM, Andrew Gierth wrote:

I don't feel particularly strongly about the name (I've also
intentionally held off on updating the pgfoundry version of the code
until this is settled so no-one else should care either).

I'm down with hstore_to_array() and hstore_to_matrix().

My own expectation is that the operator should normally be used in
preference (though obviously people's tastes will vary in this
respect).

Sure. But I realized that I forgot to ask for array_to_hstore() and
matrix_to_hstore(). :-) Would love to have those, too. Not sure about
the operators…

Best,

David

#9Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: David E. Wheeler (#8)
Re: latest hstore patch

"David" == "David E Wheeler" <david@kineticode.com> writes:

I don't feel particularly strongly about the name (I've also
intentionally held off on updating the pgfoundry version of the
code until this is settled so no-one else should care either).

David> I'm down with hstore_to_array() and hstore_to_matrix().

My own expectation is that the operator should normally be used in
preference (though obviously people's tastes will vary in this
respect).

David> Sure. But I realized that I forgot to ask for
David> array_to_hstore() and matrix_to_hstore(). :-) Would love to
David> have those, too. Not sure about the operators…

hstore(text[]) (which is also present as an explicit cast) covers both
of those cases since it can figure out from the array dimensions which
is intended.

--
Andrew (irc:RhodiumToad)

#10David E. Wheeler
david@kineticode.com
In reply to: Andrew Gierth (#9)
Re: latest hstore patch

On Sep 29, 2009, at 5:00 PM, Andrew Gierth wrote:

David> Sure. But I realized that I forgot to ask for
David> array_to_hstore() and matrix_to_hstore(). :-) Would love to
David> have those, too. Not sure about the operators…

hstore(text[]) (which is also present as an explicit cast) covers both
of those cases since it can figure out from the array dimensions which
is intended.

Oooh! RhodiumToad++

Thanks,

David

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Gierth (#1)
Re: latest hstore patch

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

Hstore patch incorporating changes as previously discussed.
In addition the requested new features of conversions to and from
array formats have been added (with docs).

Applied with some mostly-cosmetic editorialization.

regards, tom lane

#12David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#11)
Re: latest hstore patch

On Sep 30, 2009, at 12:52 PM, Tom Lane wrote:

Applied with some mostly-cosmetic editorialization.

And there was much rejoicing…

David

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#12)
Re: latest hstore patch

"David E. Wheeler" <david@kineticode.com> writes:

On Sep 30, 2009, at 12:52 PM, Tom Lane wrote:

Applied with some mostly-cosmetic editorialization.

And there was much rejoicing�

... except in the buildfarm. Must be some platform dependency we both
missed ...

regards, tom lane

#14Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: Tom Lane (#13)
Re: latest hstore patch

"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

And there was much rejoicing

Tom> ... except in the buildfarm. Must be some platform dependency
Tom> we both missed ...

"oops"

--
Andrew (irc:RhodiumToad)

#15Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#5)
Re: latest hstore patch

Tom Lane wrote:

I intentionally avoided hstore_to_array because it would be unclear
which one it meant (the 1-d or 2-d result).

hstore_to_list seems like a pretty horrible name though for something
that produces an array. I also note that "array" means "1-D array"
according to no less an authority than the SQL standard ;-). I think
we could live with hstore_to_array and hstore_to_matrix. Thoughts,
other ideas?

Off topic, but in normal English usage I thought 'vector' was a 1-D
array, and an array could be any number of dimensions.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#16Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#11)
Re: latest hstore patch

Tom Lane wrote:

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

Hstore patch incorporating changes as previously discussed.
In addition the requested new features of conversions to and from
array formats have been added (with docs).

Applied with some mostly-cosmetic editorialization.

Are there any pg_migrator affects in this patch? We had discussed this
issue in the past with this patch.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#16)
Re: latest hstore patch

Bruce Momjian <bruce@momjian.us> writes:

Are there any pg_migrator affects in this patch? We had discussed this
issue in the past with this patch.

The code is upward compatible with the old on-disk format, so that
end of things is fine.

There's still the issue of how to get the improved module definition
(new functions etc) into a migrated database. That's not specific
to hstore in any way though, it would affect any contrib module that
had added stuff in a new release.

regards, tom lane

#18Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#17)
Re: latest hstore patch

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Are there any pg_migrator affects in this patch? We had discussed this
issue in the past with this patch.

The code is upward compatible with the old on-disk format, so that
end of things is fine.

There's still the issue of how to get the improved module definition
(new functions etc) into a migrated database. That's not specific
to hstore in any way though, it would affect any contrib module that
had added stuff in a new release.

Most modules just install functions, which are easily
uninstalled/reinstalled. A data type like hstore is more complicated
assuming it is the data type that is changing and not the support
functions.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#19David E. Wheeler
david@kineticode.com
In reply to: Bruce Momjian (#18)
Re: latest hstore patch

On Oct 2, 2009, at 8:20 AM, Bruce Momjian wrote:

Most modules just install functions, which are easily
uninstalled/reinstalled. A data type like hstore is more complicated
assuming it is the data type that is changing and not the support
functions.

Lots of modules install data types. From contrib:

* hstore
* uin
* citext
* cube
* inarray
* ltree

Plus lots of stuff on pgFoundry. It's a problem that needs to be
solved. Surely someone, somewhere, has solved this problem, no?

Best,

David

#20Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: David E. Wheeler (#19)
Re: latest hstore patch

David E. Wheeler wrote:

Plus lots of stuff on pgFoundry. It's a problem that needs to be
solved. Surely someone, somewhere, has solved this problem, no?

Dump & reload?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#21David E. Wheeler
david@kineticode.com
In reply to: Alvaro Herrera (#20)
#22Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#20)
#23Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#22)
#24David E. Wheeler
david@kineticode.com
In reply to: Alvaro Herrera (#23)
#25Bruce Momjian
bruce@momjian.us
In reply to: David E. Wheeler (#21)
#26David E. Wheeler
david@kineticode.com
In reply to: Bruce Momjian (#25)
#27Bruce Momjian
bruce@momjian.us
In reply to: David E. Wheeler (#26)
#28Dimitri Fontaine
dimitri@2ndQuadrant.fr
In reply to: David E. Wheeler (#24)
#29Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: Bruce Momjian (#18)