Merge pull request rails#45772 from adrianna-chang-shopify/ac-remove-ddl-from-schema-definitions

Don't store `ddl` on schema definitions

Author: eileencodes

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

@@ -26,7 +26,6 @@ def visit_AlterTable(o)
           sql << o.foreign_key_drops.map { |fk| visit_DropForeignKey fk }.join(" ")
           sql << o.check_constraint_adds.map { |con| visit_AddCheckConstraint con }.join(" ")
           sql << o.check_constraint_drops.map { |con| visit_DropCheckConstraint con }.join(" ")
-          o.ddl = sql
         end
 
         def visit_ColumnDefinition(o)
@@ -67,7 +66,7 @@ def visit_TableDefinition(o)
           create_sql << "(#{statements.join(', ')})" if statements.present?
           add_table_options!(create_sql, o)
           create_sql << " AS #{to_sql(o.as)}" if o.as
-          o.ddl = create_sql
+          create_sql
         end
 
         def visit_PrimaryKeyDefinition(o)
@@ -107,8 +106,7 @@ def visit_CreateIndexDefinition(o)
           sql << "(#{quoted_columns(index)})"
           sql << "WHERE #{index.where}" if supports_partial_index? && index.where
 
-          sql = sql.join(" ")
-          o.ddl = sql
+          sql.join(" ")
         end
 
         def visit_CheckConstraintDefinition(o)

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

@@ -92,11 +92,11 @@ def aliased_types(name, fallback)
 
     AddColumnDefinition = Struct.new(:column) # :nodoc:
 
-    ChangeColumnDefinition = Struct.new(:column, :name, :ddl) # :nodoc:
+    ChangeColumnDefinition = Struct.new(:column, :name) # :nodoc:
 
-    ChangeColumnDefaultDefinition = Struct.new(:column, :default, :ddl) # :nodoc:
+    ChangeColumnDefaultDefinition = Struct.new(:column, :default) # :nodoc:
 
-    CreateIndexDefinition = Struct.new(:index, :algorithm, :if_not_exists, :ddl) # :nodoc:
+    CreateIndexDefinition = Struct.new(:index, :algorithm, :if_not_exists) # :nodoc:
 
     PrimaryKeyDefinition = Struct.new(:name) # :nodoc:
 
@@ -335,7 +335,6 @@ class TableDefinition
       include ColumnMethods
 
       attr_reader :name, :temporary, :if_not_exists, :options, :as, :comment, :indexes, :foreign_keys, :check_constraints
-      attr_accessor :ddl
 
       def initialize(
         conn,
@@ -582,7 +581,6 @@ class AlterTable # :nodoc:
       attr_reader :adds
       attr_reader :foreign_key_adds, :foreign_key_drops
       attr_reader :check_constraint_adds, :check_constraint_drops
-      attr_accessor :ddl
 
       def initialize(td)
         @td   = td

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

@@ -299,7 +299,7 @@ def create_table(table_name, id: :primary_key, primary_key: nil, force: nil, **o
           schema_cache.clear_data_source_cache!(table_name.to_s)
         end
 
-        result = execute(td.ddl)
+        result = execute schema_creation.accept(td)
 
         unless supports_indexes_in_create?
           td.indexes.each do |column_name, index_options|
@@ -329,7 +329,6 @@ def build_create_table_definition(table_name, id: :primary_key, primary_key: nil
 
         yield table_definition if block_given?
 
-        schema_creation.accept(table_definition)
         table_definition
       end
 
@@ -620,7 +619,7 @@ def add_column(table_name, column_name, type, **options)
         add_column_def = build_add_column_definition(table_name, column_name, type, **options)
         return unless add_column_def
 
-        execute(add_column_def.ddl)
+        execute schema_creation.accept(add_column_def)
       end
 
       def add_columns(table_name, *column_names, type:, **options) # :nodoc:
@@ -645,7 +644,6 @@ def build_add_column_definition(table_name, column_name, type, **options) # :nod
 
         alter_table = create_alter_table(table_name)
         alter_table.add_column(column_name, type, **options)
-        schema_creation.accept(alter_table)
         alter_table
       end
 
@@ -875,7 +873,7 @@ def rename_column(table_name, column_name, new_column_name)
       # For more information see the {"Transactional Migrations" section}[rdoc-ref:Migration].
       def add_index(table_name, column_name, **options)
         create_index = build_create_index_definition(table_name, column_name, **options)
-        execute(create_index.ddl)
+        execute schema_creation.accept(create_index)
       end
 
       # Builds a CreateIndexDefinition object.
@@ -885,10 +883,7 @@ def add_index(table_name, column_name, **options)
       # passing a +table_name+, +column_name+, and other additional options that can be passed.
       def build_create_index_definition(table_name, column_name, **options) # :nodoc:
         index, algorithm, if_not_exists = add_index_options(table_name, column_name, **options)
-
-        create_index = CreateIndexDefinition.new(index, algorithm, if_not_exists)
-        schema_creation.accept(create_index)
-        create_index
+        CreateIndexDefinition.new(index, algorithm, if_not_exists)
       end
 
       # Removes the given index from the table.
@@ -1715,7 +1710,7 @@ def add_column_for_alter(table_name, column_name, type, **options)
 
         def change_column_default_for_alter(table_name, column_name, default_or_changes)
           cd = build_change_column_default_definition(table_name, column_name, default_or_changes)
-          cd.ddl
+          schema_creation.accept(cd)
         end
 
         def rename_column_sql(table_name, column_name, new_column_name)

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -355,10 +355,7 @@ def build_change_column_default_definition(table_name, column_name, default_or_c
         return unless column
 
         default = extract_new_default_value(default_or_changes)
-        change_column_default_definition = ChangeColumnDefaultDefinition.new(column, default)
-        schema_creation.accept(change_column_default_definition)
-
-        change_column_default_definition
+        ChangeColumnDefaultDefinition.new(column, default)
       end
 
       def change_column_null(table_name, column_name, null, default = nil) # :nodoc:
@@ -411,10 +408,7 @@ def build_change_column_definition(table_name, column_name, type, **options) # :
 
         td = create_table_definition(table_name)
         cd = td.new_column_definition(column.name, type, **options)
-        change_column_def = ChangeColumnDefinition.new(cd, column.name)
-        schema_creation.accept(change_column_def)
-
-        change_column_def
+        ChangeColumnDefinition.new(cd, column.name)
       end
 
       def rename_column(table_name, column_name, new_column_name) # :nodoc:
@@ -426,17 +420,15 @@ def add_index(table_name, column_name, **options) # :nodoc:
         create_index = build_create_index_definition(table_name, column_name, **options)
         return unless create_index
 
-        execute(create_index.ddl)
+        execute schema_creation.accept(create_index)
       end
 
       def build_create_index_definition(table_name, column_name, **options) # :nodoc:
         index, algorithm, if_not_exists = add_index_options(table_name, column_name, **options)
 
         return if if_not_exists && index_exists?(table_name, column_name, name: index.name)
 
-        create_index = CreateIndexDefinition.new(index, algorithm)
-        schema_creation.accept(create_index)
-        create_index
+        CreateIndexDefinition.new(index, algorithm)
       end
 
       def add_sql_comment!(sql, comment) # :nodoc:
@@ -782,7 +774,7 @@ def translate_exception(exception, message:, sql:, binds:)
 
         def change_column_for_alter(table_name, column_name, type, **options)
           cd = build_change_column_definition(table_name, column_name, type, **options)
-          cd.ddl
+          schema_creation.accept(cd)
         end
 
         def rename_column_for_alter(table_name, column_name, new_column_name)

activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb

@@ -21,7 +21,7 @@ def visit_AddColumnDefinition(o)
 
           def visit_ChangeColumnDefinition(o)
             change_column_sql = +"CHANGE #{quote_column_name(o.name)} #{accept(o.column)}"
-            o.ddl = add_column_position!(change_column_sql, column_options(o.column))
+            add_column_position!(change_column_sql, column_options(o.column))
           end
 
           def visit_ChangeColumnDefaultDefinition(o)
@@ -31,13 +31,12 @@ def visit_ChangeColumnDefaultDefinition(o)
             else
               sql << quote_default_expression(o.default, o.column)
             end
-            o.ddl = sql
           end
 
           def visit_CreateIndexDefinition(o)
             sql = visit_IndexDefinition(o.index, true)
             sql << " #{o.algorithm}" if o.algorithm
-            o.ddl = sql
+            sql
           end
 
           def visit_IndexDefinition(o, create = false)

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

@@ -10,7 +10,6 @@ def visit_AlterTable(o)
             sql << o.constraint_validations.map { |fk| visit_ValidateConstraint fk }.join(" ")
             sql << o.exclusion_constraint_adds.map { |con| visit_AddExclusionConstraint con }.join(" ")
             sql << o.exclusion_constraint_drops.map { |con| visit_DropExclusionConstraint con }.join(" ")
-            o.ddl = sql
           end
 
           def visit_AddForeignKey(o)
@@ -84,7 +83,7 @@ def visit_ChangeColumnDefinition(o)
               change_column_sql << ", ALTER COLUMN #{quoted_column_name} #{options[:null] ? 'DROP' : 'SET'} NOT NULL"
             end
 
-            o.ddl = change_column_sql
+            change_column_sql
           end
 
           def visit_ChangeColumnDefaultDefinition(o)
@@ -94,7 +93,6 @@ def visit_ChangeColumnDefaultDefinition(o)
             else
               sql << "SET DEFAULT #{quote_default_expression(o.default, o.column)}"
             end
-            o.ddl = sql
           end
 
           def add_column_options!(sql, options)

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

@@ -417,10 +417,7 @@ def change_column(table_name, column_name, type, **options) # :nodoc:
         def build_change_column_definition(table_name, column_name, type, **options) # :nodoc:
           td = create_table_definition(table_name)
           cd = td.new_column_definition(column_name, type, **options)
-          change_column_def = ChangeColumnDefinition.new(cd, column_name)
-          schema_creation.accept(change_column_def)
-
-          change_column_def
+          ChangeColumnDefinition.new(cd, column_name)
         end
 
         # Changes the default value of a table column.
@@ -433,10 +430,7 @@ def build_change_column_default_definition(table_name, column_name, default_or_c
           return unless column
 
           default = extract_new_default_value(default_or_changes)
-          change_column_default_definition = ChangeColumnDefaultDefinition.new(column, default)
-          schema_creation.accept(change_column_default_definition)
-
-          change_column_default_definition
+          ChangeColumnDefaultDefinition.new(column, default)
         end
 
         def change_column_null(table_name, column_name, null, default = nil) # :nodoc:
@@ -473,7 +467,7 @@ def rename_column(table_name, column_name, new_column_name) # :nodoc:
 
         def add_index(table_name, column_name, **options) # :nodoc:
           create_index = build_create_index_definition(table_name, column_name, **options)
-          result = execute(create_index.ddl)
+          result = execute schema_creation.accept(create_index)
 
           index = create_index.index
           execute "COMMENT ON INDEX #{quote_column_name(index.name)} IS #{quote(index.comment)}" if index.comment
@@ -482,10 +476,7 @@ def add_index(table_name, column_name, **options) # :nodoc:
 
         def build_create_index_definition(table_name, column_name, **options) # :nodoc:
           index, algorithm, if_not_exists = add_index_options(table_name, column_name, **options)
-
-          create_index = CreateIndexDefinition.new(index, algorithm, if_not_exists)
-          schema_creation.accept(create_index)
-          create_index
+          CreateIndexDefinition.new(index, algorithm, if_not_exists)
         end
 
         def remove_index(table_name, column_name = nil, **options) # :nodoc:
@@ -861,7 +852,7 @@ def add_column_for_alter(table_name, column_name, type, **options)
 
           def change_column_for_alter(table_name, column_name, type, **options)
             change_col_def = build_change_column_definition(table_name, column_name, type, **options)
-            sqls = [change_col_def.ddl]
+            sqls = [schema_creation.accept(change_col_def)]
             sqls << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment)
             sqls
           end

activerecord/test/cases/migration/schema_definitions_test.rb

@@ -18,22 +18,16 @@ def test_build_create_table_definition_with_block
 
         id_column = td.columns.find { |col| col.name == "id" }
         assert_predicate id_column, :present?
-        assert id_column.type
-        assert id_column.sql_type
 
         foo_column = td.columns.find { |col| col.name == "foo" }
         assert_predicate foo_column, :present?
-        assert foo_column.type
-        assert foo_column.sql_type
       end
 
       def test_build_create_table_definition_without_block
         td = connection.build_create_table_definition(:test)
 
         id_column = td.columns.find { |col| col.name == "id" }
         assert_predicate id_column, :present?
-        assert id_column.type
-        assert id_column.sql_type
       end
 
       def test_build_create_join_table_definition_with_block
@@ -64,7 +58,6 @@ def test_build_create_index_definition
         end
         create_index = connection.build_create_index_definition(:test, :foo)
 
-        assert_match "CREATE INDEX", create_index.ddl
         assert_equal "index_test_on_foo", create_index.index.name
       ensure
         connection.drop_table(:test) if connection.table_exists?(:test)
@@ -91,12 +84,8 @@ def test_build_change_column_definition
           end
 
           change_cd = connection.build_change_column_definition(:test, :foo, :integer)
-          assert change_cd.ddl
-
           change_col = change_cd.column
           assert_equal "foo", change_col.name.to_s
-          assert change_col.type
-          assert change_col.sql_type
         ensure
           connection.drop_table(:test) if connection.table_exists?(:test)
         end
@@ -107,13 +96,10 @@ def test_build_change_column_default_definition
           end
 
           change_default_cd = connection.build_change_column_default_definition(:test, :foo, "new")
-          assert_match "SET DEFAULT 'new'", change_default_cd.ddl
           assert_equal "new", change_default_cd.default
 
           change_col = change_default_cd.column
           assert_equal "foo", change_col.name.to_s
-          assert change_col.type
-          assert change_col.sql_type
         ensure
           connection.drop_table(:test) if connection.table_exists?(:test)
         end