db:schema:dump with PostgreSQLAdapter dumps multiple column indexes in undefined order

Tarmo, this bug seems to be severe, because it can really break indexes on database.

The patch works for me. Thank You.

Confirming:

$ ./script/dbconsole -p
# \d auth_tokens
                                     Table "public.auth_tokens"
   Column   |            Type             |                        Modifiers                         
------------+-----------------------------+----------------------------------------------------------
 id         | integer                     | not null default nextval('auth_tokens_id_seq'::regclass)
 user_id    | integer                     | not null
 token      | character varying(255)      | not null
 created_at | timestamp without time zone | not null
Indexes:
    "auth_tokens_pkey" PRIMARY KEY, btree (id)
    "index_auth_tokens_on_user_id_and_token" UNIQUE, btree (user_id, token)

$ rake db:schema:dump
(in /home/antage/workspace/avaxhome)
$ cat db/schema.rb | grep auth_tokens
  create_table "auth_tokens", :force => true do |t|
  add_index "auth_tokens", ["token", "user_id"], :name => "index_auth_tokens_on_user_id_and_token", :unique => true

Rails 2.3.2.1

PostgreSQL 8.3.7

(from [5a4603fafbf4d959ebc8f9435457eaf6fa4004e9]) Fixed dumping from postgresql columns in index in wrong order. [#2515 db:schema:dump with PostgreSQLAdapter dumps multiple column indexes in undefi... state:resolved]

Signed-off-by: Tarmo Tänav tarmo@itech.ee http://github.com/rails/rails/co...

It may be safer to collect up the attnum and sort by indkey. While IN appears to a left-to-right comparison, short-circuiting on true, I'm not sure that this isn't an implementation artifact. So, the fix as written may itself introduce a subtle ordering flaw. Having said that, I don't think I've ever seen an RDBMS implement it any other way even if it technically could.

Please consider using the patch I attached to #2400 [Patch] Correct index column order for PostgreSQL. I does sort by attribute order.

Could you explain the problem in more detail? The way I see it, it looks like indkey defines the column order, and column_names is sorted exactly the same way as indkey as it's just a .map of indkey, so what does the IN in the SQL have to do with any ordering?

Tarmo, your version gets the order right, but at the price of two queries and parsing in ruby. You can have it all in one query:

SELECT i.relname, d.indisunique, a.attname
FROM pg_class t, pg_class i, pg_index d, pg_attribute a,
generate_series(0, 9) AS c(i)
WHERE i.relkind = 'i'
AND d.indexrelid = i.oid
AND d.indisprimary = 'f'
AND t.oid = d.indrelid
AND t.relname = '{table_name}'
AND i.relnamespace IN (SELECT oid FROM pg_namespace WHERE nspname IN (#{schemas}) )
AND a.attrelid = t.oid
AND d.indkey[c.i]=a.attnum
ORDER BY i.relname, c.i

Though I do not have postgresql 7.4 to test and don't really know how well rails currently works with it, I do believe that generate_series is not by default present in 7.4. The overhead of the two queries is unlikely to be significant (especially in production mode where it matters since they will be executed only once).

Tarmo,

You are correct. I missed the mapping step afterwards and thought that the patch was attempting to fix this through SQL only. It would be hard to catch because I believe if you took the mapping portion out the test would still pass as an implementation detail. Anyway, that's neither here nor there. Thanks for setting the record straight.

Michael,

Another issue with using generate_series is that it reintroduces a problem with the original code that the applied patch fixes. Namely, you're imposing a hard 10 column limit on the size of the index. Going back at least as far as PostgreSQL 7.4, the default DB limit is 32 and could be compiled higher. The applied patch removes this artificial limitation.

Tarmo, Kevin: I agree, it's better not to hard code the number of index columns.

Have a look at the attached patch that gets rid of the second explicit SQL statement by reusing a slightly extended #column_definitions (it interprets an integer table name as an oid).

I'm thinking of extending the PostgreSQL adapter for expression indexes and partial indexes. To make that easier, I'd like the code to be as free of redundancies as possible.

That's what I get for not running the tests.

And finally without debug statements. Sorry for the mess.