Enforce limit on table names length

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Enforce limit on table names length.
+
+    Fixes #45130.
+
+    *fatkodima*
+
 *   Adjust the minimum MariaDB version for check constraints support.
 
     *Eddie Lebow*

activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb

@@ -7,6 +7,11 @@ def max_identifier_length # :nodoc:
         64
       end
 
+      # Returns the maximum length of a table name.
+      def table_name_length
+        max_identifier_length
+      end
+
       # Returns the maximum length of a table alias.
       def table_alias_length
         max_identifier_length

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -290,6 +290,7 @@ def primary_key(table_name)
       #
       # See also TableDefinition#column for details on how to create columns.
       def create_table(table_name, id: :primary_key, primary_key: nil, force: nil, **options)
+        validate_table_length!(table_name) unless options[:_uses_legacy_table_name]
         td = create_table_definition(table_name, **extract_table_options!(options))
 
         if id && !td.as
@@ -491,7 +492,7 @@ def change_table(table_name, **options)
       #
       #   rename_table('octopuses', 'octopi')
       #
-      def rename_table(table_name, new_name)
+      def rename_table(table_name, new_name, **)
         raise NotImplementedError, "rename_table is not implemented"
       end
 
@@ -1585,6 +1586,12 @@ def validate_index_length!(table_name, new_name, internal = false)
           end
         end
 
+        def validate_table_length!(table_name)
+          if table_name.length > table_name_length
+            raise ArgumentError, "Table name '#{table_name}' is too long; the limit is #{table_name_length} characters"
+          end
+        end
+
         def extract_new_default_value(default_or_changes)
           if default_or_changes.is_a?(Hash) && default_or_changes.has_key?(:from) && default_or_changes.has_key?(:to)
             default_or_changes[:to]

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -309,7 +309,8 @@ def change_table_comment(table_name, comment_or_changes) # :nodoc:
       #
       # Example:
       #   rename_table('octopuses', 'octopi')
-      def rename_table(table_name, new_name)
+      def rename_table(table_name, new_name, **options)
+        validate_table_length!(new_name) unless options[:_uses_legacy_table_name]
         schema_cache.clear_data_source_cache!(table_name.to_s)
         schema_cache.clear_data_source_cache!(new_name.to_s)
         execute "RENAME TABLE #{quote_table_name(table_name)} TO #{quote_table_name(new_name)}"

activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb

@@ -377,7 +377,8 @@ def primary_keys(table_name) # :nodoc:
         #
         # Example:
         #   rename_table('octopuses', 'octopi')
-        def rename_table(table_name, new_name)
+        def rename_table(table_name, new_name, **options)
+          validate_table_length!(new_name) unless options[:_uses_legacy_table_name]
           clear_cache!
           schema_cache.clear_data_source_cache!(table_name.to_s)
           schema_cache.clear_data_source_cache!(new_name.to_s)

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -501,6 +501,14 @@ def max_identifier_length
         @max_identifier_length ||= query_value("SHOW max_identifier_length", "SCHEMA").to_i
       end
 
+      # Returns the maximum length of a table name.
+      def table_name_length
+        # PostgreSQL automatically creates an index for PRIMARY KEY with name consisting of
+        # truncated table name and "_pkey" suffix fitting into max_identifier_length number of characters.
+        # We allow smaller table names to be able to correctly rename this index when renaming the table.
+        max_identifier_length - "_pkey".length
+      end
+
       # Set the authorized user for this session
       def session_auth=(user)
         clear_cache!

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -243,7 +243,8 @@ def remove_index(table_name, column_name = nil, **options) # :nodoc:
       #
       # Example:
       #   rename_table('octopuses', 'octopi')
-      def rename_table(table_name, new_name)
+      def rename_table(table_name, new_name, **options)
+        validate_table_length!(new_name) unless options[:_uses_legacy_table_name]
         schema_cache.clear_data_source_cache!(table_name.to_s)
         schema_cache.clear_data_source_cache!(new_name.to_s)
         exec_query "ALTER TABLE #{quote_table_name(table_name)} RENAME TO #{quote_table_name(new_name)}"

activerecord/lib/active_record/migration/compatibility.rb

@@ -40,6 +40,7 @@ def raise_on_if_exist_options(options)
         end
 
         def create_table(table_name, **options)
+          options[:_uses_legacy_table_name] = true
           if block_given?
             super { |t| yield compatible_table_definition(t) }
           else
@@ -55,6 +56,11 @@ def change_table(table_name, **options)
           end
         end
 
+        def rename_table(table_name, new_name, **options)
+          options[:_uses_legacy_table_name] = true
+          super
+        end
+
         private
           def compatible_table_definition(t)
             class << t

activerecord/test/cases/adapters/postgresql/serial_test.rb

@@ -128,7 +128,7 @@ class LongerSequenceNameDetectionTest < ActiveRecord::PostgreSQLTestCase
     def setup
       @table_name = "long_table_name_to_test_sequence_name_detection_for_serial_cols"
       @connection = ActiveRecord::Base.connection
-      @connection.create_table @table_name, force: true do |t|
+      @connection.create_table @table_name, force: true, _uses_legacy_table_name: true do |t|
         t.serial :seq
         t.bigserial :bigseq
       end

activerecord/test/cases/migration/compatibility_test.rb

@@ -531,6 +531,67 @@ def migrate(x)
         connection.drop_table :more_testings rescue nil
       end
 
+      def test_create_table_on_7_0
+        long_table_name = "a" * (connection.table_name_length + 1)
+        migration = Class.new(ActiveRecord::Migration[7.0]) {
+          @@long_table_name = long_table_name
+          def version; 100 end
+          def migrate(x)
+            create_table @@long_table_name
+          end
+        }.new
+
+        if current_adapter?(:Mysql2Adapter)
+          # MySQL does not allow to create table names longer than limit
+          error = assert_raises(StandardError) do
+            ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+          end
+          if connection.mariadb?
+            assert_match(/Incorrect table name '#{long_table_name}'/i, error.message)
+          else
+            assert_match(/Identifier name '#{long_table_name}' is too long/i, error.message)
+          end
+        else
+          ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+          assert connection.table_exists?(long_table_name)
+        end
+      ensure
+        connection.drop_table(long_table_name) rescue nil
+      end
+
+      def test_rename_table_on_7_0
+        long_table_name = "a" * (connection.table_name_length + 1)
+        connection.create_table(:more_testings)
+
+        migration = Class.new(ActiveRecord::Migration[7.0]) {
+          @@long_table_name = long_table_name
+          def version; 100 end
+          def migrate(x)
+            rename_table :more_testings, @@long_table_name
+          end
+        }.new
+
+        if current_adapter?(:Mysql2Adapter)
+          # MySQL does not allow to create table names longer than limit
+          error = assert_raises(StandardError) do
+            ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+          end
+          if connection.mariadb?
+            assert_match(/Incorrect table name '#{long_table_name}'/i, error.message)
+          else
+            assert_match(/Identifier name '#{long_table_name}' is too long/i, error.message)
+          end
+        else
+          ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+          assert connection.table_exists?(long_table_name)
+          assert_not connection.table_exists?(:more_testings)
+          assert connection.table_exists?(long_table_name)
+        end
+      ensure
+        connection.drop_table(:more_testings) rescue nil
+        connection.drop_table(long_table_name) rescue nil
+      end
+
       private
         def precision_implicit_default
           if current_adapter?(:Mysql2Adapter)

activerecord/test/cases/migration/rename_table_test.rb

@@ -50,6 +50,22 @@ def test_rename_table
         assert_equal "http://www.foreverflying.com/octopus-black7.jpg", connection.select_value("SELECT url FROM octopi WHERE id=1")
       end
 
+      def test_rename_table_raises_for_long_table_names
+        name_limit = connection.table_name_length
+        long_name = "a" * (name_limit + 1)
+        short_name = "a" * name_limit
+
+        error = assert_raises(ArgumentError) do
+          connection.rename_table :test_models, long_name
+        end
+        assert_equal "Table name '#{long_name}' is too long; the limit is #{name_limit} characters", error.message
+
+        connection.rename_table :test_models, short_name
+        assert connection.table_exists?(short_name)
+      ensure
+        connection.drop_table short_name, if_exists: true
+      end
+
       def test_rename_table_with_an_index
         add_index :test_models, :url
 

activerecord/test/cases/migration_test.rb

@@ -178,6 +178,23 @@ def test_create_table_with_if_not_exists_true
     connection.drop_table :testings, if_exists: true
   end
 
+  def test_create_table_raises_for_long_table_names
+    connection = Person.connection
+    name_limit = connection.table_name_length
+    long_name = "a" * (name_limit + 1)
+    short_name = "a" * name_limit
+
+    error = assert_raises(ArgumentError) do
+      connection.create_table(long_name)
+    end
+    assert_equal "Table name '#{long_name}' is too long; the limit is #{name_limit} characters", error.message
+
+    connection.create_table(short_name)
+    assert connection.table_exists?(short_name)
+  ensure
+    connection.drop_table short_name, if_exists: true
+  end
+
   def test_create_table_with_indexes_and_if_not_exists_true
     connection = Person.connection
     connection.create_table :testings, force: true do |t|