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/04 14:28:23 UTC

[GitHub] [solr] madrob commented on a change in pull request #529: SOLR-12336 Remove Filter from Solr

madrob commented on a change in pull request #529:
URL: https://github.com/apache/solr/pull/529#discussion_r799502566



##########
File path: solr/core/src/java/org/apache/solr/query/FilterQuery.java
##########
@@ -89,13 +87,13 @@ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float bo
 
     if (!(searcher instanceof SolrIndexSearcher)) {
       // delete-by-query won't have SolrIndexSearcher
-      return new BoostQuery(new ConstantScoreQuery(q), 0).createWeight(searcher, scoreMode, 1f);
+      return new ConstantScoreQuery(q).createWeight(searcher, scoreMode, 1f);

Review comment:
       @dsmiley Does this take care of SOLR-14800 as well?

##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.solr.search;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.ConstantScoreScorer;
+import org.apache.lucene.search.ConstantScoreWeight;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.Weight;
+
+import java.io.IOException;
+import java.util.Objects;
+
+/**
+ * A class that accesses Queries based on a DocSet
+ *
+ * Refer SOLR-15257
+ */
+public class DocSetQuery extends Query implements DocSetProducer{
+    private final DocSet docSet;
+
+    public DocSetQuery(DocSet docSet) {
+        super();
+        this.docSet = docSet;
+    }
+
+    @Override
+    public String toString(String field) {
+        return "DocSetQuery(" + field + ")";
+    }
+
+    @Override
+    public void visit(QueryVisitor visitor) {
+        visitor.visitLeaf(this);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        return sameClassAs(obj) && equalsTo(getClass().cast(obj));
+    }
+
+    private boolean equalsTo(DocSetQuery other) {
+        return Objects.equals(docSet, other.docSet);
+    }
+
+    @Override
+    public int hashCode() {
+        return classHash() * 31 + docSet.hashCode();
+    }
+
+    /**
+     * @param searcher is not used because we already have a DocSet created in DocSetQuery
+     * @return the DocSet created in DocSetQuery
+     */
+    @Override
+    public DocSet createDocSet(SolrIndexSearcher searcher) throws IOException {
+        return docSet;
+    }
+
+    @Override
+    public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
+        return new ConstantScoreWeight(this, 0) {

Review comment:
       Should this be zero or the provided boost?

##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.solr.search;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.ConstantScoreScorer;
+import org.apache.lucene.search.ConstantScoreWeight;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.Weight;
+
+import java.io.IOException;
+import java.util.Objects;
+
+/**
+ * A class that accesses Queries based on a DocSet
+ *
+ * Refer SOLR-15257
+ */
+public class DocSetQuery extends Query implements DocSetProducer{
+    private final DocSet docSet;
+
+    public DocSetQuery(DocSet docSet) {
+        super();
+        this.docSet = docSet;
+    }
+
+    @Override
+    public String toString(String field) {
+        return "DocSetQuery(" + field + ")";
+    }
+
+    @Override
+    public void visit(QueryVisitor visitor) {
+        visitor.visitLeaf(this);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        return sameClassAs(obj) && equalsTo(getClass().cast(obj));
+    }
+
+    private boolean equalsTo(DocSetQuery other) {
+        return Objects.equals(docSet, other.docSet);
+    }
+
+    @Override
+    public int hashCode() {
+        return classHash() * 31 + docSet.hashCode();
+    }
+
+    /**
+     * @param searcher is not used because we already have a DocSet created in DocSetQuery
+     * @return the DocSet created in DocSetQuery
+     */
+    @Override
+    public DocSet createDocSet(SolrIndexSearcher searcher) throws IOException {
+        return docSet;
+    }
+
+    @Override
+    public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
+        return new ConstantScoreWeight(this, 0) {
+            @Override
+            public Scorer scorer(LeafReaderContext context) throws IOException {
+                DocIdSetIterator disi = docSet.iterator(context);

Review comment:
       If you check the docset for null here, then you can simplify some of the implementations in TestFilteredDocIdSet.java, but let's leave that for another time.

##########
File path: solr/core/src/java/org/apache/solr/search/facet/FacetRequest.java
##########
@@ -229,19 +230,19 @@ public static void createJoinField(FacetRequest.Domain domain, Map<String,Object
        * Creates a Query that can be used to recompute the new "base" for this domain, relative to the
        * current base of the FacetContext.
        */
-      public Query createDomainQuery(FacetContext fcontext) throws IOException {
+      public Query createDomainQuery(FacetContext fcontext) {
         // NOTE: this code lives here, instead of in FacetProcessor.handleJoin, in order to minimize
         // the number of classes that have to know about the number of possible settings on the join
         // (ie: if we add a score mode, or some other modifier to how the joins are done)
 
-        final SolrConstantScoreQuery fromQuery = new SolrConstantScoreQuery(fcontext.base.getTopFilter());
+        final ConstantScoreQuery fromQuery = new ConstantScoreQuery(fcontext.base.makeQuery());

Review comment:
       Do we need to wrap this in a constant score query?

##########
File path: solr/core/src/java/org/apache/solr/query/FilterQuery.java
##########
@@ -89,13 +87,13 @@ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float bo
 
     if (!(searcher instanceof SolrIndexSearcher)) {
       // delete-by-query won't have SolrIndexSearcher
-      return new BoostQuery(new ConstantScoreQuery(q), 0).createWeight(searcher, scoreMode, 1f);
+      return new ConstantScoreQuery(q).createWeight(searcher, scoreMode, 1f);
     }
 
     SolrIndexSearcher solrSearcher = (SolrIndexSearcher)searcher;
     DocSet docs = solrSearcher.getDocSet(q);
     // reqInfo.addCloseHook(docs);  // needed for off-heap refcounting
 
-    return new BoostQuery(new SolrConstantScoreQuery(docs.getTopFilter()), 0).createWeight(searcher, scoreMode, 1f);
+    return docs.makeQuery().createWeight(searcher, scoreMode, 1f);

Review comment:
       I'm confused what is going on here. We take the query and get the docs and then use the docs to make a query to get a constant score weight. This seems wasteful but maybe there is some nuance in the processing that I'm missing.

##########
File path: solr/core/src/java/org/apache/solr/search/facet/FacetRequest.java
##########
@@ -287,24 +288,23 @@ public static void createGraphField(FacetRequest.Domain domain, Map<String,Objec
        * Creates a Query that can be used to recompute the new "base" for this domain, relative to the
        * current base of the FacetContext.
        */
-      public Query createDomainQuery(FacetContext fcontext) throws IOException {
-        final SolrConstantScoreQuery fromQuery = new SolrConstantScoreQuery(fcontext.base.getTopFilter());
+      public Query createDomainQuery(FacetContext fcontext) {
+        final ConstantScoreQuery fromQuery = new ConstantScoreQuery(fcontext.base.makeQuery());

Review comment:
       Do we need to wrap this in a constant score query?

##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.solr.search;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.ConstantScoreScorer;
+import org.apache.lucene.search.ConstantScoreWeight;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.Weight;
+
+import java.io.IOException;
+import java.util.Objects;
+
+/**
+ * A class that accesses Queries based on a DocSet
+ *
+ * Refer SOLR-15257
+ */
+public class DocSetQuery extends Query implements DocSetProducer{
+    private final DocSet docSet;
+
+    public DocSetQuery(DocSet docSet) {
+        super();
+        this.docSet = docSet;
+    }
+
+    @Override
+    public String toString(String field) {
+        return "DocSetQuery(" + field + ")";
+    }
+
+    @Override
+    public void visit(QueryVisitor visitor) {
+        visitor.visitLeaf(this);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        return sameClassAs(obj) && equalsTo(getClass().cast(obj));
+    }
+
+    private boolean equalsTo(DocSetQuery other) {
+        return Objects.equals(docSet, other.docSet);
+    }
+
+    @Override
+    public int hashCode() {
+        return classHash() * 31 + docSet.hashCode();

Review comment:
       docSet can be null




-- 
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