-
Notifications
You must be signed in to change notification settings - Fork 19
support storing, filtering and retrieving non-key columns #444
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
Changes from all commits
5b1964a
87419ff
6ee5fef
7c444c2
4fe25e9
9c57758
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -407,6 +407,13 @@ | |
| "limit": { | ||
| "$ref": "#/components/schemas/Limit" | ||
| }, | ||
| "return_columns": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/components/schemas/ColumnName" | ||
| }, | ||
| "description": "Filtering-column names whose stored values should be returned alongside\nthe primary keys. Empty (the default) means return no column values." | ||
| }, | ||
| "vector": { | ||
| "$ref": "#/components/schemas/Vector" | ||
| } | ||
|
|
@@ -420,6 +427,18 @@ | |
| "similarity_scores" | ||
| ], | ||
| "properties": { | ||
| "column_values": { | ||
| "type": "object", | ||
| "description": "Per-column stored values for the filtering columns requested in\n`return_columns`. Each entry maps a column name to a Vec of\n`Option<Value>` — one entry per returned nearest neighbour, in the\nsame order as `similarity_scores`. `None` means the value was not\npresent for that row (e.g. the attribute did not exist when the row\nwas indexed). Absent when `return_columns` was empty.", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is "None" in json? |
||
| "additionalProperties": { | ||
| "type": "array", | ||
| "items": {} | ||
| }, | ||
| "propertyNames": { | ||
| "type": "string", | ||
| "description": "Name of the column in a db table." | ||
| } | ||
| }, | ||
| "distances": { | ||
| "type": "array", | ||
| "items": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,7 @@ impl TestClient { | |
| vector, | ||
| filter, | ||
| limit, | ||
| return_columns: vec![], | ||
| }) | ||
| .await | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,11 @@ pub(crate) async fn new() -> TestCase<TestActors> { | |
| timeout, | ||
| cql_per_row_ttl_expires_from_index, | ||
| ) | ||
| .with_test( | ||
| "cql_explicit_vector_column_deletion_removes_from_index", | ||
| timeout, | ||
| cql_explicit_vector_column_deletion_removes_from_index, | ||
| ) | ||
| } | ||
|
|
||
| #[framed] | ||
|
|
@@ -659,3 +664,94 @@ async fn cql_per_row_ttl_expires_from_index(actors: TestActors) { | |
|
|
||
| info!("finished"); | ||
| } | ||
|
|
||
| /// Regression test: explicitly deleting the vector column (`DELETE v FROM t WHERE pk = ?`) | ||
| /// must remove the item from the vector index, not leave it as "unchanged". | ||
| /// | ||
| /// Previously the CQL CDC path only treated full row/partition deletions as | ||
| /// removals. An explicit column tombstone produced `embedding = None` | ||
| /// ("unchanged") instead of `Some(None)` ("delete from index"), so the stale | ||
| /// vector remained searchable. | ||
| #[framed] | ||
| async fn cql_explicit_vector_column_deletion_removes_from_index(actors: TestActors) { | ||
| info!("started"); | ||
|
|
||
| let (session, clients) = prepare_connection_single_vs(&actors).await; | ||
| let client = clients.first().unwrap(); | ||
| let keyspace = create_keyspace(&session).await; | ||
| let table = create_table(&session, "pk INT PRIMARY KEY, v VECTOR<FLOAT, 3>", None).await; | ||
|
|
||
| session | ||
| .query_unpaged( | ||
| format!("INSERT INTO {table} (pk, v) VALUES (1, [1.0, 0.0, 0.0])"), | ||
| (), | ||
| ) | ||
| .await | ||
| .expect("failed to insert data"); | ||
| session | ||
| .query_unpaged( | ||
| format!("INSERT INTO {table} (pk, v) VALUES (2, [0.0, 1.0, 0.0])"), | ||
| (), | ||
| ) | ||
| .await | ||
| .expect("failed to insert data"); | ||
|
|
||
| let index = create_index(CreateIndexQuery::new(&session, &clients, &table, "v")).await; | ||
|
|
||
| let status = wait_for_index(client, &index).await; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead, we can wait until the SELECT returns two vectors. Prefer the CQL API over the Vector Store HTTP API in validator tests to keep them more E2E-oriented. |
||
| assert_eq!( | ||
| status.count, 2, | ||
| "Index should have 2 vectors after full scan" | ||
| ); | ||
|
|
||
| // Explicitly delete only the vector column of pk=1 (not the whole row). | ||
| // This is a column tombstone, not a row deletion — the CQL CDC path | ||
| // must recognise it via is_value_deleted() and remove the vector from the index. | ||
| session | ||
| .query_unpaged(format!("DELETE v FROM {table} WHERE pk = 1"), ()) | ||
| .await | ||
| .expect("failed to delete vector column"); | ||
|
|
||
| // Wait for the index count to drop to 1. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This wait is not needed while we have later: |
||
| wait_for( | ||
| || async { | ||
| let status = client.index_status(&index.keyspace, &index.index).await; | ||
| matches!(status, Ok(s) if s.count == 1) | ||
| }, | ||
| "Waiting for index count to drop to 1 after explicit column deletion", | ||
| FINE_GRAINED_CDC_MAX_LATENCY, | ||
| ) | ||
| .await; | ||
|
|
||
| // ANN query should only return pk=2. | ||
| let result = wait_for_value( | ||
| || async { | ||
| let result = get_opt_query_results( | ||
| format!("SELECT pk FROM {table} ORDER BY v ANN OF [0.0, 1.0, 0.0] LIMIT 10"), | ||
| &session, | ||
| ) | ||
| .await; | ||
| result.filter(|r| r.rows_num() == 1) | ||
| }, | ||
| "Waiting for ANN query to return only pk=2", | ||
| FINE_GRAINED_CDC_MAX_LATENCY, | ||
| ) | ||
| .await; | ||
| let pks: Vec<i32> = result | ||
| .rows::<(i32,)>() | ||
| .expect("failed to get rows") | ||
| .map(|r| r.expect("row error").0) | ||
| .collect(); | ||
| assert_eq!( | ||
| pks, | ||
| vec![2], | ||
| "Only pk=2 should remain after explicit vector column deletion" | ||
| ); | ||
|
|
||
| session | ||
| .query_unpaged(format!("DROP KEYSPACE {keyspace}"), ()) | ||
| .await | ||
| .expect("failed to drop keyspace"); | ||
|
|
||
| info!("finished"); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,6 +100,11 @@ pub(crate) async fn new() -> TestCase<TestActors> { | |
| timeout, | ||
| local_ann_with_timestamp_gte_filter, | ||
| ) | ||
| .with_test( | ||
| "ann_filter_by_non_pk_filtering_column", | ||
| timeout, | ||
| ann_filter_by_non_pk_filtering_column, | ||
| ) | ||
| } | ||
|
|
||
| /// Test ANN search filtered by partition key equality. | ||
|
|
@@ -1044,6 +1049,87 @@ async fn local_ann_with_timestamp_gte_filter(actors: TestActors) { | |
| info!("finished"); | ||
| } | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that this and the previous commit close VECTOR-419. This should be mentioned in the cover letter.
|
||
| /// Test ANN search filtered by a non-primary-key CQL filtering column. | ||
| /// | ||
| /// The index is created with `category` as a filtering column. Rows have two | ||
| /// category values (0 and 1). Querying with `WHERE category = 0` must return | ||
| /// only the rows whose category is 0. | ||
| #[framed] | ||
| async fn ann_filter_by_non_pk_filtering_column(actors: TestActors) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we have the same test for alternator API? |
||
| info!("started"); | ||
|
|
||
| let (session, clients) = prepare_connection(&actors).await; | ||
|
|
||
| let keyspace = create_keyspace(&session).await; | ||
| let table = create_table( | ||
| &session, | ||
| "pk INT PRIMARY KEY, category INT, v VECTOR<FLOAT, 3>", | ||
| None, | ||
| ) | ||
| .await; | ||
|
|
||
| // Insert 6 rows: pk 0..5, alternating category 0 and 1. | ||
| for pk in 0..6_i32 { | ||
| session | ||
| .query_unpaged( | ||
| format!("INSERT INTO {table} (pk, category, v) VALUES (?, ?, ?)"), | ||
| (pk, pk % 2, &vec![pk as f32, 0.0, 0.0]), | ||
| ) | ||
| .await | ||
| .expect("failed to insert data"); | ||
| } | ||
|
|
||
| let index = create_index( | ||
| CreateIndexQuery::new(&session, &clients, &table, "v").filter_columns(["category"]), | ||
| ) | ||
| .await; | ||
|
|
||
| for client in &clients { | ||
| let index_status = wait_for_index(client, &index).await; | ||
| assert_eq!(index_status.count, 6, "Expected 6 vectors to be indexed"); | ||
| } | ||
|
|
||
| let result = wait_for_value( | ||
| || async { | ||
| let result = get_opt_query_results( | ||
| format!( | ||
| "SELECT pk, category FROM {table} \ | ||
| WHERE category = 0 \ | ||
| ORDER BY v ANN OF [0.0, 0.0, 0.0] LIMIT 10 \ | ||
| ALLOW FILTERING" | ||
| ), | ||
| &session, | ||
| ) | ||
| .await; | ||
| result.filter(|r| r.rows_num() == 3) | ||
| }, | ||
| "Waiting for category=0 filtered ANN query to return 3 rows", | ||
| DEFAULT_OPERATION_TIMEOUT, | ||
| ) | ||
| .await; | ||
|
|
||
| let rows: Vec<(i32, i32)> = result | ||
| .rows::<(i32, i32)>() | ||
| .expect("failed to get rows") | ||
| .map(|row| row.expect("failed to get row")) | ||
| .collect(); | ||
|
|
||
| assert_eq!(rows.len(), 3, "Expected exactly 3 rows with category=0"); | ||
| for (pk, category) in &rows { | ||
| assert_eq!( | ||
| *category, 0, | ||
| "Expected all rows to have category=0, got pk={pk} category={category}" | ||
| ); | ||
| } | ||
|
|
||
| session | ||
| .query_unpaged(format!("DROP KEYSPACE {keyspace}"), ()) | ||
| .await | ||
| .expect("failed to drop a keyspace"); | ||
|
|
||
| info!("finished"); | ||
| } | ||
|
|
||
| #[framed] | ||
| async fn ann_filter_by_clustering_key_only_requires_allow_filtering(actors: TestActors) { | ||
| info!("started"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -265,8 +265,9 @@ fn scan_fn_mpsc( | |
| .send(( | ||
| DbEmbedding { | ||
| primary_key, | ||
| embedding, | ||
| embedding: Some(embedding), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe its worth considering to make this struct more generic and keep only column_values - put the embedding also in column_values. This potentialy can be useful for upcoming full text search changes also. |
||
| timestamp, | ||
| column_values: Default::default(), | ||
| }, | ||
| in_progress, | ||
| )) | ||
|
|
@@ -1139,7 +1140,7 @@ where | |
| }) | ||
| }) | ||
| .collect_vec(); | ||
| stream::iter(tasks.into_iter()) | ||
| stream::iter(tasks) | ||
| .then(|task| async move { task.await.unwrap() }) | ||
| .fold(Duration::ZERO, |acc, x| async move { acc + x }) | ||
| .await | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Vec of
Option<Value>" - this looks like some rust specifics, while this is description of HTTP API.