You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/01/14 17:03:25 UTC
[GitHub] [lucene-solr] irvingzhang opened a new pull request #1169:
LUCENE-9004: A minor feature and patch -- support deleting vector values
and fix segments merging
irvingzhang opened a new pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169
I think this commit belongs to this issue (https://issues.apache.org/jira/browse/LUCENE-9004). I'm not sure if I need to create a new issue. Following are my specified considerations,
1. A minor feature: Regarding to the ANN search problems, it's dangerous to delete vectors according to similarity search result in HNSW. The selected docs are neither sorted nor reduced. The number of deleted vectors is proportional to the segment count and the parameter ef. And the deleted vectors is obviously uncertain. Hence, I created a new type of Query (KnnDelQuery) And Weight (KnnDelScoreWeight) for the dedicated deleting of the exact values that matching the query vector;
2. A minor patch: For fixing the merge process while some segments may contain deleted documents that must be filtered;
The modified codes have been tested by the test cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mocobeta commented on a change in pull request #1169:
LUCENE-9004: A minor feature and patch -- support deleting vector values
and fix segments merging
Posted by GitBox <gi...@apache.org>.
mocobeta commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368224473
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnExactDeletionCondition.java
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * A query for deleting the exact values of the specified querying vector, used for
+ * {@link org.apache.lucene.index.IndexWriter#deleteDocuments(Query...)}.
+ */
+public class KnnExactDeletionCondition extends KnnGraphQuery {
Review comment:
Why do we need to include the word "deletion" in the Query name? If my understanding is correct this finds a set of documents that have exact values to the query vector, and does not concern what the user do with the query results. Additionally it seems slightly odd to me that the (public) Query subclass does not obey the naming convention (`XXXQuery`). I would prefer the name "KnnExactVectorQuery" or something like that here...
If you intend to use this Query class for special purpose ("for deleting documents"), I'd suggest you don't expose it as a public class but firstly keep it as a dedicated inner class within another class. Would it be possible?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368349044
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestKnnGraph.java
##########
@@ -92,7 +108,277 @@ public void testSingleDocRecall() throws Exception {
iw.commit();
assertConsistentGraph(iw, values);
- assertRecall(dir, 0, values[0]);
+ assertRecall(dir, 1, values[0]);
+ }
+ }
+
+ public void testDocsDeletionAndRecall() throws Exception {
+ /**
+ * {@code KnnExactVectorValueWeight} applies in-set (i.e. the query vector is exactly in the index)
+ * deletion strategy to filter all unmatched results searched by {@link org.apache.lucene.search.KnnGraphQuery.KnnExactVectorValueQuery},
+ * and deletes at most ef*segmentCnt vectors that are the same to the specified queryVector.
+ */
+ final class KnnExactVectorValueWeight extends ConstantScoreWeight {
+ private final String field;
+ private final ScoreMode scoreMode;
+ private final float[] queryVector;
+ private final int ef;
+
+ KnnExactVectorValueWeight(Query query, float score, ScoreMode scoreMode, String field, float[] queryVector, int ef) {
+ super(query, score);
+ this.field = field;
+ this.scoreMode = scoreMode;
+ this.queryVector = queryVector;
+ this.ef = ef;
+ }
+
+ /**
+ * Returns a {@link Scorer} which can iterate in order over all matching
+ * documents and assign them a score.
+ * <p>
+ * <b>NOTE:</b> null can be returned if no documents will be scored by this
+ * query.
+ * <p>
+ * <b>NOTE</b>: The returned {@link Scorer} does not have
+ * {@link LeafReader#getLiveDocs()} applied, they need to be checked on top.
+ *
+ * @param context the {@link LeafReaderContext} for which to return the {@link Scorer}.
+ * @return a {@link Scorer} which scores documents in/out-of order.
+ * @throws IOException if there is a low-level I/O error
+ */
+ @Override
+ public Scorer scorer(LeafReaderContext context) throws IOException {
+ ScorerSupplier supplier = scorerSupplier(context);
+ if (supplier == null) {
+ return null;
+ }
+ return supplier.get(Long.MAX_VALUE);
+ }
+
+ @Override
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ FieldInfo fi = context.reader().getFieldInfos().fieldInfo(field);
+ int numDimensions = fi.getVectorNumDimensions();
+ if (numDimensions != queryVector.length) {
+ throw new IllegalArgumentException("field=\"" + field + "\" was indexed with dimensions=" + numDimensions +
+ "; this is incompatible with query dimensions=" + queryVector.length);
+ }
+
+ final HNSWGraphReader hnswReader = new HNSWGraphReader(field, context);
+ final VectorValues vectorValues = context.reader().getVectorValues(field);
+ if (vectorValues == null) {
+ // No docs in this segment/field indexed any vector values
+ return null;
+ }
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ final Neighbors neighbors = hnswReader.searchNeighbors(queryVector, ef, vectorValues);
+
+ if (neighbors.size() > 0) {
+ Neighbor top = neighbors.top();
+ if (top.distance() > 0) {
+ neighbors.clear();
+ } else {
+ final List<Neighbor> toDeleteNeighbors = new ArrayList<>(neighbors.size());
Review comment:
Yes, and thanks.
I hope to test some cases where segments contain deleted vectors. I expect the deleted vector values are deterministic, making the assertions meet in any execution.
The classes, KnnExactVectorValueQuery and KnnExactVectorValueWeight, are just used for my test case, so I put them in the test file.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msokolov commented on a change in pull request #1169:
LUCENE-9004: A minor feature and patch -- support deleting vector values
and fix segments merging
Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r370709036
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestKnnGraph.java
##########
@@ -92,7 +108,277 @@ public void testSingleDocRecall() throws Exception {
iw.commit();
assertConsistentGraph(iw, values);
- assertRecall(dir, 0, values[0]);
+ assertRecall(dir, 1, values[0]);
+ }
+ }
+
+ public void testDocsDeletionAndRecall() throws Exception {
+ /**
+ * {@code KnnExactVectorValueWeight} applies in-set (i.e. the query vector is exactly in the index)
+ * deletion strategy to filter all unmatched results searched by {@link org.apache.lucene.search.KnnGraphQuery.KnnExactVectorValueQuery},
+ * and deletes at most ef*segmentCnt vectors that are the same to the specified queryVector.
+ */
+ final class KnnExactVectorValueWeight extends ConstantScoreWeight {
+ private final String field;
+ private final ScoreMode scoreMode;
+ private final float[] queryVector;
+ private final int ef;
+
+ KnnExactVectorValueWeight(Query query, float score, ScoreMode scoreMode, String field, float[] queryVector, int ef) {
+ super(query, score);
+ this.field = field;
+ this.scoreMode = scoreMode;
+ this.queryVector = queryVector;
+ this.ef = ef;
+ }
+
+ /**
+ * Returns a {@link Scorer} which can iterate in order over all matching
+ * documents and assign them a score.
+ * <p>
+ * <b>NOTE:</b> null can be returned if no documents will be scored by this
+ * query.
+ * <p>
+ * <b>NOTE</b>: The returned {@link Scorer} does not have
+ * {@link LeafReader#getLiveDocs()} applied, they need to be checked on top.
+ *
+ * @param context the {@link LeafReaderContext} for which to return the {@link Scorer}.
+ * @return a {@link Scorer} which scores documents in/out-of order.
+ * @throws IOException if there is a low-level I/O error
+ */
+ @Override
+ public Scorer scorer(LeafReaderContext context) throws IOException {
+ ScorerSupplier supplier = scorerSupplier(context);
+ if (supplier == null) {
+ return null;
+ }
+ return supplier.get(Long.MAX_VALUE);
+ }
+
+ @Override
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ FieldInfo fi = context.reader().getFieldInfos().fieldInfo(field);
+ int numDimensions = fi.getVectorNumDimensions();
+ if (numDimensions != queryVector.length) {
+ throw new IllegalArgumentException("field=\"" + field + "\" was indexed with dimensions=" + numDimensions +
+ "; this is incompatible with query dimensions=" + queryVector.length);
+ }
+
+ final HNSWGraphReader hnswReader = new HNSWGraphReader(field, context);
+ final VectorValues vectorValues = context.reader().getVectorValues(field);
+ if (vectorValues == null) {
+ // No docs in this segment/field indexed any vector values
+ return null;
+ }
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ final Neighbors neighbors = hnswReader.searchNeighbors(queryVector, ef, vectorValues);
+
+ if (neighbors.size() > 0) {
+ Neighbor top = neighbors.top();
+ if (top.distance() > 0) {
+ neighbors.clear();
+ } else {
+ final List<Neighbor> toDeleteNeighbors = new ArrayList<>(neighbors.size());
Review comment:
I think I'm having trouble seeing where this is being used: maybe I just don't understand how to do PRs on github and I'm missing it, but have you posted a test that uses this query?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368235896
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnExactDeletionCondition.java
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * A query for deleting the exact values of the specified querying vector, used for
+ * {@link org.apache.lucene.index.IndexWriter#deleteDocuments(Query...)}.
+ */
+public class KnnExactDeletionCondition extends KnnGraphQuery {
Review comment:
Yes, your understanding is correct and highly appreciated.
1. The class KnnExactVectorValueQuery (revised name) is originally designed for deleting vectors by query. When we are going to delete some vector by its exact values, Lucene seems to retrieve all the similar vectors and then delete all of them, which may not be the behavior we expect. The class KnnExactVectorValueWeight is the corresponding weight for KnnExactVectorValueQuery to filter the vectors that not exactly equal to the query vector. Therefore, the two classes are specially designed for deletion. The results that KnnGraphQuery retrieved contains that retrieved by KnnExactVectorValueWeight, so there's any necessary to establish a new query (e.g. KnnExactVectorValueQuery) for ordinary query.
2. I'm not pretty sure if there is any situation that we need to delete by query. So I moved both classes into my test case.
Now, there isn't any new feature, just some minor bug fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mocobeta commented on a change in pull request #1169:
LUCENE-9004: A minor feature and patch -- support deleting vector values
and fix segments merging
Posted by GitBox <gi...@apache.org>.
mocobeta commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368289325
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnScoreWeight.java
##########
@@ -18,6 +18,8 @@
package org.apache.lucene.search;
import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
Review comment:
Can you please remove those unused imports?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368235896
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnExactDeletionCondition.java
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * A query for deleting the exact values of the specified querying vector, used for
+ * {@link org.apache.lucene.index.IndexWriter#deleteDocuments(Query...)}.
+ */
+public class KnnExactDeletionCondition extends KnnGraphQuery {
Review comment:
Yes, your understanding is correct and highly appreciated.
1. The class KnnExactVectorValueQuery (revised name) is originally designed for deleting vectors by query. When we are going to delete some vector by its exact values, Lucene seems to retrieve all the similar vectors and then delete all of them, which may not be the behavior we expect. The class KnnExactVectorValueWeight is the corresponding weight for KnnExactVectorValueQuery to filter the vectors that not exactly equal to the query vector. Therefore, the two classes are specially designed for deletion. The results that KnnGraphQuery retrieved contains that of KnnExactVectorValueWeight retrieved, so there's any requirement to establish a new query type (e.g. KnnExactVectorValueQuery) for ordinary query.
2. I'm not pretty sure if there is any situation that we need to delete by query. So I moved both classes into my test case.
Now, there isn't any new feature, just some minor bug fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mocobeta commented on a change in pull request #1169:
LUCENE-9004: A minor feature and patch -- support deleting vector values
and fix segments merging
Posted by GitBox <gi...@apache.org>.
mocobeta commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r366914317
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90KnnGraphWriter.java
##########
@@ -216,8 +216,11 @@ private void mergeKnnGraph(FieldInfo mergeFieldInfo, final MergeState mergeState
int docid;
while ((docid = sub.nextDoc()) != NO_MORE_DOCS) {
int mappedDocId = docMap.get(docid);
+ /// deleted document (not alive)
+ if (mappedDocId < 0) {
Review comment:
You're right, this is needed for document deletion. A small correction: the condition should be `mappedDocId == -1`. (According to the Javadoc of the DocMap#get() method.)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msokolov commented on a change in pull request #1169:
LUCENE-9004: A minor feature and patch -- support deleting vector values
and fix segments merging
Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368330855
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestKnnGraph.java
##########
@@ -92,7 +108,277 @@ public void testSingleDocRecall() throws Exception {
iw.commit();
assertConsistentGraph(iw, values);
- assertRecall(dir, 0, values[0]);
+ assertRecall(dir, 1, values[0]);
+ }
+ }
+
+ public void testDocsDeletionAndRecall() throws Exception {
+ /**
+ * {@code KnnExactVectorValueWeight} applies in-set (i.e. the query vector is exactly in the index)
+ * deletion strategy to filter all unmatched results searched by {@link org.apache.lucene.search.KnnGraphQuery.KnnExactVectorValueQuery},
+ * and deletes at most ef*segmentCnt vectors that are the same to the specified queryVector.
+ */
+ final class KnnExactVectorValueWeight extends ConstantScoreWeight {
+ private final String field;
+ private final ScoreMode scoreMode;
+ private final float[] queryVector;
+ private final int ef;
+
+ KnnExactVectorValueWeight(Query query, float score, ScoreMode scoreMode, String field, float[] queryVector, int ef) {
+ super(query, score);
+ this.field = field;
+ this.scoreMode = scoreMode;
+ this.queryVector = queryVector;
+ this.ef = ef;
+ }
+
+ /**
+ * Returns a {@link Scorer} which can iterate in order over all matching
+ * documents and assign them a score.
+ * <p>
+ * <b>NOTE:</b> null can be returned if no documents will be scored by this
+ * query.
+ * <p>
+ * <b>NOTE</b>: The returned {@link Scorer} does not have
+ * {@link LeafReader#getLiveDocs()} applied, they need to be checked on top.
+ *
+ * @param context the {@link LeafReaderContext} for which to return the {@link Scorer}.
+ * @return a {@link Scorer} which scores documents in/out-of order.
+ * @throws IOException if there is a low-level I/O error
+ */
+ @Override
+ public Scorer scorer(LeafReaderContext context) throws IOException {
+ ScorerSupplier supplier = scorerSupplier(context);
+ if (supplier == null) {
+ return null;
+ }
+ return supplier.get(Long.MAX_VALUE);
+ }
+
+ @Override
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ FieldInfo fi = context.reader().getFieldInfos().fieldInfo(field);
+ int numDimensions = fi.getVectorNumDimensions();
+ if (numDimensions != queryVector.length) {
+ throw new IllegalArgumentException("field=\"" + field + "\" was indexed with dimensions=" + numDimensions +
+ "; this is incompatible with query dimensions=" + queryVector.length);
+ }
+
+ final HNSWGraphReader hnswReader = new HNSWGraphReader(field, context);
+ final VectorValues vectorValues = context.reader().getVectorValues(field);
+ if (vectorValues == null) {
+ // No docs in this segment/field indexed any vector values
+ return null;
+ }
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ final Neighbors neighbors = hnswReader.searchNeighbors(queryVector, ef, vectorValues);
+
+ if (neighbors.size() > 0) {
+ Neighbor top = neighbors.top();
+ if (top.distance() > 0) {
+ neighbors.clear();
+ } else {
+ final List<Neighbor> toDeleteNeighbors = new ArrayList<>(neighbors.size());
Review comment:
You are -- finding exact matches to the input vector, right? I don't understand what this has to do with deletion. I'm also unclear why we want to have an exact match query in the first place. What problem is it solving that we could not solve with a hashmap lookup? And ... it is implemented here in a test file. Is this supporting testing in some way? Thanks, I feel I must be missing some essential thing here...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mocobeta commented on a change in pull request #1169:
LUCENE-9004: A minor feature and patch -- support deleting vector values
and fix segments merging
Posted by GitBox <gi...@apache.org>.
mocobeta commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368224473
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnExactDeletionCondition.java
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * A query for deleting the exact values of the specified querying vector, used for
+ * {@link org.apache.lucene.index.IndexWriter#deleteDocuments(Query...)}.
+ */
+public class KnnExactDeletionCondition extends KnnGraphQuery {
Review comment:
Why do we need to include the word "deletion" in the Query name? If my understanding is correct this finds a set of documents that have exact values to the query vector, and does not concern what the user do with the query results. Additionally it seems slightly odd to me that the (public) Query subclass does not obey the naming convention (`XXXQuery`). I would prefer the name "KnnExactVectorValueQuery" or something like that here...
If you intend to use this Query class for special purpose ("for deleting documents"), I'd suggest you don't expose it as a public class but firstly keep it as a dedicated inner class within another class. Would it be possible?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mocobeta commented on issue #1169: LUCENE-9004: A
minor feature and patch -- support deleting vector values and fix segments
merging
Posted by GitBox <gi...@apache.org>.
mocobeta commented on issue #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#issuecomment-574687579
Hi @irvingzhang,
> a new type of Query (KnnDelQuery) And Weight (KnnDelScoreWeight) for the dedicated deletion of exact values that only full matches the query vector;
It looks like there is semantic mismatch between the functionality and naming of the new Query and Weight. The query finds the documents which have the same value to a query vector, right? I think the class names should represent their functionality instead of use cases (e.g, for deletion).
> I'm not sure if I need to create a new issue or not.
We don't need to create another issue (until the HNSW branch is merged into the master), let's just open pull request(s) for the HNSW branch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r367305642
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90KnnGraphWriter.java
##########
@@ -216,8 +216,11 @@ private void mergeKnnGraph(FieldInfo mergeFieldInfo, final MergeState mergeState
int docid;
while ((docid = sub.nextDoc()) != NO_MORE_DOCS) {
int mappedDocId = docMap.get(docid);
+ /// deleted document (not alive)
+ if (mappedDocId < 0) {
Review comment:
Thanks @mocobeta , I have corrected the condition for deleted docIds.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368349044
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestKnnGraph.java
##########
@@ -92,7 +108,277 @@ public void testSingleDocRecall() throws Exception {
iw.commit();
assertConsistentGraph(iw, values);
- assertRecall(dir, 0, values[0]);
+ assertRecall(dir, 1, values[0]);
+ }
+ }
+
+ public void testDocsDeletionAndRecall() throws Exception {
+ /**
+ * {@code KnnExactVectorValueWeight} applies in-set (i.e. the query vector is exactly in the index)
+ * deletion strategy to filter all unmatched results searched by {@link org.apache.lucene.search.KnnGraphQuery.KnnExactVectorValueQuery},
+ * and deletes at most ef*segmentCnt vectors that are the same to the specified queryVector.
+ */
+ final class KnnExactVectorValueWeight extends ConstantScoreWeight {
+ private final String field;
+ private final ScoreMode scoreMode;
+ private final float[] queryVector;
+ private final int ef;
+
+ KnnExactVectorValueWeight(Query query, float score, ScoreMode scoreMode, String field, float[] queryVector, int ef) {
+ super(query, score);
+ this.field = field;
+ this.scoreMode = scoreMode;
+ this.queryVector = queryVector;
+ this.ef = ef;
+ }
+
+ /**
+ * Returns a {@link Scorer} which can iterate in order over all matching
+ * documents and assign them a score.
+ * <p>
+ * <b>NOTE:</b> null can be returned if no documents will be scored by this
+ * query.
+ * <p>
+ * <b>NOTE</b>: The returned {@link Scorer} does not have
+ * {@link LeafReader#getLiveDocs()} applied, they need to be checked on top.
+ *
+ * @param context the {@link LeafReaderContext} for which to return the {@link Scorer}.
+ * @return a {@link Scorer} which scores documents in/out-of order.
+ * @throws IOException if there is a low-level I/O error
+ */
+ @Override
+ public Scorer scorer(LeafReaderContext context) throws IOException {
+ ScorerSupplier supplier = scorerSupplier(context);
+ if (supplier == null) {
+ return null;
+ }
+ return supplier.get(Long.MAX_VALUE);
+ }
+
+ @Override
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ FieldInfo fi = context.reader().getFieldInfos().fieldInfo(field);
+ int numDimensions = fi.getVectorNumDimensions();
+ if (numDimensions != queryVector.length) {
+ throw new IllegalArgumentException("field=\"" + field + "\" was indexed with dimensions=" + numDimensions +
+ "; this is incompatible with query dimensions=" + queryVector.length);
+ }
+
+ final HNSWGraphReader hnswReader = new HNSWGraphReader(field, context);
+ final VectorValues vectorValues = context.reader().getVectorValues(field);
+ if (vectorValues == null) {
+ // No docs in this segment/field indexed any vector values
+ return null;
+ }
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ final Neighbors neighbors = hnswReader.searchNeighbors(queryVector, ef, vectorValues);
+
+ if (neighbors.size() > 0) {
+ Neighbor top = neighbors.top();
+ if (top.distance() > 0) {
+ neighbors.clear();
+ } else {
+ final List<Neighbor> toDeleteNeighbors = new ArrayList<>(neighbors.size());
Review comment:
Yes, and thanks.
I hope to test some cases where segments contain deleted vectors. The classes KnnExactVectorValueQuery and KnnExactVectorValueWeight are added because I expect the deleted vector values are deterministic, making the assertions meet in any execution.
The two classes are just used for my test case, so I put them in the test file.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mocobeta merged pull request #1169: LUCENE-9004: A
minor feature and patch -- support deleting vector values and fix segments
merging
Posted by GitBox <gi...@apache.org>.
mocobeta merged pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368235896
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnExactDeletionCondition.java
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * A query for deleting the exact values of the specified querying vector, used for
+ * {@link org.apache.lucene.index.IndexWriter#deleteDocuments(Query...)}.
+ */
+public class KnnExactDeletionCondition extends KnnGraphQuery {
Review comment:
Yes, your understanding is correct and highly appreciated.
1. The class KnnExactVectorValueQuery (revised name) is originally designed for deleting/updating vectors by query. When we are going to delete some vector by its exact values, Lucene seems to retrieve the similar vectors and then delete all of them, which may not be the behavior we expect. The class KnnExactVectorValueWeight is the corresponding weight for KnnExactVectorValueQuery to filter the vectors that not exactly equal to the query vector in value. Therefore, the two classes are specially designed for deletion and updating. The results that KnnGraphQuery retrieved contains that retrieved by KnnExactVectorValueWeight, so there's any necessary to establish a new query (e.g. KnnExactVectorValueQuery) for ordinary query.
2. I'm not pretty sure if there is any situation that we need to delete by query. So I moved both classes into my test case.
Now, there isn't any new feature, just some minor bug fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368235896
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnExactDeletionCondition.java
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * A query for deleting the exact values of the specified querying vector, used for
+ * {@link org.apache.lucene.index.IndexWriter#deleteDocuments(Query...)}.
+ */
+public class KnnExactDeletionCondition extends KnnGraphQuery {
Review comment:
Yes, your understanding is correct and highly appreciated.
1. The class KnnExactVectorValueQuery (revised name) is originally designed for deleting vectors by query. When we are going to delete some vector by its exact values, Lucene seems to retrieve all the similar vectors and then delete all of them, which may not be the behavior we expect. The class KnnExactVectorValueWeight is the corresponding weight for KnnExactVectorValueQuery to filter the vectors that not exactly equal to the query vector. Therefore, the two classes are designed for dedicated usage.
2. I'm not pretty sure if there is any situation that we need to delete by queries. So I moved both classes into my test case.
Now, there isn't any new feature, just some minor bug fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368235896
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnExactDeletionCondition.java
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * A query for deleting the exact values of the specified querying vector, used for
+ * {@link org.apache.lucene.index.IndexWriter#deleteDocuments(Query...)}.
+ */
+public class KnnExactDeletionCondition extends KnnGraphQuery {
Review comment:
Yes, your understanding is correct and highly appreciated.
1. The class KnnExactVectorValueQuery (revised name) is originally designed for deleting vectors by query. When we are going to delete some vector by its exact values, Lucene seems to retrieve all the similar vectors and then delete all of them, which may not be the behavior we expect. The class KnnExactVectorValueWeight is the corresponding weight for KnnExactVectorValueQuery to filter the vectors that not exactly equal to the query vector. Therefore, the two classes are designed for dedicated usage.
2. I'm not pretty sure if there is any situation that we need to delete by queries. So I moved both classes into my test case.
Now, there isn't any new feature, just some minor bug fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ErickErickson commented on issue #1169: LUCENE-9004:
A minor feature and patch -- support deleting vector values and fix
segments merging
Posted by GitBox <gi...@apache.org>.
ErickErickson commented on issue #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#issuecomment-583848170
Julie:
Moving the conversation about forceMerge over from the JIRA as per Julie.
I can imagine ways to shorten the merge process, but it'll still take quite a long time. My main concern was that I didn't know if the problem Julie was talking about was functional or not. So it sounds like the issue is "just" performance.
Ways to shorten it: First, I'm assuming you're using TieredMergePolicy, which is the default. The forceMerge(1) option _may_ rewrite any given segment multiple times. There's a limit of 30 segments merged at any given time, see maxMergeAtOnceExplicit. So say you have 300 segments, first you'd have 10 merges of 30 segments in the first pass, then another merge of the resulting segments. Each pass is a complete rewrite of the entire index. Depending on the number of segments, there could be more passes. That limit is mainly there so forceMerge doesn't consume too many resources if, say, indexing or searching are going on, but in your case I'd guess you don't care about that. So you could set it to a very large number and get it done in a single pass.
I think that's about the most savings you'd get, I don't think (but haven't measured) whether merging 150 small segments totaling 300G in a single pass is any slower or faster than merging 10 segments totaling 300G, if you wanted to try that you could set maxMergedSegmentMB. That would simply do more merging in the background during indexing to produce fewer, larger segments. Like I said, though, I don't think this will make any difference.
So my guess is that if you bump maxMergeAtOnceExplicit to a very large number, you'll cut your merge time in half (or a third or quarter, or... depending on the number of passes). It'll still take considerable time, but may be acceptable.
Best,
Erick
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] irvingzhang commented on a change in pull request
#1169: LUCENE-9004: A minor feature and patch -- support deleting vector
values and fix segments merging
Posted by GitBox <gi...@apache.org>.
irvingzhang commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368235896
##########
File path: lucene/core/src/java/org/apache/lucene/search/KnnExactDeletionCondition.java
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * A query for deleting the exact values of the specified querying vector, used for
+ * {@link org.apache.lucene.index.IndexWriter#deleteDocuments(Query...)}.
+ */
+public class KnnExactDeletionCondition extends KnnGraphQuery {
Review comment:
Yes, your understanding is correct and highly appreciated.
1. The class KnnExactVectorValueQuery (revised name) is originally designed for deleting vectors by query. When we are going to delete some vector by its exact values, Lucene seems to retrieve all the similar vectors and then delete all of them, which may not be the behavior we expect. The class KnnExactVectorValueWeight is the corresponding weight for KnnExactVectorValueQuery to filter the vectors that not exactly equal to the query vector. Therefore, the two classes are specially designed for deletion and updating. The results that KnnGraphQuery retrieved contains that retrieved by KnnExactVectorValueWeight, so there's any necessary to establish a new query (e.g. KnnExactVectorValueQuery) for ordinary query.
2. I'm not pretty sure if there is any situation that we need to delete by query. So I moved both classes into my test case.
Now, there isn't any new feature, just some minor bug fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mocobeta commented on a change in pull request #1169:
LUCENE-9004: A minor feature and patch -- support deleting vector values
and fix segments merging
Posted by GitBox <gi...@apache.org>.
mocobeta commented on a change in pull request #1169: LUCENE-9004: A minor feature and patch -- support deleting vector values and fix segments merging
URL: https://github.com/apache/lucene-solr/pull/1169#discussion_r368289244
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestKnnGraph.java
##########
@@ -92,7 +108,277 @@ public void testSingleDocRecall() throws Exception {
iw.commit();
assertConsistentGraph(iw, values);
- assertRecall(dir, 0, values[0]);
+ assertRecall(dir, 1, values[0]);
+ }
+ }
+
+ public void testDocsDeletionAndRecall() throws Exception {
+ /**
+ * {@code KnnExactVectorValueWeight} applies in-set (i.e. the query vector is exactly in the index)
+ * deletion strategy to filter all unmatched results searched by {@link org.apache.lucene.search.KnnGraphQuery.KnnExactVectorValueQuery},
+ * and deletes at most ef*segmentCnt vectors that are the same to the specified queryVector.
+ */
+ final class KnnExactVectorValueWeight extends ConstantScoreWeight {
Review comment:
Thanks, it looks almost okay to me but the Weight and Query classes can be (and should be) *static* classes.
It would look like this:
```
public class TestKnnGraph extends LuceneTestCase {
....
private static final class KnnExactVectorValueWeight extends ConstantScoreWeight {
....
}
private static final class KnnExactVectorValueQuery extends Query {
....
}
public void testDocsDeletionAndRecall() throws Exception {
....
Query query = new KnnExactVectorValueQuery(...);
....
}
....
}
```
Please avoid *non-static* inner classes whenever you can do so, because they consume extra memory and object references ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org