Rename column with long name and index result in postgres error #12585

Closed
bobbus opened this Issue Oct 18, 2013 · 8 comments

Projects

None yet

4 participants

@bobbus
Contributor
bobbus commented Oct 18, 2013

In Rails 4, an error is raise when we rename a column with an index to a too long name.

Error stack :

Input string is longer than NAMEDATALEN-1 (63)/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/connection_adapters/postgresql/quoting.rb:152:in `quote_ident'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/connection_adapters/postgresql/quoting.rb:152:in `quote_column_name'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/connection_adapters/postgresql/quoting.rb:139:in `quote_table_name'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/connection_adapters/postgresql/schema_statements.rb:424:in `rename_index'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/connection_adapters/abstract/schema_statements.rb:854:in `block in rename_column_indexes'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/connection_adapters/abstract/schema_statements.rb:848:in `each'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/connection_adapters/abstract/schema_statements.rb:848:in `rename_column_indexes'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/connection_adapters/postgresql/schema_statements.rb:411:in `rename_column'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/migration.rb:625:in `block in method_missing'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/migration.rb:597:in `block in say_with_time'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/1.9.1/benchmark.rb:280:in `measure'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/migration.rb:597:in `say_with_time'
/home/bob/.rbenv/versions/1.9.3-p429/lib/ruby/gems/1.9.1/gems/activerecord-4.0.0/lib/active_record/migration.rb:617:in `method_missing'
/home/bob/doc/dev/test-apps/testing4/db/migrate/20131018132332_rename_table.rb:3:in `change'

An app with the failure could be found here, relevant commit is : bobbus/testing-rails4@4296e5d

@laurocaetano
Contributor

The problem is that rails is trying to create an index for this new column, and it's named index_table_with_long_names_on_field_with_too_too_too_too_too_long_name.

This will raise Input string is longer than NAMEDATALEN-1 (63). You can find more about it here: http://rubydoc.info/github/ged/ruby-pg/PGconn.quote_ident

It's not a Rails issue. Thanks.

@bobbus
Contributor
bobbus commented Oct 21, 2013

Thanks for these informations @laurocaetano . Actually, a workaround could be to rescue the line which rename the column but this is not really elegant and I would think it could be good to add a way to prevent this in Rails ?

@senny
Member
senny commented Oct 22, 2013

@bobbus I'm not sure we can prevent this problem completely. If you have to work with very long table / field names It would be best to specify the index names. This way Rails won't rename them when you rename a field. Also you can pick shorter index names as Rails would.

EDIT: This is not really a problem of rename_column. You'll run into the same issue when you try to call add_index for a long table and field name.

@robinbortlik

I got same problem, so I monkey patched index_name method like this

module ActiveRecord
  module ConnectionAdapters
    module SchemaStatements

      def index_name_with_length(table_name, options)
        name = index_name_without_length(table_name, options)
        name = name[0..63] if name.length > 64
        name
      end
      alias_method_chain :index_name, :length

    end
  end
end
@laurocaetano
Contributor

Closing this one, as commented in #13209 (comment)

Thanks guys.

@bobbus
Contributor
bobbus commented Mar 12, 2014

@laurocaetano , I can understand that we do not want the fix in #13209 in Rails but I believe a better error message or a warning would be useful.

The actual error message is a bit brittle, don't you think ?

Also, I'm wondering which fix you would recommend in the case of rename_column ? It could be renaming the index before renaming the column but it seems odd ? Actually, I use a rescue clause on the rename_column line for this error, it is not really elegant..

@laurocaetano
Contributor

@bobbus I agree that the error message is a bit brittle.

We could work on improving the error message instead of preventing the error, what do you think? cc @rafaelfranca

@senny
Member
senny commented Mar 25, 2014

@bobbus what is the full error you get? If possible I'd like to forward the error from the database to the user. It signals that it's not a Rails limitation but something related to the DB backend. Of course that only works if the error links to the cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment