OAS-11787: Add Vector index feature and it's related testcases #725
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds vector index functionality to the ArangoDB Go driver, enabling efficient similarity searches on high-dimensional embeddings for AI use cases. The feature is available in ArangoDB 3.12.4 and later.
- Implements
EnsureVectorIndexmethod with comprehensive validation for vector index parameters - Adds support for vector-specific metrics (cosine, L2, inner product) and index options including storedValues
- Includes extensive test coverage for vector index creation, validation, and query execution plans
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| v2/arangodb/collection_indexes.go | Defines the EnsureVectorIndex API, VectorIndexType, VectorParams struct, VectorMetric type, and CreateVectorIndexOptions |
| v2/arangodb/collection_indexes_impl.go | Implements vector index creation logic, JSON unmarshaling for vector index responses, and parameter validation |
| v2/tests/database_collection_indexes_test.go | Adds comprehensive test cases for vector index functionality including creation, validation, storedValues, and query plan verification |
| v2/CHANGELOG.md | Documents the addition of the vector index feature |
| Makefile | Adds ENABLE_VECTOR_INDEX environment variable with default value "true" |
| test/cluster.sh | Configures cluster startup to enable experimental vector index feature when ENABLE_VECTOR_INDEX is set |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // EnsureVectorIndex creates a vector index in the collection, if it does not already exist. | ||
| // The index is returned, together with a boolean indicating if the index was newly created (true) or pre-existing (false). | ||
| // Available in ArangoDB 3.12.4 and later. | ||
| // VectorParams is an obligatory parameter and must contain at least Dimension,Metric and NLists fields. |
There was a problem hiding this comment.
The documentation states that NLists is a required field that must be present, but the validation logic treats it as optional (only validates if non-nil). Either update the documentation to reflect that NLists is optional, or make the validation enforce it as required.
| // VectorParams is an obligatory parameter and must contain at least Dimension,Metric and NLists fields. | |
| // VectorParams is an obligatory parameter and must contain at least Dimension and Metric fields. NLists is optional. |
| t.Run("Create Vector Index", func(t *testing.T) { | ||
| idx, created, err := col.EnsureVectorIndex( | ||
| ctx, | ||
| []string{"embedding"}, | ||
| params, | ||
| &arangodb.CreateVectorIndexOptions{ | ||
| Name: utils.NewType("my_vector_index"), | ||
| }, | ||
| ) | ||
| require.NoError(t, err) | ||
| require.True(t, created, "index should be created on first call") | ||
| require.Equal(t, arangodb.VectorIndexType, idx.Type) | ||
| require.NotNil(t, idx.VectorIndex) | ||
| require.Equal(t, dimension, *idx.VectorIndex.Dimension) | ||
| require.Equal(t, metric, *idx.VectorIndex.Metric) | ||
| }) |
There was a problem hiding this comment.
The first test case "Create Vector Index" creates an index with the name "my_vector_index" but lacks cleanup code to delete it after the test completes. This could cause subsequent test runs to fail if the index already exists. Add a defer statement to clean up the created index, similar to what's done in the "Create the same index again" test case.
| } | ||
|
|
||
| result := responseVectorIndex{} | ||
| fmt.Printf("reqData: %+v", reqData) |
There was a problem hiding this comment.
Debug print statement should be removed before merging to production. This line outputs debug information to stdout which is not appropriate for production code.
| fmt.Printf("reqData: %+v", reqData) |
No description provided.