diff --git a/internal/db/collection_index.go b/internal/db/collection_index.go index 008964314e..49e796f824 100644 --- a/internal/db/collection_index.go +++ b/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 diff --git a/internal/db/fetcher/indexer_iterators.go b/internal/db/fetcher/indexer_iterators.go index 906c6e6103..8df1b114a5 100644 --- a/internal/db/fetcher/indexer_iterators.go +++ b/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) { diff --git a/internal/db/index.go b/internal/db/index.go index 92fad980e6..dfaaf20415 100644 --- a/internal/db/index.go +++ b/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()) diff --git a/tests/integration/schema/updates/add/field/with_index_test.go b/tests/integration/schema/updates/add/field/with_index_test.go index 9398276836..3c80f6cede 100644 --- a/tests/integration/schema/updates/add/field/with_index_test.go +++ b/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) +}