Merge PR #48608

activerecord/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Fully support `NULLS [NOT] DISTINCT` for PostgreSQL 15+ indexes.
+
+    Previous work was done to allow the index to be created in a migration, but it was not
+    supported in schema.rb. Additionally, the matching for `NULLS [NOT] DISTINCT` was not
+    in the correct order, which could have resulted in inconsistent schema detection.
+
+    *Gregory Jones*
+
 *   Allow escaping of literal colon characters in `sanitize_sql_*` methods when named bind variables are used
 
     *Justin Bull*

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

@@ -17,6 +17,7 @@ module ActiveRecord
         :options_include_default?, :supports_indexes_in_create?, :use_foreign_keys?,
         :quoted_columns_for_index, :supports_partial_index?, :supports_check_constraints?,
         :supports_index_include?, :supports_exclusion_constraints?, :supports_unique_keys?,
+        :supports_nulls_not_distinct?,
         to: :@conn, private: true
 
       private
@@ -110,6 +111,7 @@ module ActiveRecord
           sql << "USING #{index.using}" if supports_index_using? && index.using
           sql << "(#{quoted_columns(index)})"
           sql << "INCLUDE (#{quoted_include_columns(index.include)})" if supports_index_include? && index.include
+          sql << "NULLS NOT DISTINCT" if supports_nulls_not_distinct? && index.nulls_not_distinct
           sql << "WHERE #{index.where}" if supports_partial_index? && index.where
 
           sql.join(" ")

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

@@ -7,7 +7,7 @@ module ActiveRecord
     # this type are typically created and returned by methods in database
     # adapters. e.g. ActiveRecord::ConnectionAdapters::MySQL::SchemaStatements#indexes
     class IndexDefinition # :nodoc:
-      attr_reader :table, :name, :unique, :columns, :lengths, :orders, :opclasses, :where, :type, :using, :include, :comment, :valid
+      attr_reader :table, :name, :unique, :columns, :lengths, :orders, :opclasses, :where, :type, :using, :include, :nulls_not_distinct, :comment, :valid
 
       def initialize(
         table, name,
@@ -20,6 +20,7 @@ module ActiveRecord
         type: nil,
         using: nil,
         include: nil,
+        nulls_not_distinct: nil,
         comment: nil,
         valid: true
       )
@@ -34,6 +35,7 @@ module ActiveRecord
         @type = type
         @using = using
         @include = include
+        @nulls_not_distinct = nulls_not_distinct
         @comment = comment
         @valid = valid
       end
@@ -50,13 +52,14 @@ module ActiveRecord
         }
       end
 
-      def defined_for?(columns = nil, name: nil, unique: nil, valid: nil, include: nil, **options)
+      def defined_for?(columns = nil, name: nil, unique: nil, valid: nil, include: nil, nulls_not_distinct: nil, **options)
         columns = options[:column] if columns.blank?
         (columns.nil? || Array(self.columns) == Array(columns).map(&:to_s)) &&
           (name.nil? || self.name == name.to_s) &&
           (unique.nil? || self.unique == unique) &&
           (valid.nil? || self.valid == valid) &&
-          (include.nil? || Array(self.include) == Array(include).map(&:to_s))
+          (include.nil? || Array(self.include) == Array(include).map(&:to_s)) &&
+          (nulls_not_distinct.nil? || self.nulls_not_distinct == nulls_not_distinct)
       end
 
       private

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

@@ -1402,7 +1402,7 @@ module ActiveRecord
       end
 
       def add_index_options(table_name, column_name, name: nil, if_not_exists: false, internal: false, **options) # :nodoc:
-        options.assert_valid_keys(:unique, :length, :order, :opclass, :where, :type, :using, :comment, :algorithm, :include)
+        options.assert_valid_keys(:unique, :length, :order, :opclass, :where, :type, :using, :comment, :algorithm, :include, :nulls_not_distinct)
 
         column_names = index_column_names(column_name)
 
@@ -1422,6 +1422,7 @@ module ActiveRecord
           type: options[:type],
           using: options[:using],
           include: options[:include],
+          nulls_not_distinct: options[:nulls_not_distinct],
           comment: options[:comment]
         )
 

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -575,6 +575,10 @@ module ActiveRecord
         true
       end
 
+      def supports_nulls_not_distinct?
+        false
+      end
+
       def return_value_after_insert?(column) # :nodoc:
         column.auto_incremented_by_db?
       end

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

@@ -108,7 +108,7 @@ module ActiveRecord
             oid = row[4]
             comment = row[5]
             valid = row[6]
-            using, expressions, include, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: NULLS(?: NOT)? DISTINCT)?(?: INCLUDE \((.+?)\))?(?: WHERE (.+))?\z/m).flatten
+            using, expressions, include, nulls_not_distinct, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: INCLUDE \((.+?)\))?( NULLS NOT DISTINCT)?(?: WHERE (.+))?\z/m).flatten
 
             orders = {}
             opclasses = {}
@@ -149,6 +149,7 @@ module ActiveRecord
               where: where,
               using: using.to_sym,
               include: include_columns.presence,
+              nulls_not_distinct: nulls_not_distinct.present?,
               comment: comment.presence,
               valid: valid
             )

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -277,6 +277,10 @@ module ActiveRecord
         database_version >= 12_00_00 # >= 12.0
       end
 
+      def supports_nulls_not_distinct?
+        database_version >= 15_00_00 # >= 15.0
+      end
+
       def index_algorithms
         { concurrently: "CONCURRENTLY" }
       end

activerecord/lib/active_record/schema_dumper.rb

@@ -249,6 +249,7 @@ module ActiveRecord
         index_parts << "where: #{index.where.inspect}" if index.where
         index_parts << "using: #{index.using.inspect}" if !@connection.default_index_type?(index)
         index_parts << "include: #{index.include.inspect}" if index.include
+        index_parts << "nulls_not_distinct: #{index.nulls_not_distinct.inspect}" if index.nulls_not_distinct
         index_parts << "type: #{index.type.inspect}" if index.type
         index_parts << "comment: #{index.comment.inspect}" if index.comment
         index_parts

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

@@ -73,6 +73,9 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase
     expected = %(CREATE INDEX IF NOT EXISTS "index_people_on_last_name" ON "people" ("last_name"))
     assert_equal expected, add_index(:people, :last_name, if_not_exists: true)
 
+    expected = %(CREATE INDEX "index_people_on_last_name" ON "people" ("last_name") NULLS NOT DISTINCT)
+    assert_equal expected, add_index(:people, :last_name, nulls_not_distinct: true)
+
     assert_raise ArgumentError do
       add_index(:people, :last_name, algorithm: :copy)
     end

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

@@ -383,7 +383,7 @@ module ActiveRecord
       end
 
       def test_index_with_not_distinct_nulls
-        skip if ActiveRecord::Base.connection.database_version < 15_00_00
+        skip("current adapter doesn't support nulls not distinct") unless supports_nulls_not_distinct?
 
         with_example_table do
           @connection.execute(<<~SQL)

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

@@ -780,3 +780,46 @@ class SchemaIndexIncludeColumnsTest < ActiveRecord::PostgreSQLTestCase
     end
   end
 end
+
+class SchemaIndexNullsNotDistinctTest < ActiveRecord::PostgreSQLTestCase
+  include SchemaDumpingHelper
+
+  setup do
+    @connection = ActiveRecord::Base.connection
+    @connection.create_table "trains" do |t|
+      t.string :name
+    end
+  end
+
+  teardown do
+    @connection.drop_table "trains", if_exists: true
+  end
+
+  def test_nulls_not_distinct_is_dumped
+    skip("current adapter doesn't support nulls not distinct") unless supports_nulls_not_distinct?
+
+    @connection.execute "CREATE INDEX trains_name ON trains USING btree(name) NULLS NOT DISTINCT"
+
+    output = dump_table_schema "trains"
+
+    assert_match(/nulls_not_distinct: true/, output)
+  end
+
+  def test_nulls_distinct_is_dumped
+    skip("current adapter doesn't support nulls not distinct") unless supports_nulls_not_distinct?
+
+    @connection.execute "CREATE INDEX trains_name ON trains USING btree(name) NULLS DISTINCT"
+
+    output = dump_table_schema "trains"
+
+    assert_no_match(/nulls_not_distinct/, output)
+  end
+
+  def test_nulls_not_set_is_dumped
+    @connection.execute "CREATE INDEX trains_name ON trains USING btree(name)"
+
+    output = dump_table_schema "trains"
+
+    assert_no_match(/nulls_not_distinct/, output)
+  end
+end

activerecord/test/cases/migration/index_test.rb

@@ -299,6 +299,16 @@ module ActiveRecord
           connection.remove_index("testings", "last_name")
           assert_not connection.index_exists?("testings", "last_name", include: :foo, where: "first_name = 'john doe'")
         end
+
+        def test_add_index_with_nulls_not_distinct_assert_exists_with_same_values
+          connection.add_index("testings", "last_name", nulls_not_distinct: true)
+          assert connection.index_exists?("testings", "last_name", nulls_not_distinct: true)
+        end
+
+        def test_add_index_with_nulls_not_distinct_assert_exists_with_different_values
+          connection.add_index("testings", "last_name", nulls_not_distinct: false)
+          assert_not connection.index_exists?("testings", "last_name", nulls_not_distinct: true)
+        end
       end
 
       private

activerecord/test/cases/migration/invalid_options_test.rb

@@ -20,7 +20,7 @@ module ActiveRecord
       end
 
       def invalid_add_index_option_exception_message(key)
-        "Unknown key: :#{key}. Valid keys are: :unique, :length, :order, :opclass, :where, :type, :using, :comment, :algorithm, :include"
+        "Unknown key: :#{key}. Valid keys are: :unique, :length, :order, :opclass, :where, :type, :using, :comment, :algorithm, :include, :nulls_not_distinct"
       end
 
       def invalid_create_table_option_exception_message(key)

activerecord/test/cases/schema_dumper_test.rb

@@ -191,6 +191,15 @@ class SchemaDumperTest < ActiveRecord::TestCase
     end
   end
 
+  def test_schema_dumps_nulls_not_distinct
+    index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*company_nulls_not_distinct/).first.strip
+    if supports_nulls_not_distinct?
+      assert_equal 't.index ["firm_id"], name: "company_nulls_not_distinct", nulls_not_distinct: true', index_definition
+    else
+      assert_equal 't.index ["firm_id"], name: "company_nulls_not_distinct"', index_definition
+    end
+  end
+
   def test_schema_dumps_index_sort_order
     index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*_name_and_rating/).first.strip
     if ActiveRecord::Base.connection.supports_index_sort_order?

activerecord/test/schema/schema.rb

@@ -396,6 +396,7 @@ ActiveRecord::Schema.define do
     t.index [:name, :description], length: 10
     t.index [:firm_id, :type, :rating], name: "company_index", length: { type: 10 }, order: { rating: :desc }
     t.index [:firm_id, :type], name: "company_partial_index", where: "(rating > 10)"
+    t.index [:firm_id], name: "company_nulls_not_distinct", nulls_not_distinct: true
     t.index :name, name: "company_name_index", using: :btree
     t.index "(CASE WHEN rating > 0 THEN lower(name) END) DESC", name: "company_expression_index" if supports_expression_index?
   end

activerecord/test/support/adapter_helper.rb

@@ -55,6 +55,7 @@ module AdapterHelper
     supports_insert_conflict_target?
     supports_optimizer_hints?
     supports_datetime_with_precision?
+    supports_nulls_not_distinct?
   ].each do |method_name|
     define_method method_name do
       ActiveRecord::Base.connection.public_send(method_name)