Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRIVERS-2768 [Vector Search GA] Add support for types in search index creation #1541

Merged
merged 7 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions source/index-management/index-management.md
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,15 @@ interface IndexOptions {
* @example For an index of name: 1, age: -1, the generated name would be "name_1_age_-1".
*/
name: String;

/**
* Optionally specify a type for the index. Defaults to "search" if not provided.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many other driver options are implemented such that we only send a value if the user explicitly provides one. How do you feel about mandating that driver MUST NOT send this field if not provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me if that's a consistent pattern among drivers. The server should default to search implicitly if type is not provided, so the behavior is consistent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this change be made to the CreateSearchIndexOptions instead of IndexOptions? https://github.com/mongodb/specifications/blob/master/source/index-management/index-management.md#common-interfaces

Copy link
Contributor Author

@NoahStapp NoahStapp Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CreateSearchIndexOptions is currently unused. If drivers support adding type to IndexOptions, that seems to be a better fit as then the top-level fields of type and name are present in the same interface.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, looking more closely it probably belongs in the SearchIndexModel?

IndexOptions is used for regular indexes, not search indexes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good catch! Yes, SearchIndexModel seems like the correct place for type.

* Either "search" for regular search indexes or "vectorSearch" for vector search indexes.
*
* Note that to create a vector search index using a helper method, the type "vectorSearch" must be provided.
*
*/
type: String;

/**
* Optionally tells the index to only reference documents with the specified field in
Expand Down
11 changes: 11 additions & 0 deletions source/index-management/index-management.rst
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,16 @@ Common API Components
*/
name: String;

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we just switched this spec to Markdown, so you'll have to move these changes to that file.

* Optionally specify a type for the index. Defaults to "search" if not provided.
* Either "search" for regular search indexes or "vectorSearch" for vector search indexes.
*
* Note that to create a vector search index using a helper method, the type "vectorSearch" must be provided.
*
*/
type: String;


/**
* Optionally tells the index to only reference documents with the specified field in
* the index.
Expand Down Expand Up @@ -1149,3 +1159,4 @@ Changelog
:2023-07-27: Add search index management clarifications.
:2023-11-08: Clarify that ``readConcern`` and ``writeConcern`` must not be
applied to search index management commands.
:2024-03-06: Update tests to include search index typing.
100 changes: 100 additions & 0 deletions source/index-management/tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,3 +231,103 @@ This test fails if it times out waiting for the deletion to succeed.
of `true`.

6. Assert that `index` has a property `latestDefinition` whose value is `{ 'mappings': { 'dynamic': false } }`

#### Case 7: Driver can successfully handle search index types when creating indexes

01. Create a collection with the "create" command using a randomly generated name (referred to as `coll0`).

02. Create a new search index on `coll0` with the `createSearchIndex` helper. Use the following definition:
baileympearson marked this conversation as resolved.
Show resolved Hide resolved

```typescript

{
name: 'test-search-index-case7-implicit',
definition: {
mappings: { dynamic: false }
}
}
```

03. Assert that the command returns the name of the index: `"test-search-index-case7-implicit"`.

04. Run `coll0.listSearchIndexes('test-search-index-case7-implicit')` repeatedly every 5 seconds until the following
condition is satisfied and store the value in a variable `index1`:

- An index with the `name` of `test-search-index-case7-implicit` is present and the index has a field `queryable`
with a value of `true`.

05. Assert that `index1` has a property `type` whose value is `search`.

06. Create a new search index on `coll0` with the `createSearchIndex` helper. Use the following definition:

```typescript

{
name: 'test-search-index-case7-explicit',
type: 'search',
definition: {
mappings: { dynamic: false }
}
}
```

07. Assert that the command returns the name of the index: `"test-search-index-case7-explicit"`.

08. Run `coll0.listSearchIndexes('test-search-index-case7-explicit')` repeatedly every 5 seconds until the following
condition is satisfied and store the value in a variable `index2`:

- An index with the `name` of `test-search-index-case7-explicit` is present and the index has a field `queryable`
with a value of `true`.

09. Assert that `index2` has a property `type` whose value is `search`.

10. Create a new vector search index on `coll0` with the `createSearchIndex` helper. Use the following definition:

```typescript

{
name: 'test-search-index-case7-vector',
type: 'vectorSearch',
definition: {
"fields": [
{
"type": "vector",
"path": "plot_embedding",
"numDimensions": 1536,
"similarity": "euclidean",
},
]
}
}
```

11. Assert that the command returns the name of the index: `"test-search-index-case7-vector"`.

12. Run `coll0.listSearchIndexes('test-search-index-case7-vector')` repeatedly every 5 seconds until the following
condition is satisfied and store the value in a variable `index3`:

- An index with the `name` of `test-search-index-case7-vector` is present and the index has a field `queryable` with
a value of `true`.

13. Assert that `index3` has a property `type` whose value is `vectorSearch`.

14. Create a new vector search index on `coll0` with the `createSearchIndex` helper. Use the following definition:
baileympearson marked this conversation as resolved.
Show resolved Hide resolved

```typescript

{
name: 'test-search-index-case7-error',
definition: {
"fields": [
{
"type": "vector",
"path": "plot_embedding",
"numDimensions": 1536,
"similarity": "euclidean",
},
]
}
}
```

15. Assert that the command throws an exception due to the `mappings` field missing.
72 changes: 68 additions & 4 deletions source/index-management/tests/createSearchIndex.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 26 additions & 4 deletions source/index-management/tests/createSearchIndex.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ tests:
- name: createSearchIndex
object: *collection0
arguments:
model: { definition: &definition { mappings: { dynamic: true } } }
model: { definition: &definition { mappings: { dynamic: true } } , type: 'search' }
expectError:
# This test always errors in a non-Atlas environment. The test functions as a unit test by asserting
# that the driver constructs and sends the correct command.
Expand All @@ -39,15 +39,15 @@ tests:
- commandStartedEvent:
command:
createSearchIndexes: *collection0
indexes: [ { definition: *definition } ]
indexes: [ { definition: *definition, type: 'search'} ]
$db: *database0

- description: "name provided for an index definition"
operations:
- name: createSearchIndex
object: *collection0
arguments:
model: { definition: &definition { mappings: { dynamic: true } } , name: 'test index' }
model: { definition: &definition { mappings: { dynamic: true } } , name: 'test index', type: 'search' }
expectError:
# This test always errors in a non-Atlas environment. The test functions as a unit test by asserting
# that the driver constructs and sends the correct command.
Expand All @@ -60,5 +60,27 @@ tests:
- commandStartedEvent:
command:
createSearchIndexes: *collection0
indexes: [ { definition: *definition, name: 'test index' } ]
indexes: [ { definition: *definition, name: 'test index', type: 'search' } ]
$db: *database0

- description: "create a vector search index"
operations:
- name: createSearchIndex
object: *collection0
arguments:
model: { definition: &definition { fields: [ {"type": "vector", "path": "plot_embedding", "numDimensions": 1536, "similarity": "euclidean"} ] }
, name: 'test index', type: 'vectorSearch' }
expectError:
# This test always errors in a non-Atlas environment. The test functions as a unit test by asserting
# that the driver constructs and sends the correct command.
# The expected error message was changed in SERVER-83003. Check for the substring "Atlas" shared by both error messages.
isError: true
errorContains: Atlas
expectEvents:
- client: *client0
events:
- commandStartedEvent:
command:
createSearchIndexes: *collection0
indexes: [ { definition: *definition, name: 'test index', type: 'vectorSearch' } ]
$db: *database0
Loading
Loading