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 2022/09/20 09:11:07 UTC
[GitHub] [lucene] javanna opened a new pull request, #11794: Guard FieldExistsQuery against null pointers
javanna opened a new pull request, #11794:
URL: https://github.com/apache/lucene/pull/11794
FieldExistsQuery checks if there are points for a certain field, and then retrieves the corresponding point values. When all documents that had points for a certain field have been deleted from a certain segments, as well as merged away, field info may report that there are points yet the corresponding point values are null.
With this change we add a null check in FieldExistsQuery. Long term, we will likely want to prevent this situation from happening.
Relates #11393
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz merged pull request #11794: Guard FieldExistsQuery against null pointers
Posted by GitBox <gi...@apache.org>.
jpountz merged PR #11794:
URL: https://github.com/apache/lucene/pull/11794
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] javanna commented on a diff in pull request #11794: Guard FieldExistsQuery against null pointers
Posted by GitBox <gi...@apache.org>.
javanna commented on code in PR #11794:
URL: https://github.com/apache/lucene/pull/11794#discussion_r975098043
##########
lucene/core/src/test/org/apache/lucene/search/TestFieldExistsQuery.java:
##########
@@ -702,6 +702,28 @@ private float[] randomVector(int dim) {
return v;
}
+ public void testDeleteAllPointDocs() throws Exception {
+ try (Directory dir = newDirectory();
+ RandomIndexWriter iw = new RandomIndexWriter(random(), dir)) {
+
+ Document doc = new Document();
+ doc.add(new StringField("id", "0", Field.Store.NO));
+ doc.add(new LongPoint("long", 17));
+ doc.add(new NumericDocValuesField("long", 17));
+ iw.addDocument(doc);
+ iw.addDocument(new Document());
+ iw.commit();
+
+ iw.deleteDocuments(new Term("id", "0"));
+ iw.forceMerge(1);
Review Comment:
good point, I pushed an update
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz commented on a diff in pull request #11794: Guard FieldExistsQuery against null pointers
Posted by GitBox <gi...@apache.org>.
jpountz commented on code in PR #11794:
URL: https://github.com/apache/lucene/pull/11794#discussion_r975170911
##########
lucene/core/src/test/org/apache/lucene/search/TestFieldExistsQuery.java:
##########
@@ -711,13 +711,15 @@ public void testDeleteAllPointDocs() throws Exception {
doc.add(new LongPoint("long", 17));
doc.add(new NumericDocValuesField("long", 17));
iw.addDocument(doc);
+ iw.flush();
iw.addDocument(new Document());
iw.commit();
iw.deleteDocuments(new Term("id", "0"));
iw.forceMerge(1);
try (IndexReader reader = iw.getReader()) {
+ assert reader.leaves().size() == 1 && reader.hasDeletions() == false;
Review Comment:
use `assertTrue` so that it does rely on assertions being enabled?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz commented on a diff in pull request #11794: Guard FieldExistsQuery against null pointers
Posted by GitBox <gi...@apache.org>.
jpountz commented on code in PR #11794:
URL: https://github.com/apache/lucene/pull/11794#discussion_r975269403
##########
lucene/core/src/test/org/apache/lucene/search/TestFieldExistsQuery.java:
##########
@@ -702,6 +704,30 @@ private float[] randomVector(int dim) {
return v;
}
+ public void testDeleteAllPointDocs() throws Exception {
+ try (Directory dir = newDirectory();
+ RandomIndexWriter iw = new RandomIndexWriter(random(), dir)) {
+
+ Document doc = new Document();
+ doc.add(new StringField("id", "0", Field.Store.NO));
+ doc.add(new LongPoint("long", 17));
+ doc.add(new NumericDocValuesField("long", 17));
+ iw.addDocument(doc);
+ iw.flush();
Review Comment:
you should also add another document before the flush, otherwise the segment only has the document that you want to delete and the merge simply ignores the segment without carrying over its field infos
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] javanna commented on a diff in pull request #11794: Guard FieldExistsQuery against null pointers
Posted by GitBox <gi...@apache.org>.
javanna commented on code in PR #11794:
URL: https://github.com/apache/lucene/pull/11794#discussion_r975282324
##########
lucene/core/src/test/org/apache/lucene/search/TestFieldExistsQuery.java:
##########
@@ -702,6 +704,30 @@ private float[] randomVector(int dim) {
return v;
}
+ public void testDeleteAllPointDocs() throws Exception {
+ try (Directory dir = newDirectory();
+ RandomIndexWriter iw = new RandomIndexWriter(random(), dir)) {
+
+ Document doc = new Document();
+ doc.add(new StringField("id", "0", Field.Store.NO));
+ doc.add(new LongPoint("long", 17));
+ doc.add(new NumericDocValuesField("long", 17));
+ iw.addDocument(doc);
+ iw.flush();
Review Comment:
oh boy!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz commented on a diff in pull request #11794: Guard FieldExistsQuery against null pointers
Posted by GitBox <gi...@apache.org>.
jpountz commented on code in PR #11794:
URL: https://github.com/apache/lucene/pull/11794#discussion_r975093030
##########
lucene/core/src/test/org/apache/lucene/search/TestFieldExistsQuery.java:
##########
@@ -702,6 +702,28 @@ private float[] randomVector(int dim) {
return v;
}
+ public void testDeleteAllPointDocs() throws Exception {
+ try (Directory dir = newDirectory();
+ RandomIndexWriter iw = new RandomIndexWriter(random(), dir)) {
+
+ Document doc = new Document();
+ doc.add(new StringField("id", "0", Field.Store.NO));
+ doc.add(new LongPoint("long", 17));
+ doc.add(new NumericDocValuesField("long", 17));
+ iw.addDocument(doc);
+ iw.addDocument(new Document());
+ iw.commit();
+
+ iw.deleteDocuments(new Term("id", "0"));
+ iw.forceMerge(1);
Review Comment:
Do we need to add an `iw.flush()` call between the two `addDocument` calls? Otherwise there might already be a single segment and forceMerge will be a no-op. Maybe add an assertion below that the reader has a single segment without deletions?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org