fix: Maintain indexes across schema versions (#3367)

## Relevant issue(s)

Resolves #3365

## Description

Completes the fix started in
#3366 - I missed a test
case there and only tested with docs created after the patch. This adds
a new test, testing with docs created before the patch, and then updates
all index related references from col.ID to col.RootID.

internal/db/collection_index.go

@@ -414,7 +414,7 @@ func (c *collection) dropIndex(ctx context.Context, indexName string) error {
 			break
 		}
 	}
-	key := keys.NewCollectionIndexKey(immutable.Some(c.ID()), indexName)
+	key := keys.NewCollectionIndexKey(immutable.Some(c.Description().RootID), indexName)
 	err = txn.Systemstore().Delete(ctx, key.ToDS())
 	if err != nil {
 		return err
@@ -426,7 +426,7 @@ func (c *collection) dropIndex(ctx context.Context, indexName string) error {
 func (c *collection) dropAllIndexes(ctx context.Context) error {
 	// callers of this function must set a context transaction
 	txn := mustGetContextTxn(ctx)
-	prefix := keys.NewCollectionIndexKey(immutable.Some(c.ID()), "")
+	prefix := keys.NewCollectionIndexKey(immutable.Some(c.Description().RootID), "")
 
 	keys, err := datastore.FetchKeysForPrefix(ctx, prefix.ToString(), txn.Systemstore())
 	if err != nil {
@@ -549,7 +549,7 @@ func generateIndexName(col client.Collection, fields []client.IndexedFieldDescri
 	if col.Name().HasValue() {
 		sb.WriteString(col.Name().Value())
 	} else {
-		sb.WriteString(fmt.Sprint(col.ID()))
+		sb.WriteString(fmt.Sprint(col.Description().RootID))
 	}
 	sb.WriteByte('_')
 	// we can safely assume that there is at least one field in the slice

internal/db/fetcher/indexer_iterators.go

@@ -396,7 +396,7 @@ func (f *IndexFetcher) newInIndexIterator(
 }
 
 func (f *IndexFetcher) newIndexDataStoreKey() keys.IndexDataStoreKey {
-	return keys.IndexDataStoreKey{CollectionID: f.col.ID(), IndexID: f.indexDesc.ID}
+	return keys.IndexDataStoreKey{CollectionID: f.col.Description().RootID, IndexID: f.indexDesc.ID}
 }
 
 func (f *IndexFetcher) newIndexDataStoreKeyWithValues(values []client.NormalValue) keys.IndexDataStoreKey {
@@ -405,7 +405,7 @@ func (f *IndexFetcher) newIndexDataStoreKeyWithValues(values []client.NormalValu
 		fields[i].Value = values[i]
 		fields[i].Descending = f.indexDesc.Fields[i].Descending
 	}
-	return keys.NewIndexDataStoreKey(f.col.ID(), f.indexDesc.ID, fields)
+	return keys.NewIndexDataStoreKey(f.col.Description().RootID, f.indexDesc.ID, fields)
 }
 
 func (f *IndexFetcher) createIndexIterator() (indexIterator, error) {

internal/db/index.go

@@ -191,7 +191,7 @@ func (index *collectionBaseIndex) getDocumentsIndexKey(
 	if appendDocID {
 		fields = append(fields, keys.IndexedField{Value: client.NewNormalString(doc.ID().String())})
 	}
-	return keys.NewIndexDataStoreKey(index.collection.ID(), index.desc.ID, fields), nil
+	return keys.NewIndexDataStoreKey(index.collection.Description().RootID, index.desc.ID, fields), nil
 }
 
 func (index *collectionBaseIndex) deleteIndexKey(
@@ -213,7 +213,7 @@ func (index *collectionBaseIndex) deleteIndexKey(
 // field values for all documents.
 func (index *collectionBaseIndex) RemoveAll(ctx context.Context, txn datastore.Txn) error {
 	prefixKey := keys.IndexDataStoreKey{}
-	prefixKey.CollectionID = index.collection.ID()
+	prefixKey.CollectionID = index.collection.Description().RootID
 	prefixKey.IndexID = index.desc.ID
 
 	keys, err := datastore.FetchKeysForPrefix(ctx, prefixKey.ToString(), txn.Datastore())

tests/integration/schema/updates/add/field/with_index_test.go

@@ -19,7 +19,7 @@ import (
 	testUtils "github.com/sourcenetwork/defradb/tests/integration"
 )
 
-func TestSchemaUpdatesAddFieldSimple_WithExistingIndex(t *testing.T) {
+func TestSchemaUpdatesAddFieldSimple_WithExistingIndexDocsCreatedAfterPatch(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Test patching schema for collection with index still works",
 		Actions: []any{
@@ -101,3 +101,88 @@ func TestSchemaUpdatesAddFieldSimple_WithExistingIndex(t *testing.T) {
 	}
 	testUtils.ExecuteTestCase(t, test)
 }
+
+func TestSchemaUpdatesAddFieldSimple_WithExistingIndexDocsCreatedBeforePatch(t *testing.T) {
+	test := testUtils.TestCase{
+		Description: "Test patching schema for collection with index still works",
+		Actions: []any{
+			testUtils.SchemaUpdate{
+				Schema: `
+					type Users {
+						name: String @index
+					}
+				`,
+			},
+			// It is important to test this with docs created *before* the patch, as well as after (see other test).
+			// A bug was missed by missing this test case.
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"Shahzad"
+					}`,
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"John"
+					}`,
+			},
+			testUtils.SchemaPatch{
+				Patch: `
+					[
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "email", "Kind": 11} }
+					]
+				`,
+			},
+			// It is important to test that the index shows up in both the `GetIndexes` call,
+			// *and* the `GetCollections` call, as indexes are stored in multiple places and we had a bug
+			// where patching a schema would result in the index disappearing from one of those locations.
+			testUtils.GetIndexes{
+				ExpectedIndexes: []client.IndexDescription{
+					{
+						Name:   "Users_name_ASC",
+						ID:     1,
+						Unique: false,
+						Fields: []client.IndexedFieldDescription{
+							{
+								Name: "name",
+							},
+						},
+					},
+				},
+			},
+			testUtils.GetCollections{
+				ExpectedResults: []client.CollectionDescription{
+					{
+						ID:             2,
+						Name:           immutable.Some("Users"),
+						IsMaterialized: true,
+						Indexes: []client.IndexDescription{
+							{
+								Name:   "Users_name_ASC",
+								ID:     1,
+								Unique: false,
+								Fields: []client.IndexedFieldDescription{
+									{
+										Name: "name",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			testUtils.Request{
+				Request: `query @explain(type: execute) {
+					Users(filter: {name: {_eq: "John"}}) {
+						name
+					}
+				}`,
+				Asserter: testUtils.NewExplainAsserter().WithFieldFetches(0).WithIndexFetches(1),
+			},
+		},
+	}
+	testUtils.ExecuteTestCase(t, test)
+}