Make changing fts and ai indexes work consistently in migrations (#7218)

Since only one of each kind is allowed, we need to make sure to drop
the old one before creating the new.

Also, reject having multiple fts/ai indexes at migration creation,
not just at migration application.

edb/schema/indexes.py

@@ -1178,6 +1178,7 @@ def validate_object(
         # The checks below apply only to concrete indexes.
         subject = referrer_ctx.scls
         assert isinstance(subject, (s_types.Type, s_pointers.Pointer))
+        assert isinstance(subject, IndexableSubject)
 
         if (
             is_object_scope_index(schema, self.scls)
@@ -1209,6 +1210,13 @@ def validate_object(
             assert abs_index.get_abstract(schema)
             root = abs_index.get_root(schema)
 
+            # For indexes that can only appear once per object, call
+            # get_effective_object_index for its side-effect of
+            # checking the error.
+            if is_exclusive_object_scope_index(schema, self.scls):
+                get_effective_object_index(
+                    schema, subject, root.get_name(schema), span=self.span)
+
             # Make sure that kwargs and parameters match in name and type.
             # Also make sure that all parameters have values at this point
             # (either default or provided in kwargs).
@@ -1606,6 +1614,7 @@ def get_effective_object_index(
     schema: s_schema.Schema,
     subject: IndexableSubject,
     base_idx_name: sn.QualName,
+    span: Optional[parsing.Span] = None,
 ) -> Tuple[Optional[Index], bool]:
     """
     Returns the effective index of a subject and a boolean indicating
@@ -1635,8 +1644,9 @@ def get_effective_object_index(
 
         if len(object_indexes_defined_here) > 1:
             subject_name = subject.get_displayname(schema)
-            raise errors.SchemaDefinitionError(
-                f'multiple {base_idx_name} indexes defined for {subject_name}'
+            raise errors.InvalidDefinitionError(
+                f'multiple {base_idx_name} indexes defined for {subject_name}',
+                span=span,
             )
         effective = object_indexes_defined_here[0]
         has_overridden = len(object_indexes) >= 2
@@ -1647,9 +1657,10 @@ def get_effective_object_index(
 
         if len(object_indexes) > 1:
             subject_name = subject.get_displayname(schema)
-            raise errors.SchemaDefinitionError(
+            raise errors.InvalidDefinitionError(
                 f'multiple {base_idx_name} indexes '
-                f'inherited for {subject_name}'
+                f'inherited for {subject_name}',
+                span=span,
             )
 
         effective = object_indexes[0]
@@ -1670,6 +1681,13 @@ def is_object_scope_index(
     )
 
 
+def is_exclusive_object_scope_index(
+    schema: s_schema.Schema,
+    index: Index,
+) -> bool:
+    return is_object_scope_index(schema, index)
+
+
 def is_fts_index(
     schema: s_schema.Schema,
     index: Index,

edb/schema/ordering.py

@@ -40,9 +40,11 @@
 from . import delta as sd
 from . import expraliases as s_expraliases
 from . import functions as s_func
+from . import indexes as s_indexes
 from . import inheriting
 from . import name as sn
 from . import objects as so
+from . import objtypes as s_objtypes
 from . import pointers as s_pointers
 from . import constraints as s_constraints
 from . import referencing
@@ -849,6 +851,26 @@ def write_ref_deps(
                 for old_func in old_funcs:
                     deps.add(('delete', str(old_func.get_name(old_schema))))
 
+            # Some index types only allow one per object type. Make
+            # sure we drop the old one before creating the new.
+            if (
+                isinstance(obj, s_indexes.Index)
+                and s_indexes.is_exclusive_object_scope_index(new_schema, obj)
+                and old_schema is not None
+                and (subject := obj.get_subject(new_schema))
+                and (old_subject := old_schema.get(
+                    subject.get_name(new_schema),
+                    type=s_objtypes.ObjectType,
+                    default=None
+                ))
+                and (eff_index := s_indexes.get_effective_object_index(
+                    old_schema,
+                    old_subject,
+                    obj.get_root(new_schema).get_name(new_schema),
+                )[0])
+            ):
+                deps.add(('delete', str(eff_index.get_name(old_schema))))
+
         if tag == 'alter':
             # Alteration must happen after creation, if any.
             deps.add(('create', this_name_str))

tests/test_edgeql_data_migration.py

@@ -12193,13 +12193,6 @@ async def test_edgeql_migration_ai_01(self):
             using extension ai;
         ''', explicit_modules=True)
 
-    @test.xerror('''
-        "multiple ext::ai::index indexes defined for default::Astronomy"
-
-        The first step, going from -small to -large works, but going
-        the other way fails. The different is probably alphabet
-        related.
-    ''')
     async def test_edgeql_migration_ai_02(self):
         await self.migrate('''
             using extension ai;

tests/test_schema.py

@@ -675,7 +675,7 @@ def test_schema_bad_type_16(self):
         errors.InvalidDefinitionError,
         "index 'fts::index' of object type 'test::Foo' " "was already declared",
     )
-    def test_schema_bad_type_17(self):
+    def test_schema_bad_type_17a(self):
         """
         type Foo {
             property val -> str;
@@ -691,6 +691,23 @@ def test_schema_bad_type_17(self):
         };
         """
 
+    @tb.must_fail(
+        errors.InvalidDefinitionError,
+        "multiple fts::index indexes defined for test::Foo",
+    )
+    def test_schema_bad_type_17b(self):
+        """
+        type Foo {
+            property val -> str;
+            index fts::index on (
+                fts::with_options(.val, language := fts::Language.eng)
+            );
+            index fts::index on (
+                fts::with_options(.val, language := fts::Language.ita)
+            );
+        };
+        """
+
     @tb.must_fail(
         errors.InvalidPropertyDefinitionError,
         "this type cannot be anonymous",