Remove the default 255 char limit on string columns in Postgres #13435

Closed
wants to merge 1 commit into
from

Projects

None yet
@tmikoss

The underlying storage for varchar and text is the same in PG - defining column limits where application logic does not call for them only makes the database do unnecessary work.

See http://www.postgresql.org/docs/9.3/static/datatype-character.html for additional information.

@tmikoss

The CI build is failing on railities gem, but so is the current master. Seems to have started with https://travis-ci.org/rails/rails/builds/15798384

@robin850
Ruby on Rails member

Hello,

Thanks for your patch. Could you add a changelog entry please ?

@tmikoss

@robin850 changelog updated.

@seuros
Ruby on Rails member

@tmikoss IMO, it will be better to say : Removed default length in string columns in PostgreSQL.

@robin850
Ruby on Rails member

@seuros : I would rather say "on string columns" but 👍

@tmikoss : Could you update the changelog accordingly and squash your commits please ?

@egilburg

Many devs don't bother setting a specific limit, and unlimited string could cause performance issues, given that most input use cases rarely in fact go beyond that limit.

Perhaps it's better to default to 255 as a sensible default upper limit, unless user explicitly says otherwise.

@tmikoss

The thing is, a limited string is actually slower in PG than unlimited one, since the only difference between them is additional check whether given input fits within this limit :)

And while 255 does indeed cover most use cases, I've encountered enough cases where usually-around-50-symbols fields suddenly need to contain much more, because of some corner case in client business to warrant including a config/initializers file that changes this one setting in every new Rails + PG project I do.

Moreover, I think that limits on field lengths should be imposed as AR validators rather than DB constraints, given that the DB is primarily being written to by a Rails app. A validator will fail graciously and set the model to invalid state - something most Rails devs know how to handle, and most learning resources cover. A failed DB constraint results in exception being thrown, and that will turn in a "something went wrong" page in most apps I know. Therefore, until something like https://github.com/lomba/schema_validations gets merged into Rails core, you have to add length validators for every constrained string field in database to properly handle user input. And more importantly, you have to keep them both in sync over time.

So in absence of DB-side performance benefits, I think no limit is the better default. I have been successfully using such configuration in my own projects, and now I'd like to see it as default in Rails core.

@tmikoss tmikoss Removed default length on string columns in PostgreSQL
The underlying storage for varchar and text is the same in PG -
defining column limits where application logic does not call for them
only makes the database do unnecessary work. See
http://www.postgresql.org/docs/9.3/static/datatype-character.html
aeb7ab1
@dmathieu

I would provide a warning when someone does a migration without providing any limit on a string.

@asiniy

Like that? #9153

@tmikoss

I take that as sign of approval by public :D

@ebeigarts

+1 for no limit

@osegrums

makes sense to me. +1

@nishantmodak

Why leave sqlite3 alone! that can also be changed..

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb#L66

Reference:

  • http://www.sqlite.org/datatype3.html (Look for Section 2.2)

    Note that numeric arguments in parentheses that following the type name (ex: "VARCHAR(255)") are ignored by SQLite - SQLite does not impose any length restrictions (other than the large global SQLITE_MAX_LENGTH limit) on the length of strings, BLOBs or numeric values.

@veriel

+1, thanx.

@Envek

Moreover, current rails migrations are bugged with current behaviour.

If you change a string to have no limit by SQL and then execute db:schema:dump rake task you will get next entry:

t.string   "column", limit: nil

After recreating database with rake db:drop && rake db:create && rake db:schema:load you will get this column with 255 chars limit. :limit option ignored and db/schema.rb doesn't represent actual database schema anymore.

The same stays true for migrations, next migration will create column with 255 chars limit, it's discouraging, and this is a bug

class AddAwesomnessToTable < ActiveRecord::Migration
  def change
    add_column :table, :awesomness, :string, limit: nil
  end
end

This PR fixes it and in my opinion should be merged as soon as possible.

So, +1 for this. (Hope my voice will be heard by rails devs)

@PeterMozesMerl

It would be nice to have this in Rails 4.1. Otherwise we have to remove the limits manually from the migration or the database.

@senny senny added a commit to senny/rails that referenced this pull request Apr 4, 2014
@senny senny test, show current adapter behavior for `add_column limit: nil`.
This is an illustration of rails#13435 (comment)
Removing the limit from the PG and SQLite adapter solves the issue.
MySQL is still affected by the issue.
654f680
@senny senny added a commit to senny/rails that referenced this pull request Apr 4, 2014
@senny senny test, show current adapter behavior for `add_column limit: nil`.
This is an illustration of rails#13435 (comment)
Removing the limit from the PG and SQLite adapter solves the issue.
MySQL is still affected by the issue.
80f4a65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment