dsmiley commented on a change in pull request #529: URL: https://github.com/apache/solr/pull/529#discussion_r789901078
########## File path: solr/core/src/java/org/apache/solr/search/DocSet.java ########## @@ -117,11 +118,11 @@ public int andNotSize(DocSet other) { } /** - * Returns a Filter for use in Lucene search methods, assuming this DocSet + * Returns a Query for use in Lucene search methods, assuming this DocSet Review comment: Say it runs a constant scoring Query in particular. And I'd further remind the reader that DocSets and thus this query do not match deleted docs. ########## File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java ########## @@ -0,0 +1,86 @@ +/* + * 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.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 implements DocSetProducer{ + DocSet docSet; + + public DocSetQuery(DocSet docSet) { + super(); + this.docSet = docSet; + } + + public DocSetQuery() { + } + + @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 DocSet createDocSet(SolrIndexSearcher searcher) throws IOException { + return null; + } + + @Override + public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { + return new ConstantScoreWeight(this, boost) { + @Override + public Scorer scorer(LeafReaderContext context) throws IOException { + DocIdSetIterator disi = docSet.iterator(context); + if (disi == null) { + return null; + } + return new ConstantScoreScorer(this, score(), scoreMode, disi); + } + + @Override + public boolean isCacheable(LeafReaderContext ctx) { + return false; Review comment: This is Cacheable in fact. This is supposed to be weights on DocValues fields that might have had realtime updates. ########## File path: solr/contrib/analytics/src/java/org/apache/solr/analytics/AnalyticsGroupingManager.java ########## @@ -98,16 +98,15 @@ public boolean getStreamingFacets(Consumer<StreamingFacet> cons) { * One {@link FacetValueQueryExecuter} is created for each facet value to be returned for a facet. * Since every {@link AbstractSolrQueryFacet} has discrete and user-defined facet values, * unlike {@link StreamingFacet}s, a discrete number of {@link FacetValueQueryExecuter}s are created and returned. - * - * @param filter representing the overall Solr Query of the request, + * @param query representing the overall Solr Query of the request, * will be combined with the facet value queries * @param queryRequest from the overall search request * @param cons where the executers are passed to */ - public void getFacetExecuters(Filter filter, SolrQueryRequest queryRequest, Consumer<FacetValueQueryExecuter> cons) { + public void getFacetExecuters(Query query, SolrQueryRequest queryRequest, Consumer<FacetValueQueryExecuter> cons) { Review comment: again; keep named "filter" ########## File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java ########## @@ -0,0 +1,86 @@ +/* + * 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.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 implements DocSetProducer{ + DocSet docSet; + + public DocSetQuery(DocSet docSet) { + super(); + this.docSet = docSet; + } + + public DocSetQuery() { + } + + @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 DocSet createDocSet(SolrIndexSearcher searcher) throws IOException { + return null; Review comment: uh? I suspect this is your IDE default, which IMO is a bad default. FWIW I changed my IDE default here to be a //nocommit. ########## File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java ########## @@ -0,0 +1,86 @@ +/* + * 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.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 implements DocSetProducer{ + DocSet docSet; + + public DocSetQuery(DocSet docSet) { + super(); + this.docSet = docSet; + } + + public DocSetQuery() { + } + + @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 DocSet createDocSet(SolrIndexSearcher searcher) throws IOException { + return null; + } + + @Override + public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { + return new ConstantScoreWeight(this, boost) { Review comment: The scores of a Filter used to be 0, so in this PR lets just use 0. In a separate PR for JIRA SOLR-14800 I'll change it ########## File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java ########## @@ -0,0 +1,86 @@ +/* + * 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.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 implements DocSetProducer{ Review comment: Why does this extend SolrConstantScoreQuery? ########## File path: solr/core/src/test/org/apache/solr/search/TestFilteredDocIdSet.java ########## @@ -127,22 +130,42 @@ 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 Review comment: Query doesn't return DocIdSet. You changed it to return a null Scorer which is close enough to the original intent. ########## File path: solr/core/src/test/org/apache/solr/search/TestFilteredDocIdSet.java ########## @@ -113,7 +116,7 @@ protected boolean match(int docid) { } public void testNullDocIdSet() throws Exception { - // Tests that if a Filter produces a null DocIdSet, which is given to + // Tests that if a Query produces a null DocIdSet, which is given to Review comment: Filters produces a DocIdSet; Query does not. You could try prefixing the comment with "(historical note)" or something ########## File path: solr/core/src/test/org/apache/solr/search/TestFilteredDocIdSet.java ########## @@ -171,25 +194,25 @@ public void testNullIteratorFilteredDocIdSet() 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 Review comment: no; a null *Scorer* ########## File path: solr/core/src/test/org/apache/solr/search/TestDocSet.java ########## @@ -556,41 +558,55 @@ public void doFilterTest(IndexReader reader) throws IOException { doTestIteratorEqual(da, db); ***/ - DocIdSet da; - DocIdSet db; List<LeafReaderContext> leaves = topLevelContext.leaves(); - // first test in-sequence sub readers for (LeafReaderContext readerContext : leaves) { - da = fa.getDocIdSet(readerContext, null); - db = fb.getDocIdSet(readerContext, null); - // there are various ways that disis can be retrieved for each leafReader; they should all be equivalent. - doTestIteratorEqual(da.bits(), disiSupplier(da), disiSupplier(db), () -> a.iterator(readerContext), () -> b.iterator(readerContext)); - + doTestIteratorEqual(getExpectedBits(a, readerContext), () -> a.iterator(readerContext), () -> b.iterator(readerContext)); // set b is SortedIntDocSet, so derivatives should not support random-access via Bits Review comment: this comment is now obsolete once you removed the line that followed ########## 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 will certainly change *then*. This issue is about DocSet.getTopFilter. VSRF doesn't use that nor the reverse. ########## File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java ########## @@ -0,0 +1,86 @@ +/* + * 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.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 implements DocSetProducer{ + DocSet docSet; Review comment: private final ########## File path: solr/core/src/java/org/apache/solr/search/DocSetQuery.java ########## @@ -0,0 +1,86 @@ +/* + * 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.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 implements DocSetProducer{ Review comment: Add javadocs! Refer to who created this. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org