You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/02/15 04:39:27 UTC

[GitHub] [solr] dsmiley opened a new pull request #638: SOLR-12336: DocSetQuery can have 1 score

dsmiley opened a new pull request #638:
URL: https://github.com/apache/solr/pull/638


   ... and some misc adjustments
   
   https://issues.apache.org/jira/browse/SOLR-12336
   
   Mostly; instead of making DocSetQuery score as 0, make FilteredQuery do that.  Since FilteredQuery wraps DocSetQuery, there's no difference for FilteredQuery.  DocSetQuery is more internal.  It's users don't see to care about the score any way.


-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r808410660



##########
File path: solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
##########
@@ -311,7 +311,5 @@ Example has been provided in `sample_techproducts_configs` to override content-t
 * SOLR-15124: Removed three core level admin API endpoints because they are already registered at the node level
 where they really belong: /admin/threads, /admin/properties, /admin/logging
 
-* SOLR-12336: Remove Filter, SolrFilter and SolrConstantScoreQuery

Review comment:
       Yes.  Plugin writers will find out one way or another :-). Maybe they will read CHANGES.txt.  The audience of this page is our users.




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r806440724



##########
File path: solr/core/src/java/org/apache/solr/query/FilterQuery.java
##########
@@ -68,7 +74,7 @@ public String toString(String field) {
 
   @Override
   public void visit(QueryVisitor visitor) {
-    q.visit(visitor);
+    q.visit(visitor.getSubVisitor(BooleanClause.Occur.FILTER, this));

Review comment:
       CSQ does this; makes sense.

##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -2075,12 +2075,10 @@ public int numDocs(Query a, DocSet b) throws IOException {
       return a == absQ ? b.intersectionSize(positiveA) : b.andNotSize(positiveA);
     } else {
       // If there isn't a cache, then do a single filtered query
-      // NOTE: we cannot use FilteredQuery, because BitDocSet assumes it will never

Review comment:
       obsolete comment; FilteredQuery hasn't existed in quite some time, and even then I'm a bit confused by the comment as this doesn't use UninvertedField so who cares.




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] Caa52 commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
Caa52 commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r809291799



##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -78,11 +78,9 @@ public DocSet createDocSet(SolrIndexSearcher searcher) throws IOException {
 
     @Override
     public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
-        //This should probably use the provided boost as scorer. However, that causes
-        // TestSolrQueryParser.testFilter to fail.
-        return new ConstantScoreWeight(this, 0) {
+        return new ConstantScoreWeight(this, boost) {

Review comment:
       Ah, ok got it, thank you




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r808410660



##########
File path: solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
##########
@@ -311,7 +311,5 @@ Example has been provided in `sample_techproducts_configs` to override content-t
 * SOLR-15124: Removed three core level admin API endpoints because they are already registered at the node level
 where they really belong: /admin/threads, /admin/properties, /admin/logging
 
-* SOLR-12336: Remove Filter, SolrFilter and SolrConstantScoreQuery

Review comment:
       Yes.  Plugin writers will find out one way or another :-). Maybe they will read CHANGES.txt.  The audience of this page is users our users.




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] madrob commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r806962695



##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -31,9 +31,10 @@
 import java.util.Objects;
 
 /**
- * A class that accesses Queries based on a DocSet
+ * A Query based on a {@link DocSet}.  The un-boosted score is always 1.
  *
- * Refer SOLR-15257
+ * @see DocSet#makeQuery()
+ * @since 9.0
  */
 public class DocSetQuery extends Query implements DocSetProducer{

Review comment:
       Would this class make sense as not public?




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] madrob commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r806962695



##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -31,9 +31,10 @@
 import java.util.Objects;
 
 /**
- * A class that accesses Queries based on a DocSet
+ * A Query based on a {@link DocSet}.  The un-boosted score is always 1.
  *
- * Refer SOLR-15257
+ * @see DocSet#makeQuery()
+ * @since 9.0
  */
 public class DocSetQuery extends Query implements DocSetProducer{

Review comment:
       Would this class make sense as not public?




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] Caa52 commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
Caa52 commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r808332415



##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -78,11 +78,9 @@ public DocSet createDocSet(SolrIndexSearcher searcher) throws IOException {
 
     @Override
     public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
-        //This should probably use the provided boost as scorer. However, that causes
-        // TestSolrQueryParser.testFilter to fail.
-        return new ConstantScoreWeight(this, 0) {
+        return new ConstantScoreWeight(this, boost) {

Review comment:
       When I attempted refactoring this like so, `TestSolrQueryParser.testFilter` was failing. Granted, I didn't make the changes in FilterQuery. I am not sure I completely follow why the test now passes after making the changes in FilterQuery. Please explain. 




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley merged pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
dsmiley merged pull request #638:
URL: https://github.com/apache/solr/pull/638


   


-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r808411400



##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -78,11 +78,9 @@ public DocSet createDocSet(SolrIndexSearcher searcher) throws IOException {
 
     @Override
     public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
-        //This should probably use the provided boost as scorer. However, that causes
-        // TestSolrQueryParser.testFilter to fail.
-        return new ConstantScoreWeight(this, 0) {
+        return new ConstantScoreWeight(this, boost) {

Review comment:
       The net change is no difference for that test -- it's FilterQuery -> DocSetQuery and thus if *either* are scored zero, then zero times whatever is zero.




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] madrob commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r808143858



##########
File path: solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
##########
@@ -311,7 +311,5 @@ Example has been provided in `sample_techproducts_configs` to override content-t
 * SOLR-15124: Removed three core level admin API endpoints because they are already registered at the node level
 where they really belong: /admin/threads, /admin/properties, /admin/logging
 
-* SOLR-12336: Remove Filter, SolrFilter and SolrConstantScoreQuery

Review comment:
       Are those all internal? It feels like a significant API change for... somebody. Plugin writers?




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r807293187



##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -31,9 +31,10 @@
 import java.util.Objects;
 
 /**
- * A class that accesses Queries based on a DocSet
+ * A Query based on a {@link DocSet}.  The un-boosted score is always 1.
  *
- * Refer SOLR-15257
+ * @see DocSet#makeQuery()
+ * @since 9.0
  */
 public class DocSetQuery extends Query implements DocSetProducer{

Review comment:
       I agree; I've been thinking the same thing




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a change in pull request #638: SOLR-12336: DocSetQuery can have 1 score

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #638:
URL: https://github.com/apache/solr/pull/638#discussion_r806440724



##########
File path: solr/core/src/java/org/apache/solr/query/FilterQuery.java
##########
@@ -68,7 +74,7 @@ public String toString(String field) {
 
   @Override
   public void visit(QueryVisitor visitor) {
-    q.visit(visitor);
+    q.visit(visitor.getSubVisitor(BooleanClause.Occur.FILTER, this));

Review comment:
       CSQ does this; makes sense.

##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -2075,12 +2075,10 @@ public int numDocs(Query a, DocSet b) throws IOException {
       return a == absQ ? b.intersectionSize(positiveA) : b.andNotSize(positiveA);
     } else {
       // If there isn't a cache, then do a single filtered query
-      // NOTE: we cannot use FilteredQuery, because BitDocSet assumes it will never

Review comment:
       obsolete comment; FilteredQuery hasn't existed in quite some time, and even then I'm a bit confused by the comment as this doesn't use UninvertedField so who cares.




-- 
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@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org