Skip to content

Commit ec86fa6

Browse files
jewoodevsobychacko
authored andcommitted
Fix S3 vector filter deletion
Replace the S3VectorStore filter delete implementation so it no longer uses QueryVectors as a filter-only key lookup. The delete path now lists vectors with metadata, evaluates the Spring AI filter locally, and deletes matching keys in DeleteVectors batches. Add coverage for metadata listing, compound filter evaluation, pagination, delete batching, empty matches, and avoiding QueryVectors. Fixes #6422 Closes #6466 Signed-off-by: jewoodev <jewoos15@naver.com>
1 parent 1e6714b commit ec86fa6

3 files changed

Lines changed: 392 additions & 24 deletions

File tree

vector-stores/spring-ai-s3-vector-store/src/main/java/org/springframework/ai/vectorstore/s3/S3VectorStore.java

Lines changed: 62 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.ai.vectorstore.s3;
1818

1919
import java.util.ArrayList;
20+
import java.util.Collections;
2021
import java.util.HashMap;
2122
import java.util.List;
2223
import java.util.Map;
@@ -26,6 +27,9 @@
2627
import org.jspecify.annotations.Nullable;
2728
import software.amazon.awssdk.services.s3vectors.S3VectorsClient;
2829
import software.amazon.awssdk.services.s3vectors.model.DeleteVectorsRequest;
30+
import software.amazon.awssdk.services.s3vectors.model.ListOutputVector;
31+
import software.amazon.awssdk.services.s3vectors.model.ListVectorsRequest;
32+
import software.amazon.awssdk.services.s3vectors.model.ListVectorsResponse;
2933
import software.amazon.awssdk.services.s3vectors.model.PutInputVector;
3034
import software.amazon.awssdk.services.s3vectors.model.PutVectorsRequest;
3135
import software.amazon.awssdk.services.s3vectors.model.QueryOutputVector;
@@ -44,12 +48,18 @@
4448
import org.springframework.ai.vectorstore.observation.VectorStoreObservationContext;
4549
import org.springframework.beans.factory.InitializingBean;
4650
import org.springframework.util.Assert;
51+
import org.springframework.util.StringUtils;
4752

4853
/**
4954
* @author Matej Nedic
55+
* @author Jewoo Shin
5056
*/
5157
public class S3VectorStore extends AbstractObservationVectorStore implements InitializingBean {
5258

59+
private static final int LIST_VECTORS_MAX_RESULTS = 500;
60+
61+
private static final int DELETE_VECTORS_MAX_KEYS = 500;
62+
5363
private final S3VectorsClient s3VectorsClient;
5464

5565
private final String vectorBucketName;
@@ -58,6 +68,8 @@ public class S3VectorStore extends AbstractObservationVectorStore implements Ini
5868

5969
private final S3VectorFilterExpressionConverter filterExpressionConverter;
6070

71+
private final S3VectorStoreFilterExpressionEvaluator filterExpressionEvaluator;
72+
6173
/**
6274
* Creates a new S3VectorStore instance with the specified builder settings.
6375
* Initializes observation-related components and the embedding model.
@@ -73,6 +85,7 @@ protected S3VectorStore(Builder builder) {
7385
this.s3VectorsClient = builder.s3VectorsClient;
7486
this.indexName = builder.indexName;
7587
this.filterExpressionConverter = builder.filterExpressionConverter;
88+
this.filterExpressionEvaluator = new S3VectorStoreFilterExpressionEvaluator();
7689
this.vectorBucketName = builder.vectorBucketName;
7790
}
7891

@@ -100,35 +113,38 @@ public void doAdd(List<Document> documents) {
100113

101114
@Override
102115
public void doDelete(List<String> idList) {
103-
this.s3VectorsClient.deleteVectors(DeleteVectorsRequest.builder()
104-
.keys(idList)
105-
.indexName(this.indexName)
106-
.vectorBucketName(this.vectorBucketName)
107-
.build());
116+
deleteVectors(idList);
108117
}
109118

110119
@Override
111120
public void doDelete(Filter.Expression filterExpression) {
112-
Assert.notNull(filterExpression, "Filter expression mus not be null");
121+
Assert.notNull(filterExpression, "Filter expression must not be null");
122+
123+
String nextToken = null;
124+
List<String> keys = new ArrayList<>();
125+
do {
126+
ListVectorsRequest.Builder requestBuilder = ListVectorsRequest.builder()
127+
.vectorBucketName(this.vectorBucketName)
128+
.indexName(this.indexName)
129+
.maxResults(LIST_VECTORS_MAX_RESULTS)
130+
.returnMetadata(true)
131+
.returnData(false);
132+
133+
if (StringUtils.hasText(nextToken)) {
134+
requestBuilder.nextToken(nextToken);
135+
}
136+
137+
ListVectorsResponse response = this.s3VectorsClient.listVectors(requestBuilder.build());
138+
for (ListOutputVector vector : response.vectors()) {
139+
if (matchesFilter(vector, filterExpression)) {
140+
keys.add(vector.key());
141+
}
142+
}
143+
nextToken = response.nextToken();
144+
}
145+
while (StringUtils.hasText(nextToken));
113146

114-
software.amazon.awssdk.core.document.Document filterDoc = this.filterExpressionConverter
115-
.convertExpression(filterExpression);
116-
QueryVectorsRequest request = QueryVectorsRequest.builder()
117-
.filter(filterDoc)
118-
.vectorBucketName(this.vectorBucketName)
119-
.indexName(this.indexName)
120-
.build();
121-
List<String> keys = this.s3VectorsClient.queryVectors(request)
122-
.vectors()
123-
.stream()
124-
.map(QueryOutputVector::key)
125-
.toList();
126-
127-
this.s3VectorsClient.deleteVectors(DeleteVectorsRequest.builder()
128-
.vectorBucketName(this.vectorBucketName)
129-
.keys(keys)
130-
.indexName(this.indexName)
131-
.build());
147+
deleteVectors(keys);
132148
}
133149

134150
@Override
@@ -168,6 +184,28 @@ private Document toDocument(QueryOutputVector vector) {
168184
return Document.builder().metadata(metadata).text(vector.key()).build();
169185
}
170186

187+
private boolean matchesFilter(ListOutputVector vector, Filter.Expression filterExpression) {
188+
software.amazon.awssdk.core.document.Document metadataDocument = vector.metadata();
189+
Map<String, Object> metadata = (metadataDocument != null) ? DocumentUtils.fromDocument(metadataDocument) : null;
190+
return this.filterExpressionEvaluator.evaluate(filterExpression,
191+
(metadata != null) ? metadata : Collections.emptyMap());
192+
}
193+
194+
private void deleteVectors(List<String> keys) {
195+
Assert.notNull(keys, "Keys must not be null");
196+
if (keys.isEmpty()) {
197+
return;
198+
}
199+
for (int i = 0; i < keys.size(); i += DELETE_VECTORS_MAX_KEYS) {
200+
List<String> batch = keys.subList(i, Math.min(i + DELETE_VECTORS_MAX_KEYS, keys.size()));
201+
this.s3VectorsClient.deleteVectors(DeleteVectorsRequest.builder()
202+
.keys(batch)
203+
.indexName(this.indexName)
204+
.vectorBucketName(this.vectorBucketName)
205+
.build());
206+
}
207+
}
208+
171209
private static software.amazon.awssdk.core.document.Document constructMetadata(
172210
Map<String, Object> originalMetadata) {
173211
Map<String, software.amazon.awssdk.core.document.Document> metadata = new HashMap<>(originalMetadata.size());
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/*
2+
* Copyright 2023-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.ai.vectorstore.s3;
18+
19+
import java.time.ZoneOffset;
20+
import java.time.format.DateTimeFormatter;
21+
import java.util.Date;
22+
import java.util.List;
23+
import java.util.Map;
24+
import java.util.Objects;
25+
26+
import org.jspecify.annotations.Nullable;
27+
28+
import org.springframework.ai.vectorstore.filter.Filter;
29+
30+
/**
31+
* Package-private helper used by {@link S3VectorStore} to evaluate a
32+
* {@link Filter.Expression} against metadata returned by the S3 Vectors ListVectors API.
33+
*
34+
* @author Jewoo Shin
35+
*/
36+
final class S3VectorStoreFilterExpressionEvaluator {
37+
38+
private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'")
39+
.withZone(ZoneOffset.UTC);
40+
41+
boolean evaluate(Filter.Expression expression, Map<String, Object> metadata) {
42+
return evaluateExpression(expression, metadata);
43+
}
44+
45+
private boolean evaluateOperand(Filter.Operand operand, Map<String, Object> metadata) {
46+
if (operand instanceof Filter.Group group) {
47+
return evaluateOperand(group.content(), metadata);
48+
}
49+
if (operand instanceof Filter.Expression expression) {
50+
return evaluateExpression(expression, metadata);
51+
}
52+
throw new IllegalArgumentException("Unsupported operand type: " + operand.getClass().getName());
53+
}
54+
55+
private boolean evaluateExpression(Filter.Expression expression, Map<String, Object> metadata) {
56+
return switch (expression.type()) {
57+
case AND -> evaluateOperand(left(expression), metadata) && evaluateOperand(right(expression), metadata);
58+
case OR -> evaluateOperand(left(expression), metadata) || evaluateOperand(right(expression), metadata);
59+
case NOT -> !evaluateOperand(left(expression), metadata);
60+
case EQ -> compare(metadataValue(left(expression), metadata), filterValue(right(expression))) == 0;
61+
case NE -> compare(metadataValue(left(expression), metadata), filterValue(right(expression))) != 0;
62+
case GT -> compare(metadataValue(left(expression), metadata), filterValue(right(expression))) > 0;
63+
case GTE -> compare(metadataValue(left(expression), metadata), filterValue(right(expression))) >= 0;
64+
case LT -> compare(metadataValue(left(expression), metadata), filterValue(right(expression))) < 0;
65+
case LTE -> compare(metadataValue(left(expression), metadata), filterValue(right(expression))) <= 0;
66+
case IN -> {
67+
Object metaVal = metadataValue(left(expression), metadata);
68+
List<?> list = asList(filterValue(right(expression)), expression);
69+
yield list.stream().anyMatch(item -> compare(metaVal, item) == 0);
70+
}
71+
case NIN -> {
72+
Object metaVal = metadataValue(left(expression), metadata);
73+
List<?> list = asList(filterValue(right(expression)), expression);
74+
yield list.stream().noneMatch(item -> compare(metaVal, item) == 0);
75+
}
76+
case ISNULL -> metadataValue(left(expression), metadata) == null;
77+
case ISNOTNULL -> metadataValue(left(expression), metadata) != null;
78+
};
79+
}
80+
81+
private Filter.Operand left(Filter.Expression expression) {
82+
Filter.Operand left = expression.left();
83+
if (left == null) {
84+
throw new IllegalArgumentException(
85+
"Expression of type %s requires a left operand".formatted(expression.type()));
86+
}
87+
return left;
88+
}
89+
90+
private Filter.Operand right(Filter.Expression expression) {
91+
Filter.Operand right = expression.right();
92+
if (right == null) {
93+
throw new IllegalArgumentException(
94+
"Expression of type %s requires a right operand".formatted(expression.type()));
95+
}
96+
return right;
97+
}
98+
99+
private @Nullable Object metadataValue(Filter.Operand operand, Map<String, Object> metadata) {
100+
if (operand instanceof Filter.Key key) {
101+
String k = key.key();
102+
if (k.length() >= 2
103+
&& ((k.startsWith("\"") && k.endsWith("\"")) || (k.startsWith("'") && k.endsWith("'")))) {
104+
k = k.substring(1, k.length() - 1);
105+
}
106+
return metadata.get(k);
107+
}
108+
throw new IllegalArgumentException("Expected a Key operand but got: " + operand.getClass().getName());
109+
}
110+
111+
private Object filterValue(Filter.Operand operand) {
112+
if (operand instanceof Filter.Value filterValue) {
113+
Object value = filterValue.value();
114+
return (value instanceof Date date) ? DATE_FORMATTER.format(date.toInstant()) : value;
115+
}
116+
throw new IllegalArgumentException("Expected a Value operand but got: " + operand.getClass().getName());
117+
}
118+
119+
@SuppressWarnings("unchecked")
120+
private int compare(@Nullable Object metaVal, @Nullable Object filterVal) {
121+
if (metaVal == null && filterVal == null) {
122+
return 0;
123+
}
124+
if (metaVal == null) {
125+
return -1;
126+
}
127+
if (filterVal == null) {
128+
return 1;
129+
}
130+
if (metaVal instanceof Number n1 && filterVal instanceof Number n2) {
131+
return Double.compare(n1.doubleValue(), n2.doubleValue());
132+
}
133+
if (Objects.equals(metaVal, filterVal)) {
134+
return 0;
135+
}
136+
if (metaVal instanceof Comparable comparable && filterVal instanceof Comparable) {
137+
try {
138+
return comparable.compareTo(filterVal);
139+
}
140+
catch (ClassCastException ex) {
141+
throw new IllegalArgumentException("Cannot compare values of incompatible types %s and %s"
142+
.formatted(metaVal.getClass().getName(), filterVal.getClass().getName()), ex);
143+
}
144+
}
145+
throw new IllegalArgumentException("Cannot compare values of types %s and %s"
146+
.formatted(metaVal.getClass().getName(), filterVal.getClass().getName()));
147+
}
148+
149+
private List<?> asList(Object value, Filter.Expression expression) {
150+
if (value instanceof List<?> list) {
151+
return list;
152+
}
153+
throw new IllegalArgumentException(
154+
"Expected a List value for %s expression but got: %s".formatted(expression.type(), value));
155+
}
156+
157+
}

0 commit comments

Comments
 (0)