From f4226c3ab6651f6871e02f3c6754c29ab155b938 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Thu, 3 Apr 2014 14:59:53 +0200 Subject: [PATCH 1/2] PostgreSQL and SQLite, remove varchar limit. [Vladimir Sazhin & Toms Mikoss & Yves Senn] There is no reason for the PG adapter to have a default limit of 255 on :string columns. See this snippet from the PG docs: Tip: There is no performance difference among these three types, apart from increased storage space when using the blank-padded type, and a few extra CPU cycles to check the length when storing into a length-constrained column. While character(n) has performance advantages in some other database systems, there is no such advantage in PostgreSQL; in fact character(n) is usually the slowest of the three because of its additional storage costs. In most situations text or character varying should be used instead. --- activerecord/CHANGELOG.md | 6 ++++++ .../active_record/connection_adapters/postgresql_adapter.rb | 2 +- .../active_record/connection_adapters/sqlite3_adapter.rb | 2 +- activerecord/test/cases/adapters/postgresql/array_test.rb | 2 +- activerecord/test/cases/reflection_test.rb | 2 +- activerecord/test/schema/schema.rb | 2 +- 6 files changed, 11 insertions(+), 5 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 98fe0fbd62..dce2692b0d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* PostgreSQL and SQLite string columns no longer have a default limit of 255. + + Fixes #13435, #9153. + + *Vladimir Sazhin*, *Toms Mikoss*, *Yves Senn* + * Block a few default Class methods as scope name. For instance, this will raise: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 9fe8e0497e..3510e4f3b0 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -209,7 +209,7 @@ module ActiveRecord NATIVE_DATABASE_TYPES = { primary_key: "serial primary key", - string: { name: "character varying", limit: 255 }, + string: { name: "character varying" }, text: { name: "text" }, integer: { name: "integer" }, float: { name: "float" }, diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 6e6a51dab8..cd1f7a16c6 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -63,7 +63,7 @@ module ActiveRecord NATIVE_DATABASE_TYPES = { primary_key: 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL', - string: { name: "varchar", limit: 255 }, + string: { name: "varchar" }, text: { name: "text" }, integer: { name: "integer" }, float: { name: "float" }, diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 36ded66998..18dd4a6de8 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -25,7 +25,7 @@ class PostgresqlArrayTest < ActiveRecord::TestCase def test_column assert_equal :string, @column.type - assert_equal "character varying(255)", @column.sql_type + assert_equal "character varying", @column.sql_type assert @column.array assert_not @column.text? assert_not @column.number? diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index d7ad5ed29f..ad77472333 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -63,7 +63,7 @@ class ReflectionTest < ActiveRecord::TestCase def test_column_string_type_and_limit assert_equal :string, @first.column_for_attribute("title").type - assert_equal 255, @first.column_for_attribute("title").limit + assert_equal 250, @first.column_for_attribute("title").limit end def test_column_null_not_null diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index a9c4980283..d79cce8cca 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -673,7 +673,7 @@ ActiveRecord::Schema.define do end create_table :topics, force: true do |t| - t.string :title + t.string :title, limit: 250 t.string :author_name t.string :author_email_address if mysql_56? From 80f4a65bbd2372c01cfdf174e6446cd232d81e44 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Fri, 4 Apr 2014 09:16:02 +0200 Subject: [PATCH 2/2] test, show current adapter behavior for `add_column limit: nil`. This is an illustration of https://github.com/rails/rails/pull/13435#issuecomment-33789752 Removing the limit from the PG and SQLite adapter solves the issue. MySQL is still affected by the issue. --- .../test/cases/migration/column_attributes_test.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/activerecord/test/cases/migration/column_attributes_test.rb b/activerecord/test/cases/migration/column_attributes_test.rb index ccf19fb4d0..6a02873cba 100644 --- a/activerecord/test/cases/migration/column_attributes_test.rb +++ b/activerecord/test/cases/migration/column_attributes_test.rb @@ -35,6 +35,14 @@ module ActiveRecord assert_no_column TestModel, :last_name end + def test_add_column_without_limit + # TODO: limit: nil should work with all adapters. + skip "MySQL wrongly enforces a limit of 255" if current_adapter?(:MysqlAdapter, :Mysql2Adapter) + add_column :test_models, :description, :string, limit: nil + TestModel.reset_column_information + assert_nil TestModel.columns_hash["description"].limit + end + if current_adapter?(:MysqlAdapter, :Mysql2Adapter) def test_unabstracted_database_dependent_types add_column :test_models, :intelligence_quotient, :tinyint