Merge pull request rails#53735 from kamipo/not_valid_constraint_should_not_in_create_table

NOT VALID constraints should not dump in `create_table`

Author: kamipo

activerecord/lib/active_record/schema_dumper.rb

@@ -207,12 +207,17 @@ def table(table, stream)
           end
 
           indexes_in_create(table, tbl)
-          check_constraints_in_create(table, tbl) if @connection.supports_check_constraints?
+          remaining = check_constraints_in_create(table, tbl) if @connection.supports_check_constraints?
           exclusion_constraints_in_create(table, tbl) if @connection.supports_exclusion_constraints?
           unique_constraints_in_create(table, tbl) if @connection.supports_unique_constraints?
 
           tbl.puts "  end"
 
+          if remaining
+            tbl.puts
+            tbl.print remaining.string
+          end
+
           stream.print tbl.string
         rescue => e
           stream.puts "# Could not dump table #{table.inspect} because of following #{e.class}"
@@ -277,24 +282,37 @@ def index_parts(index)
 
       def check_constraints_in_create(table, stream)
         if (check_constraints = @connection.check_constraints(table)).any?
-          add_check_constraint_statements = check_constraints.map do |check_constraint|
-            parts = [
-              "t.check_constraint #{check_constraint.expression.inspect}"
-            ]
+          check_valid, check_invalid = check_constraints.partition { |chk| chk.validate? }
 
-            if check_constraint.export_name_on_schema_dump?
-              parts << "name: #{check_constraint.name.inspect}"
+          unless check_valid.empty?
+            check_constraint_statements = check_valid.map do |check|
+              "    t.check_constraint #{check_parts(check).join(', ')}"
             end
 
-            parts << "validate: #{check_constraint.validate?.inspect}" unless check_constraint.validate?
-
-            "    #{parts.join(', ')}"
+            stream.puts check_constraint_statements.sort.join("\n")
           end
 
-          stream.puts add_check_constraint_statements.sort.join("\n")
+          unless check_invalid.empty?
+            remaining = StringIO.new
+            table_name = remove_prefix_and_suffix(table).inspect
+
+            add_check_constraint_statements = check_invalid.map do |check|
+              "  add_check_constraint #{([table_name] + check_parts(check)).join(', ')}"
+            end
+
+            remaining.puts add_check_constraint_statements.sort.join("\n")
+            remaining
+          end
         end
       end
 
+      def check_parts(check)
+        check_parts = [ check.expression.inspect ]
+        check_parts << "name: #{check.name.inspect}" if check.export_name_on_schema_dump?
+        check_parts << "validate: #{check.validate?.inspect}" unless check.validate?
+        check_parts
+      end
+
       def foreign_keys(table, stream)
         if (foreign_keys = @connection.foreign_keys(table)).any?
           add_foreign_key_statements = foreign_keys.map do |foreign_key|

activerecord/test/cases/migration/check_constraint_test.rb

@@ -196,7 +196,7 @@ def test_schema_dumping_with_validate_false
 
             output = dump_table_schema "trades"
 
-            assert_match %r{\s+t.check_constraint "quantity > 0", name: "quantity_check", validate: false$}, output
+            assert_match %r{\s+add_check_constraint "trades", "quantity > 0", name: "quantity_check", validate: false$}, output
           end
 
           def test_schema_dumping_with_validate_true