You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "iverase (via GitHub)" <gi...@apache.org> on 2024/07/19 11:05:04 UTC
[PR] Take advantage of the doc value skipper when it is primary sort [lucene]
iverase opened a new pull request, #13592:
URL: https://github.com/apache/lucene/pull/13592
Similar to what we do in `IndexSortSortedNumericDocValuesRangeQuery` for primary sort, we can do the same in `SortedSetDocValuesRangeQuery` and `SortedNumericDocValuesRangeQuery` when it is primary sort and there are not delete documents.
We find the first and last document using the skipper and return the dense iterator.
relates https://github.com/apache/lucene/issues/11432
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13592:
URL: https://github.com/apache/lucene/pull/13592#issuecomment-2274597503
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on code in PR #13592:
URL: https://github.com/apache/lucene/pull/13592#discussion_r1725315073
##########
lucene/core/src/java/org/apache/lucene/index/DocValuesSkipper.java:
##########
@@ -98,4 +98,29 @@ public abstract class DocValuesSkipper {
/** Return the global number of documents with a value for the field. */
public abstract int docCount();
+
+ /**
+ * Advance this skipper so that all levels intersects the range given by {@code minValue} and
+ * {@code maxValue}. If there are no intersecting levels, the skipper is exhausted.
+ *
+ * <p><b>NOTE</b>: The behavior is undefined if this method is called and {@link #advance(int)}
+ * has not been called yet.
+ */
+ public final void advance(long minValue, long maxValue) throws IOException {
+ while (true) {
Review Comment:
make sense, I wonder if we should actively advance the iterator if it hasn't been called yet.
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #13592:
URL: https://github.com/apache/lucene/pull/13592#discussion_r1725906610
##########
lucene/core/src/test/org/apache/lucene/search/TestDocValuesQueries.java:
##########
@@ -42,34 +45,100 @@
public class TestDocValuesQueries extends LuceneTestCase {
+ private Codec getCodec() {
+ // small interval size to test with many intervals
+ return TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(random().nextInt(4, 16)));
+ }
+
public void testDuelPointRangeSortedNumericRangeQuery() throws IOException {
- doTestDuelPointRangeNumericRangeQuery(true, 1);
+ doTestDuelPointRangeNumericRangeQuery(true, 1, false);
+ }
+
+ public void testDuelPointRangeSortedNumericRangeWithSlipperQuery() throws IOException {
+ doTestDuelPointRangeNumericRangeQuery(true, 1, true);
}
public void testDuelPointRangeMultivaluedSortedNumericRangeQuery() throws IOException {
- doTestDuelPointRangeNumericRangeQuery(true, 3);
+ doTestDuelPointRangeNumericRangeQuery(true, 3, false);
+ }
+
+ public void testDuelPointRangeMultivaluedSortedNumericRangeWithSkipperQuery() throws IOException {
+ doTestDuelPointRangeNumericRangeQuery(true, 3, true);
}
public void testDuelPointRangeNumericRangeQuery() throws IOException {
- doTestDuelPointRangeNumericRangeQuery(false, 1);
+ doTestDuelPointRangeNumericRangeQuery(false, 1, false);
}
- private void doTestDuelPointRangeNumericRangeQuery(boolean sortedNumeric, int maxValuesPerDoc)
- throws IOException {
+ public void testDuelPointRangeNumericRangeWithSkipperQuery() throws IOException {
+ doTestDuelPointRangeNumericRangeQuery(false, 1, true);
+ }
+
+ public void testDuelPointNumericSortedWithSkipperRangeQuery() throws IOException {
+ Directory dir = newDirectory();
+ IndexWriterConfig config = new IndexWriterConfig();
+ config.setIndexSort(new Sort(new SortField("dv", SortField.Type.LONG, random().nextBoolean())));
+ RandomIndexWriter iw = new RandomIndexWriter(random(), dir, config);
+ config.setCodec(getCodec());
Review Comment:
can you move the call to `setCodec` before the IndexWriter is created? since the codec is not supposed to be updateable (the setter is only on IndexWriterConfig, not LiveIndexWriterConfig), it would be legal for IndexWriter to cache the codec that it gets at creation time
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "gsmiller (via GitHub)" <gi...@apache.org>.
gsmiller commented on code in PR #13592:
URL: https://github.com/apache/lucene/pull/13592#discussion_r1725418294
##########
lucene/core/src/java/org/apache/lucene/index/DocValuesSkipper.java:
##########
@@ -98,4 +98,29 @@ public abstract class DocValuesSkipper {
/** Return the global number of documents with a value for the field. */
public abstract int docCount();
+
+ /**
+ * Advance this skipper so that all levels intersects the range given by {@code minValue} and
+ * {@code maxValue}. If there are no intersecting levels, the skipper is exhausted.
+ *
+ * <p><b>NOTE</b>: The behavior is undefined if this method is called and {@link #advance(int)}
+ * has not been called yet.
+ */
+ public final void advance(long minValue, long maxValue) throws IOException {
+ while (true) {
Review Comment:
Good point. I can't think of a good reason why we shouldn't do this? It would make the API a little easier for callers. I suppose it adds a very slight overhead to the call, but seems trivial.
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on PR #13592:
URL: https://github.com/apache/lucene/pull/13592#issuecomment-2244734279
I tried to find a generic method that could help here but I think the logic relies too much on the fact that the index is sorted.
For example, your mental model somewhat breaks if the range lies between too blocks so there is no intersecting block. In that case you would need to read the full index just to check there is no intersecting block.
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on PR #13592:
URL: https://github.com/apache/lucene/pull/13592#issuecomment-2247035303
I introduced the method DocValuesSkipper#advance(long,long) to advance the iterator to a matching range. wdyt?
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "gsmiller (via GitHub)" <gi...@apache.org>.
gsmiller commented on code in PR #13592:
URL: https://github.com/apache/lucene/pull/13592#discussion_r1723543404
##########
lucene/core/src/java/org/apache/lucene/index/DocValuesSkipper.java:
##########
@@ -98,4 +98,29 @@ public abstract class DocValuesSkipper {
/** Return the global number of documents with a value for the field. */
public abstract int docCount();
+
+ /**
+ * Advance this skipper so that all levels intersects the range given by {@code minValue} and
+ * {@code maxValue}. If there are no intersecting levels, the skipper is exhausted.
+ *
+ * <p><b>NOTE</b>: The behavior is undefined if this method is called and {@link #advance(int)}
+ * has not been called yet.
+ */
+ public final void advance(long minValue, long maxValue) throws IOException {
+ while (true) {
Review Comment:
If it's required that `#advance` has already been called at least once before this is used, should we add something like `assert minDocID(0) > -1`?
##########
lucene/core/src/java/org/apache/lucene/index/DocValuesSkipper.java:
##########
@@ -98,4 +98,29 @@ public abstract class DocValuesSkipper {
/** Return the global number of documents with a value for the field. */
public abstract int docCount();
+
+ /**
+ * Advance this skipper so that all levels intersects the range given by {@code minValue} and
+ * {@code maxValue}. If there are no intersecting levels, the skipper is exhausted.
+ *
+ * <p><b>NOTE</b>: The behavior is undefined if this method is called and {@link #advance(int)}
+ * has not been called yet.
+ */
+ public final void advance(long minValue, long maxValue) throws IOException {
+ while (true) {
+ if (minDocID(0) == DocIdSetIterator.NO_MORE_DOCS
+ || (minValue(0) <= maxValue && maxValue(0) >= minValue)) {
+ break;
+ } else {
+ int maxDocID = maxDocID(0);
Review Comment:
This might just be me, but I got really confused for a while until I realized that `maxDocID` does not necessarily return the docID associated with `maxValue` (in the standard ascending sort case it would, but it's reversed in the reverse sort case... assuming I'm getting this right). I had the same confusing back in the calling code in the reverse-sort case. Would you mind adding a couple comments to this code to make that a little more clear in case someone else suffers from my same confusion?
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on code in PR #13592:
URL: https://github.com/apache/lucene/pull/13592#discussion_r1726673384
##########
lucene/core/src/test/org/apache/lucene/search/TestDocValuesQueries.java:
##########
@@ -42,34 +45,100 @@
public class TestDocValuesQueries extends LuceneTestCase {
+ private Codec getCodec() {
+ // small interval size to test with many intervals
+ return TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(random().nextInt(4, 16)));
+ }
+
public void testDuelPointRangeSortedNumericRangeQuery() throws IOException {
- doTestDuelPointRangeNumericRangeQuery(true, 1);
+ doTestDuelPointRangeNumericRangeQuery(true, 1, false);
+ }
+
+ public void testDuelPointRangeSortedNumericRangeWithSlipperQuery() throws IOException {
+ doTestDuelPointRangeNumericRangeQuery(true, 1, true);
}
public void testDuelPointRangeMultivaluedSortedNumericRangeQuery() throws IOException {
- doTestDuelPointRangeNumericRangeQuery(true, 3);
+ doTestDuelPointRangeNumericRangeQuery(true, 3, false);
+ }
+
+ public void testDuelPointRangeMultivaluedSortedNumericRangeWithSkipperQuery() throws IOException {
+ doTestDuelPointRangeNumericRangeQuery(true, 3, true);
}
public void testDuelPointRangeNumericRangeQuery() throws IOException {
- doTestDuelPointRangeNumericRangeQuery(false, 1);
+ doTestDuelPointRangeNumericRangeQuery(false, 1, false);
}
- private void doTestDuelPointRangeNumericRangeQuery(boolean sortedNumeric, int maxValuesPerDoc)
- throws IOException {
+ public void testDuelPointRangeNumericRangeWithSkipperQuery() throws IOException {
+ doTestDuelPointRangeNumericRangeQuery(false, 1, true);
+ }
+
+ public void testDuelPointNumericSortedWithSkipperRangeQuery() throws IOException {
+ Directory dir = newDirectory();
+ IndexWriterConfig config = new IndexWriterConfig();
+ config.setIndexSort(new Sort(new SortField("dv", SortField.Type.LONG, random().nextBoolean())));
+ RandomIndexWriter iw = new RandomIndexWriter(random(), dir, config);
+ config.setCodec(getCodec());
Review Comment:
sure!
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on code in PR #13592:
URL: https://github.com/apache/lucene/pull/13592#discussion_r1726675935
##########
lucene/core/src/java/org/apache/lucene/index/DocValuesSkipper.java:
##########
@@ -98,4 +98,29 @@ public abstract class DocValuesSkipper {
/** Return the global number of documents with a value for the field. */
public abstract int docCount();
+
+ /**
+ * Advance this skipper so that all levels intersects the range given by {@code minValue} and
+ * {@code maxValue}. If there are no intersecting levels, the skipper is exhausted.
+ *
+ * <p><b>NOTE</b>: The behavior is undefined if this method is called and {@link #advance(int)}
+ * has not been called yet.
+ */
+ public final void advance(long minValue, long maxValue) throws IOException {
+ while (true) {
+ if (minDocID(0) == DocIdSetIterator.NO_MORE_DOCS
+ || (minValue(0) <= maxValue && maxValue(0) >= minValue)) {
+ break;
+ } else {
+ int maxDocID = maxDocID(0);
Review Comment:
I added some comments which hopefully makes it more clear.
--
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on code in PR #13592:
URL: https://github.com/apache/lucene/pull/13592#discussion_r1726674931
##########
lucene/core/src/java/org/apache/lucene/index/DocValuesSkipper.java:
##########
@@ -98,4 +98,29 @@ public abstract class DocValuesSkipper {
/** Return the global number of documents with a value for the field. */
public abstract int docCount();
+
+ /**
+ * Advance this skipper so that all levels intersects the range given by {@code minValue} and
+ * {@code maxValue}. If there are no intersecting levels, the skipper is exhausted.
+ *
+ * <p><b>NOTE</b>: The behavior is undefined if this method is called and {@link #advance(int)}
+ * has not been called yet.
+ */
+ public final void advance(long minValue, long maxValue) throws IOException {
+ while (true) {
Review Comment:
I have changed the API so we accept unpositioned skippers to call this 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.
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
Re: [PR] Take advantage of the doc value skipper when it is primary sort [lucene]
Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase merged PR #13592:
URL: https://github.com/apache/lucene/pull/13592
--
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