Return PostgreSQL columns in the order they are declared #1374 (perlguy@gmail.com). Unit test column order, adapter housekeeping, simplify users of columns_hash.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1405 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 34 insertions, 30 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 5738815e78..454ab93cfa 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Return PostgreSQL columns in the order they were declared #1374 [perlguy@gmail.com]
+
 * Allow before/after update hooks to work on models using optimistic locking 
 
 * Eager loading of dependent has_one associations won't delete the association #1212
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 49d6a0c7d6..0a953cfcd0 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -617,9 +617,9 @@ module ActiveRecord #:nodoc:
       def columns_hash
         @columns_hash ||= columns.inject({}) { |hash, column| hash[column.name] = column; hash }
       end
-      
+
       def column_names
-        @column_names ||= columns_hash.keys
+        @column_names ||= columns.map { |column| column.name }
       end
 
       # Returns an array of columns objects where the primary id, all columns ending in "_id" or "_count", 
@@ -632,7 +632,7 @@ module ActiveRecord #:nodoc:
       # and true as the value. This makes it possible to do O(1) lookups in respond_to? to check if a given method for attribute
       # is available. 
       def column_methods_hash
-        @dynamic_methods_hash ||= columns_hash.keys.inject(Hash.new(false)) do |methods, attr|
+        @dynamic_methods_hash ||= column_names.inject(Hash.new(false)) do |methods, attr|
           methods[attr.to_sym]       = true
           methods["#{attr}=".to_sym] = true
           methods["#{attr}?".to_sym] = true
@@ -1293,16 +1293,14 @@ module ActiveRecord #:nodoc:
       # Returns copy of the attributes hash where all the values have been safely quoted for use in
       # an SQL statement. 
       def attributes_with_quotes(include_primary_key = true)
-        columns_hash = self.class.columns_hash
-
-        attrs_quoted = attributes.inject({}) do |attrs_quoted, pair| 
-          attrs_quoted[pair.first] = quote(pair.last, columns_hash[pair.first]) unless !include_primary_key && pair.first == self.class.primary_key
-          attrs_quoted
+        attributes.inject({}) do |quoted, (name, value)| 
+          if column = column_for_attribute(name)
+            quoted[name] = quote(value, column) unless !include_primary_key && name == self.class.primary_key
+          end
+          quoted
         end
-
-        attrs_quoted.delete_if { |key, value| !self.class.columns_hash.keys.include?(key) }
       end
-      
+
       # Quote strings appropriately for SQL statements.
       def quote(value, column = nil)
         connection.quote(value, column)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 9161878d5a..39990db8fc 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -78,19 +78,21 @@ module ActiveRecord
       end
 
       def insert(sql, name = nil, pk = nil, id_value = nil)
-        execute(sql, name = nil)
+        execute(sql, name)
         table = sql.split(" ", 4)[2]
         return id_value || last_insert_id(table, pk)
       end
 
+      def query(sql, name = nil)
+        log(sql, name) { @connection.query(sql) }
+      end
+
       def execute(sql, name = nil)
-        log(sql, name, @connection) { |connection| connection.query(sql) }
+        log(sql, name) { @connection.exec(sql) }
       end
 
       def update(sql, name = nil)
-        result = nil
-        log(sql, name, @connection) { |connection| result = connection.exec(sql) }
-        result.cmdtuples
+        execute(sql, name).cmdtuples
       end
 
       alias_method :delete, :update
@@ -122,9 +124,7 @@ module ActiveRecord
         end
 
         def select(sql, name = nil)
-          res = nil
-          log(sql, name, @connection) { |connection| res = connection.exec(sql) }
-  
+          res = execute(sql, name)
           results = res.result           
           rows = []
           if results.length > 0
@@ -169,23 +169,22 @@ module ActiveRecord
         def table_structure(table_name)
           database_name = @connection.db
           schema_name, table_name = split_table_schema(table_name)
-          
+
           # Grab a list of all the default values for the columns.
           sql =  "SELECT column_name, column_default, character_maximum_length, data_type "
           sql << "  FROM information_schema.columns "
           sql << " WHERE table_catalog = '#{database_name}' "
           sql << "   AND table_schema = '#{schema_name}' "
-          sql << "   AND table_name = '#{table_name}';"
+          sql << "   AND table_name = '#{table_name}'"
+          sql << " ORDER BY ordinal_position"
 
-          column_defaults = nil
-          log(sql, nil, @connection) { |connection| column_defaults = connection.query(sql) }
-          column_defaults.collect do |row|
-              field   = row[0]
-              type    = type_as_string(row[3], row[2])
-              default = default_value(row[1])
-              length  = row[2]
+          query(sql).collect do |row|
+            field   = row[0]
+            type    = type_as_string(row[3], row[2])
+            default = default_value(row[1])
+            length  = row[2]
 
-              [field, type, default, length]
+            [field, type, default, length]
           end
         end
 
diff --git a/activerecord/test/fixtures/db_definitions/postgresql.sql b/activerecord/test/fixtures/db_definitions/postgresql.sql
index 2fd210cb1b..6eb2304957 100644
--- a/activerecord/test/fixtures/db_definitions/postgresql.sql
+++ b/activerecord/test/fixtures/db_definitions/postgresql.sql
@@ -50,10 +50,10 @@ CREATE TABLE topics (
     bonus_time time,
     last_read date,
     content text,
+    approved smallint DEFAULT 1,
     replies_count integer default 0,
     parent_id integer,
     "type" character varying(50),
-    approved smallint DEFAULT 1,
     PRIMARY KEY (id)
 );
 SELECT setval('topics_id_seq', 100);
diff --git a/activerecord/test/reflection_test.rb b/activerecord/test/reflection_test.rb
index a892fb4acb..1658408d7a 100644
--- a/activerecord/test/reflection_test.rb
+++ b/activerecord/test/reflection_test.rb
@@ -22,6 +22,11 @@ class ReflectionTest < Test::Unit::TestCase
     assert_equal 12, Topic.columns.length
   end
 
+  def test_columns_are_returned_in_the_order_they_were_declared
+    column_names = Topic.columns.map { |column| column.name }
+    assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content approved replies_count parent_id type), column_names
+  end
+
   def test_content_columns
     assert_equal 8, Topic.content_columns.length
   end