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 2021/09/07 15:44:37 UTC
[GitHub] [lucene] mayya-sharipova opened a new pull request #286: LUCENE-10087 Disable numeric sort optim when needed
mayya-sharipova opened a new pull request #286:
URL: https://github.com/apache/lucene/pull/286
Add a method to SortField that allows to disable numeric sort optimization,
which is enabled by default from 9.0.
--
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] rmuir commented on a change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704440549
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -130,6 +130,9 @@
// Used for 'sortMissingFirst/Last'
protected Object missingValue = null;
+ // Indicates if numeric sort optimization is disabled. Enabled by default.
+ private boolean pointSortOptimizationDisabled = false;
Review comment:
Can we make this "positive" to avoid double negatives? I suggest `private boolean optimizePoints = true;` It should also be deprecated.
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,26 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization to use the Points index to skip over non-competitive
+ * documents. By default sorting on a numeric field activates point sort optimization that can
+ * efficiently skip non-competitive hits. Sort optimization has a number of requirements, one of
+ * which is that SortField.Type matches the Point type with which the field was indexed (e.g. sort
+ * on IntPoint field should use SortField.Type.INT). Another requirement is that the same data is
+ * indexed with points and doc values for the field.
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ *
+ * @deprecated should only be used for compatibility with 8.x indices that got created with
+ * inconsistent data across fields, or the wrong sort configuration in the index sort
+ */
+ @Deprecated // Remove in Lucene 9
+ public void disablePointSortOptimization() {
+ this.pointSortOptimizationDisabled = true;
+ }
+
+ protected boolean pointSortOptimizationDisabled() {
Review comment:
Can we make this an ordinary getter? I suggest `getOptimizePoints()`. It should be deprecated and public IMO.
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,26 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization to use the Points index to skip over non-competitive
+ * documents. By default sorting on a numeric field activates point sort optimization that can
+ * efficiently skip non-competitive hits. Sort optimization has a number of requirements, one of
+ * which is that SortField.Type matches the Point type with which the field was indexed (e.g. sort
+ * on IntPoint field should use SortField.Type.INT). Another requirement is that the same data is
+ * indexed with points and doc values for the field.
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ *
+ * @deprecated should only be used for compatibility with 8.x indices that got created with
+ * inconsistent data across fields, or the wrong sort configuration in the index sort
+ */
+ @Deprecated // Remove in Lucene 9
+ public void disablePointSortOptimization() {
+ this.pointSortOptimizationDisabled = true;
+ }
Review comment:
Can we make this an ordinary setter? I suggest `setOptimizePoints(boolean)`.
--
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] mayya-sharipova commented on a change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704358433
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,20 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization. By default sorting on a numeric field activates sort
+ * optimization that can efficiently skip non-competitive hits. Sort optimization has a number of
+ * requirements, one of which is that SortField.Type matches the Point type with which the field
+ * was indexed (e.g. sort on IntPoint field should use SortField.Type.INT).
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ */
+ public void disableSortOptimization() {
+ this.sortOptimizationDisabled = true;
+ }
+
+ protected boolean sortOptimizationDisabled() {
+ return sortOptimizationDisabled;
+ }
Review comment:
Addressed in 9fe721c02ecefd5cee261da4c64b4b785a044f88
--
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] mayya-sharipova commented on pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on pull request #286:
URL: https://github.com/apache/lucene/pull/286#issuecomment-915181037
@romseygeek Thanks for the review.
@jpountz I've addressed your comments. Can you please continue the review.
--
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] mayya-sharipova commented on a change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704707532
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,26 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization to use the Points index to skip over non-competitive
+ * documents. By default sorting on a numeric field activates point sort optimization that can
+ * efficiently skip non-competitive hits. Sort optimization has a number of requirements, one of
+ * which is that SortField.Type matches the Point type with which the field was indexed (e.g. sort
+ * on IntPoint field should use SortField.Type.INT). Another requirement is that the same data is
+ * indexed with points and doc values for the field.
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ *
+ * @deprecated should only be used for compatibility with 8.x indices that got created with
+ * inconsistent data across fields, or the wrong sort configuration in the index sort
+ */
+ @Deprecated // Remove in Lucene 9
+ public void disablePointSortOptimization() {
+ this.pointSortOptimizationDisabled = true;
+ }
Review comment:
@rmuir Thanks for a suggestion. What we wanted is to have sort optimization to be always enabled for a request, unless a user explicitly disables it. We don't have a use case for `setOptimizePoints(true)`, that's why we decided to call this method `disablePointSortOptimization()`.
--
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 change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704360843
##########
File path: lucene/CHANGES.txt
##########
@@ -141,6 +141,10 @@ API Changes
The Weight#count API represents a cleaner way for Query classes to optimize their counting method.
(Gautam Worah, Adrien Grand)
+* LUCENE-10089 Add a method to SortField that allows to disable numeric sort
Review comment:
```suggestion
* LUCENE-10089: Add a method to SortField that allows to disable numeric sort
```
--
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] mayya-sharipova commented on a change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r705294105
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,26 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization to use the Points index to skip over non-competitive
+ * documents. By default sorting on a numeric field activates point sort optimization that can
+ * efficiently skip non-competitive hits. Sort optimization has a number of requirements, one of
+ * which is that SortField.Type matches the Point type with which the field was indexed (e.g. sort
+ * on IntPoint field should use SortField.Type.INT). Another requirement is that the same data is
+ * indexed with points and doc values for the field.
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ *
+ * @deprecated should only be used for compatibility with 8.x indices that got created with
+ * inconsistent data across fields, or the wrong sort configuration in the index sort
+ */
+ @Deprecated // Remove in Lucene 9
+ public void disablePointSortOptimization() {
+ this.pointSortOptimizationDisabled = true;
+ }
Review comment:
Thanks @rmuir and @jpountz for the additional comments. Addressed in a355c172abb9b1ec3f202de31f74e710d5936d0c
--
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] rmuir commented on a change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704816931
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,26 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization to use the Points index to skip over non-competitive
+ * documents. By default sorting on a numeric field activates point sort optimization that can
+ * efficiently skip non-competitive hits. Sort optimization has a number of requirements, one of
+ * which is that SortField.Type matches the Point type with which the field was indexed (e.g. sort
+ * on IntPoint field should use SortField.Type.INT). Another requirement is that the same data is
+ * indexed with points and doc values for the field.
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ *
+ * @deprecated should only be used for compatibility with 8.x indices that got created with
+ * inconsistent data across fields, or the wrong sort configuration in the index sort
+ */
+ @Deprecated // Remove in Lucene 9
+ public void disablePointSortOptimization() {
+ this.pointSortOptimizationDisabled = true;
+ }
Review comment:
personally i think the default is handled by the boolean defaulting to `true` ? Just thinking that an ordinary getter/setter is the most obvious way to control it to have an intuitive API. It already looks much better
--
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] mayya-sharipova commented on a change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704732855
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -130,6 +130,9 @@
// Used for 'sortMissingFirst/Last'
protected Object missingValue = null;
+ // Indicates if numeric sort optimization is disabled. Enabled by default.
+ private boolean pointSortOptimizationDisabled = false;
Review comment:
Thanks for comment, addressed in 23724f72057655ca51996570e6504f54d75af307
--
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] mayya-sharipova merged pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
mayya-sharipova merged pull request #286:
URL: https://github.com/apache/lucene/pull/286
--
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 change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704166498
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,20 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization. By default sorting on a numeric field activates sort
+ * optimization that can efficiently skip non-competitive hits. Sort optimization has a number of
+ * requirements, one of which is that SortField.Type matches the Point type with which the field
+ * was indexed (e.g. sort on IntPoint field should use SortField.Type.INT).
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ */
+ public void disableSortOptimization() {
+ this.sortOptimizationDisabled = true;
+ }
+
+ protected boolean sortOptimizationDisabled() {
+ return sortOptimizationDisabled;
+ }
Review comment:
I think we should be more specific regarding what optimization we are talking about. Maybe call it something like `pointSortOptimization` and update javadocs to say something like `Disable numeric sort optimization to use the Points index to skip over non-competitive documents`?
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,20 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization. By default sorting on a numeric field activates sort
+ * optimization that can efficiently skip non-competitive hits. Sort optimization has a number of
+ * requirements, one of which is that SortField.Type matches the Point type with which the field
+ * was indexed (e.g. sort on IntPoint field should use SortField.Type.INT).
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ */
+ public void disableSortOptimization() {
Review comment:
I wonder if we should mark it `@Deprecated` and highlight in javadocs that this should only be used for compatibility with 8.x indices that got created with inconsistent data across fields, or the wrong sort configuration in the index sort?
--
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 change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r705237964
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,26 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization to use the Points index to skip over non-competitive
+ * documents. By default sorting on a numeric field activates point sort optimization that can
+ * efficiently skip non-competitive hits. Sort optimization has a number of requirements, one of
+ * which is that SortField.Type matches the Point type with which the field was indexed (e.g. sort
+ * on IntPoint field should use SortField.Type.INT). Another requirement is that the same data is
+ * indexed with points and doc values for the field.
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ *
+ * @deprecated should only be used for compatibility with 8.x indices that got created with
+ * inconsistent data across fields, or the wrong sort configuration in the index sort
+ */
+ @Deprecated // Remove in Lucene 9
+ public void disablePointSortOptimization() {
+ this.pointSortOptimizationDisabled = true;
+ }
Review comment:
I have a preference for a traditional getter/setter too.
--
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] mayya-sharipova commented on a change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704733017
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,26 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization to use the Points index to skip over non-competitive
+ * documents. By default sorting on a numeric field activates point sort optimization that can
+ * efficiently skip non-competitive hits. Sort optimization has a number of requirements, one of
+ * which is that SortField.Type matches the Point type with which the field was indexed (e.g. sort
+ * on IntPoint field should use SortField.Type.INT). Another requirement is that the same data is
+ * indexed with points and doc values for the field.
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ *
+ * @deprecated should only be used for compatibility with 8.x indices that got created with
+ * inconsistent data across fields, or the wrong sort configuration in the index sort
+ */
+ @Deprecated // Remove in Lucene 9
+ public void disablePointSortOptimization() {
+ this.pointSortOptimizationDisabled = true;
+ }
+
+ protected boolean pointSortOptimizationDisabled() {
Review comment:
Thanks for comment, addressed in 23724f72057655ca51996570e6504f54d75af307
--
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] mayya-sharipova commented on a change in pull request #286: LUCENE-10089 Disable numeric sort optim when needed
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #286:
URL: https://github.com/apache/lucene/pull/286#discussion_r704358532
##########
File path: lucene/core/src/java/org/apache/lucene/search/SortField.java
##########
@@ -606,4 +609,20 @@ public IndexSorter getIndexSorter() {
return null;
}
}
+
+ /**
+ * Disable numeric sort optimization. By default sorting on a numeric field activates sort
+ * optimization that can efficiently skip non-competitive hits. Sort optimization has a number of
+ * requirements, one of which is that SortField.Type matches the Point type with which the field
+ * was indexed (e.g. sort on IntPoint field should use SortField.Type.INT).
+ *
+ * <p>This allows to disable sort optimization, in cases where these requirements can't be met.
+ */
+ public void disableSortOptimization() {
Review comment:
Addressed in 9fe721c02ecefd5cee261da4c64b4b785a044f88
--
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