Merge branch 'joelhoffman-postgres_schema_builder' into instance_reader

* joelhoffman-postgres_schema_builder:
  Also support writing the hstore back to the database
  Hstore values are all strings
  string_to_hstore / hstore_to_string, serializing
  don't test schema where hstore not installed
  schema dumper tests for hstore
  Additional hstore tests, supporting null values, better compliance with postgres docs
  add hstore to postgres native types and defaults

Conflicts:
	activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

6 files changed, 126 insertions, 36 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb
index 34d88edff3..7ef3218a49 100644
--- a/activerecord/lib/active_record/connection_adapters/column.rb
+++ b/activerecord/lib/active_record/connection_adapters/column.rb
@@ -98,7 +98,7 @@ module ActiveRecord
         when :date                 then klass.value_to_date(value)
         when :binary               then klass.binary_to_string(value)
         when :boolean              then klass.value_to_boolean(value)
-        when :hstore               then klass.cast_hstore(value)
+        when :hstore               then klass.string_to_hstore(value)
         else value
         end
       end
@@ -116,7 +116,7 @@ module ActiveRecord
         when :date                 then "#{klass}.value_to_date(#{var_name})"
         when :binary               then "#{klass}.binary_to_string(#{var_name})"
         when :boolean              then "#{klass}.value_to_boolean(#{var_name})"
-        when :hstore               then "#{klass}.cast_hstore(#{var_name})"
+        when :hstore               then "#{klass}.string_to_hstore(#{var_name})"
         else var_name
         end
       end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
index 006742688e..8110db78c4 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
@@ -133,7 +133,7 @@ module ActiveRecord
           def type_cast(value)
             return if value.nil?
 
-            ConnectionAdapters::PostgreSQLColumn.cast_hstore value
+            ConnectionAdapters::PostgreSQLColumn.string_to_hstore value
           end
         end
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index f38fca6576..eccf3595aa 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -53,40 +53,42 @@ module ActiveRecord
           end
         end
 
-        def cast_hstore(object)
+        def hstore_to_string(object)
           if Hash === object
             object.map { |k,v|
               "#{escape_hstore(k)}=>#{escape_hstore(v)}"
-            }.join ', '
+            }.join ','
           else
-            kvs = object.scan(/(?<!\\)".*?(?<!\\)"/).map { |o|
-              unescape_hstore(o[1...-1])
-            }
-            Hash[kvs.each_slice(2).to_a]
+            object
           end
         end
 
-        private
-        HSTORE_ESCAPE = {
-            ' '  => '\\ ',
-            '\\' => '\\\\',
-            '"'  => '\\"',
-            '='  => '\\=',
-        }
-        HSTORE_ESCAPE_RE   = Regexp.union(HSTORE_ESCAPE.keys)
-        HSTORE_UNESCAPE    = HSTORE_ESCAPE.invert
-        HSTORE_UNESCAPE_RE = Regexp.union(HSTORE_UNESCAPE.keys)
-
-        def unescape_hstore(value)
-          value.gsub(HSTORE_UNESCAPE_RE) do |match|
-            HSTORE_UNESCAPE[match]
+        def string_to_hstore(string)
+          if string.nil?
+            nil
+          elsif String === string
+            Hash[string.scan(HstorePair).map { |k,v|
+              v = v.upcase == 'NULL' ? nil : v.gsub(/^"(.*)"$/,'\1').gsub(/\\(.)/, '\1')
+              k = k.gsub(/^"(.*)"$/,'\1').gsub(/\\(.)/, '\1')
+              [k,v]
+            }]
+          else
+            string
           end
         end
 
+        private
+        HstorePair = begin
+          quoted_string = /"[^"\\]*(?:\\.[^"\\]*)*"/
+          unquoted_string = /(?:\\.|[^\s,])[^\s=,\\]*(?:\\.[^\s=,\\]*|=[^,>])*/
+          /(#{quoted_string}|#{unquoted_string})\s*=>\s*(#{quoted_string}|#{unquoted_string})/
+        end
+
         def escape_hstore(value)
-          value.gsub(HSTORE_ESCAPE_RE) do |match|
-            HSTORE_ESCAPE[match]
-          end
+            value.nil?         ? 'NULL'
+          : value =~ /[=\s,>]/ ? '"%s"' % value.gsub(/(["\\])/, '\\\\\1')
+          : value == ""        ? '""'
+          :                      value.to_s.gsub(/(["\\])/, '\\\\\1')
         end
       end
       # :startdoc:
@@ -139,6 +141,9 @@ module ActiveRecord
           # Arrays
           when /\A'(.*)'::"?\D+"?\[\]\z/
             $1
+          # Hstore
+          when /\A'(.*)'::hstore\z/
+            $1
           # Object identifier types
           when /\A-?\d+\z/
             $1
@@ -285,7 +290,8 @@ module ActiveRecord
         :binary      => { :name => "bytea" },
         :boolean     => { :name => "boolean" },
         :xml         => { :name => "xml" },
-        :tsvector    => { :name => "tsvector" }
+        :tsvector    => { :name => "tsvector" },
+        :hstore      => { :name => "hstore" }
       }
 
       # Returns 'PostgreSQL' as adapter name for identification purposes.
@@ -475,6 +481,11 @@ module ActiveRecord
         return super unless column
 
         case value
+        when Hash
+          case column.sql_type
+          when 'hstore' then super(PostgreSQLColumn.hstore_to_string(value), column)
+          else super
+          end
         when Float
           return super unless value.infinite? && column.type == :datetime
           "'#{value.to_s.downcase}'"
@@ -506,6 +517,9 @@ module ActiveRecord
         when String
           return super unless 'bytea' == column.sql_type
           { :value => value, :format => 1 }
+        when Hash
+          return super unless 'hstore' == column.sql_type
+          PostgreSQLColumn.hstore_to_string(value)
         else
           super
         end
diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb
index 33bf4478cc..1644a58d92 100644
--- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb
@@ -1,4 +1,6 @@
 require "cases/helper"
+require 'active_record/base'
+require 'active_record/connection_adapters/postgresql_adapter'
 
 class PostgresqlHstoreTest < ActiveRecord::TestCase
   class Hstore < ActiveRecord::Base
@@ -10,12 +12,13 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase
     begin
       @connection.transaction do
         @connection.create_table('hstores') do |t|
-          t.hstore 'tags'
+          t.hstore 'tags', :default => ''
         end
       end
     rescue ActiveRecord::StatementInvalid
       return skip "do not test on PG without hstore"
     end
+    @column = Hstore.columns.find { |c| c.name == 'tags' }
   end
 
   def teardown
@@ -23,21 +26,74 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase
   end
 
   def test_column
-    column = Hstore.columns.find { |c| c.name == 'tags' }
-    assert column
-    assert_equal :hstore, column.type
+    assert_equal :hstore, @column.type
   end
 
   def test_type_cast_hstore
-    column = Hstore.columns.find { |c| c.name == 'tags' }
-    assert column
+    assert @column
 
     data = "\"1\"=>\"2\""
-    hash = column.class.cast_hstore data
+    hash = @column.class.string_to_hstore data
     assert_equal({'1' => '2'}, hash)
-    assert_equal({'1' => '2'}, column.type_cast(data))
+    assert_equal({'1' => '2'}, @column.type_cast(data))
+
+    assert_equal({}, @column.type_cast(""))
+    assert_equal({'key'=>nil}, @column.type_cast('key => NULL'))
+    assert_equal({'c'=>'}','"a"'=>'b "a b'}, @column.type_cast(%q(c=>"}", "\"a\""=>"b \"a b")))
+  end
+
+  def test_gen1
+    assert_equal(%q(" "=>""), @column.class.hstore_to_string({' '=>''}))
+  end
+
+  def test_gen2
+    assert_equal(%q(","=>""), @column.class.hstore_to_string({','=>''}))
+  end
+
+  def test_gen3
+    assert_equal(%q("="=>""), @column.class.hstore_to_string({'='=>''}))
+  end
+
+  def test_gen4
+    assert_equal(%q(">"=>""), @column.class.hstore_to_string({'>'=>''}))
+  end
+
+  def test_parse1
+    assert_equal({'a'=>nil,'b'=>nil,'c'=>'NuLl','null'=>'c'}, @column.type_cast('a=>null,b=>NuLl,c=>"NuLl",null=>c'))
+  end
+
+  def test_parse2
+    assert_equal({" " => " "},  @column.type_cast("\\ =>\\ "))
+  end
+
+  def test_parse3
+    assert_equal({"=" => ">"},  @column.type_cast("==>>"))
   end
 
+  def test_parse4
+    assert_equal({"=a"=>"q=w"},   @column.type_cast('\=a=>q=w'))
+  end
+
+  def test_parse5
+    assert_equal({"=a"=>"q=w"},   @column.type_cast('"=a"=>q\=w'))
+  end
+
+  def test_parse6
+    assert_equal({"\"a"=>"q>w"},  @column.type_cast('"\"a"=>q>w'))
+  end
+
+  def test_parse7
+    assert_equal({"\"a"=>"q\"w"}, @column.type_cast('\"a=>q"w'))
+  end
+
+  def test_rewrite
+    @connection.execute "insert into hstores (tags) VALUES ('1=>2')"
+    x = Hstore.find :first
+    x.tags = { '"a\'' => 'b' }
+    assert x.save!
+  end
+
+
   def test_select
     @connection.execute "insert into hstores (tags) VALUES ('1=>2')"
     x = Hstore.find :first
@@ -54,6 +110,10 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase
     assert_cycle('a' => 'b', '1' => '2')
   end
 
+  def test_nil
+    assert_cycle('a' => nil)
+  end
+
   def test_quotes
     assert_cycle('a' => 'b"ar', '1"foo' => '2')
   end
diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
index c79382a984..4de0999dbb 100644
--- a/activerecord/test/cases/schema_dumper_test.rb
+++ b/activerecord/test/cases/schema_dumper_test.rb
@@ -238,6 +238,13 @@ class SchemaDumperTest < ActiveRecord::TestCase
 
     def test_schema_dump_includes_tsvector_shorthand_definition
       output = standard_dump
+      if %r{create_table "postgresql_hstores"} =~ output
+        assert_match %r{t.hstore "hash_store", default => ""}, output
+      end
+    end
+
+    def test_schema_dump_includes_tsvector_shorthand_definition
+      output = standard_dump
       if %r{create_table "postgresql_tsvectors"} =~ output
         assert_match %r{t.tsvector "text_vector"}, output
       end
diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb
index 5cf9a207f3..25b416a906 100644
--- a/activerecord/test/schema/postgresql_specific_schema.rb
+++ b/activerecord/test/schema/postgresql_specific_schema.rb
@@ -1,6 +1,6 @@
 ActiveRecord::Schema.define do
 
-  %w(postgresql_tsvectors postgresql_arrays postgresql_moneys postgresql_numbers postgresql_times postgresql_network_addresses postgresql_bit_strings
+  %w(postgresql_tsvectors postgresql_hstores postgresql_arrays postgresql_moneys postgresql_numbers postgresql_times postgresql_network_addresses postgresql_bit_strings
       postgresql_oids postgresql_xml_data_type defaults geometrics postgresql_timestamp_with_zones).each do |table_name|
     execute "DROP TABLE  IF EXISTS #{quote_table_name table_name}"
   end
@@ -63,6 +63,15 @@ _SQL
   );
 _SQL
 
+  if 't' == select_value("select 'hstore'=ANY(select typname from pg_type)")
+  execute <<_SQL
+  CREATE TABLE postgresql_hstores (
+    id SERIAL PRIMARY KEY,
+    hash_store hstore default ''::hstore
+  );
+_SQL
+  end
+
   execute <<_SQL
   CREATE TABLE postgresql_moneys (
     id SERIAL PRIMARY KEY,