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/01/19 03:30:07 UTC
[GitHub] [solr] dsmiley commented on a change in pull request #529: SOLR-15257 Add DocSetQuery in lieu of DocSet.getTopFilter
dsmiley commented on a change in pull request #529:
URL: https://github.com/apache/solr/pull/529#discussion_r787319124
##########
File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.DocIdSetIterator;
+import org.apache.lucene.search.IndexSearcher;
+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;
+
+public class DocSetQuery extends SolrConstantScoreQuery {
+ protected final DocSet docSet;
+
+ public DocSetQuery(DocSet docSet) {
+ super();
+ this.docSet = docSet;
+ }
+
+ public DocSet getDocSet() {
+ return 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) && Objects.equals(docSet, getClass().cast(obj).docSet);
+ }
+
+ @Override
+ public int hashCode() {
+ return classHash() * 31 + docSet.hashCode();
+ }
+
+ @Override
+ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
+ return new SolrConstantScoreQuery.ConstantWeight(searcher, scoreMode, boost) {
Review comment:
Can we please not use SolrConstantScoreQuery? Try ConstantScoreWeight in Lucene
##########
File path: solr/core/src/java/org/apache/solr/search/FunctionRangeQuery.java
##########
@@ -39,10 +40,20 @@ public FunctionRangeQuery(ValueSourceRangeFilter filter) {
this.cost = 100; // default behavior should be PostFiltering
}
+ @Override
Review comment:
I'm not sure why changes are here but I suspect it's related to the increased scope. Maybe this can be punted for another issue.
##########
File path: solr/contrib/analytics/src/java/org/apache/solr/analytics/facet/AbstractSolrQueryFacet.java
##########
@@ -50,11 +49,11 @@ protected AbstractSolrQueryFacet(String name) {
*
* Each of these executors will be executed after the streaming phase in the {@link AnalyticsDriver}.
*
- * @param filter the overall filter representing the documents being used for the analytics request
Review comment:
I think the name "filter" is reasonable as it conveys it's purpose is only for filtering and not also for scoring. If possible, we could use DocSet?
##########
File path: solr/core/src/java/org/apache/solr/search/BitDocSet.java
##########
@@ -311,72 +307,8 @@ public long cost() {
};
}
- @Override
- public Filter getTopFilter() {
- // TODO: if cardinality isn't cached, do a quick measure of sparseness
- // and return null from bits() if too sparse.
-
- return new Filter() {
- final FixedBitSet bs = bits;
-
- @Override
- public DocIdSet getDocIdSet(final LeafReaderContext context, final Bits acceptDocs) {
- // all Solr DocSets that are used as filters only include live docs
- final Bits acceptDocs2 = acceptDocs == null ? null : (context.reader().getLiveDocs() == acceptDocs ? null : acceptDocs);
-
- return BitsFilteredDocIdSet.wrap(new DocIdSet() {
- @Override
- public DocIdSetIterator iterator() {
- return BitDocSet.this.iterator(context);
- }
-
- @Override
- public long ramBytesUsed() {
- return BitDocSet.this.ramBytesUsed();
- }
-
- @Override
- public Bits bits() {
- if (context.isTopLevel) {
- return bits;
- }
-
- final int base = context.docBase;
- final int length = context.reader().maxDoc();
- final FixedBitSet bs = bits;
-
- return new Bits() {
- @Override
- public boolean get(int index) {
- return bs.get(index + base);
- }
-
- @Override
- public int length() {
- return length;
- }
- };
- }
-
- }, acceptDocs2);
- }
-
- @Override
- public String toString(String field) {
- return "BitSetDocTopFilter";
- }
-
- @Override
- public boolean equals(Object other) {
- return sameClassAs(other) &&
- Objects.equals(bs, getClass().cast(other).bs);
- }
-
- @Override
- public int hashCode() {
- return classHash() * 31 + bs.hashCode();
- }
- };
+ public DocSetQuery makeQuery() {
+ return new DocSetQuery(this);
Review comment:
All implementations of bits() return null these days. It's not obvious; you need to find-usages a couple layers and look carefully.
##########
File path: solr/contrib/analytics/src/java/org/apache/solr/analytics/facet/QueryFacet.java
##########
@@ -40,7 +39,7 @@ public QueryFacet(String name, Map<String, String> queries) {
}
@Override
- public void createFacetValueExecuters(final Filter filter, SolrQueryRequest queryRequest, Consumer<FacetValueQueryExecuter> consumer) {
+ public void createFacetValueExecuters(final Query qry, SolrQueryRequest queryRequest, Consumer<FacetValueQueryExecuter> consumer) {
Review comment:
again, "filter" is a reasonable name
##########
File path: solr/contrib/analytics/src/java/org/apache/solr/analytics/AnalyticsDriver.java
##########
@@ -37,28 +37,28 @@
*
* @param manager of the request to drive
* @param searcher the results of the query
- * @param filter that represents the overall query
+ * @param query that represents the overall query
* @param queryRequest used for the search request
* @throws IOException if an error occurs while reading from Solr
*/
- public static void drive(AnalyticsRequestManager manager, SolrIndexSearcher searcher, Filter filter, SolrQueryRequest queryRequest) throws IOException {
+ public static void drive(AnalyticsRequestManager manager, SolrIndexSearcher searcher, Query query, SolrQueryRequest queryRequest) throws IOException {
Review comment:
It appears the caller should pass a DocSet (and leave named "filter")? When calling getFacetExecutors, you can then call filter.makeQuery(). A DocIdSetIterator can be retrieved from the DocSet.
##########
File path: solr/core/src/test/org/apache/solr/search/TestFilteredDocIdSet.java
##########
@@ -127,22 +130,49 @@ public void testNullDocIdSet() throws Exception {
IndexSearcher searcher = newSearcher(reader);
Assert.assertEquals(1, searcher.search(new MatchAllDocsQuery(), 10).totalHits.value);
- // Now search w/ a Filter which returns a null DocIdSet
- Filter f = new Filter() {
+ // Now search w/ a Query which returns a null DocIdSet
+ Query f = new Query() {
Review comment:
This test should be testing DocSetQuery. If it's a null set of IDs, then the underlying DocIdSetIterator will be null.
##########
File path: solr/contrib/analytics/src/java/org/apache/solr/analytics/facet/RangeFacet.java
##########
@@ -54,7 +53,7 @@ public RangeFacet(String name, SchemaField field, String start, String end, List
}
@Override
- public void createFacetValueExecuters(final Filter filter, SolrQueryRequest queryRequest, Consumer<FacetValueQueryExecuter> consumer) {
+ public void createFacetValueExecuters(final Query query, SolrQueryRequest queryRequest, Consumer<FacetValueQueryExecuter> consumer) {
Review comment:
"filter" is a reasonable name
##########
File path: solr/core/src/test/org/apache/solr/search/TestSort.java
##########
@@ -236,16 +240,43 @@ public void testSort() throws Exception {
assertTrue(reader.leaves().size() > 1);
for (int i=0; i<qiter; i++) {
- Filter filt = new Filter() {
Review comment:
appears out of scope of this JIRA
##########
File path: solr/core/src/java/org/apache/solr/search/SolrConstantScoreQuery.java
##########
@@ -41,17 +41,20 @@
* Experimental and subject to change.
*/
public class SolrConstantScoreQuery extends Query implements ExtendedQuery {
- private final Filter filter;
Review comment:
Out of scope of this issue, but SolrConstantScoreQuery can go away. It's fundamental job is to make a Filter look like a Query way back when Filter wasn't a Query. For many years, Filter has been one. It also does what SolrExtendedQuery (or whatever it's called) does.
##########
File path: solr/core/src/java/org/apache/solr/search/function/ValueSourceRangeFilter.java
##########
@@ -35,7 +37,7 @@
/**
* RangeFilter over a ValueSource.
*/
-public class ValueSourceRangeFilter extends SolrFilter {
Review comment:
This in not in scope of this JIRA issue
##########
File path: solr/core/src/java/org/apache/solr/search/BitsFilteredDocIdSet.java
##########
@@ -23,15 +23,11 @@
/**
* This implementation supplies a filtered DocIdSet, that excludes all
- * docids which are not in a Bits instance. This is especially useful in
- * {@link org.apache.solr.search.Filter} to apply the {@code acceptDocs}
- * passed to {@code getDocIdSet()} before returning the final DocIdSet.
+ * docids which are not in a Bits instance.
*
* @see DocIdSet
- * @see org.apache.solr.search.Filter
*/
public final class BitsFilteredDocIdSet extends FilteredDocIdSet {
Review comment:
Nice catch Mike!
--
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