dsmiley commented on a change in pull request #1381: SOLR-14364: LTR 
SolrFeature fq improvements
URL: https://github.com/apache/lucene-solr/pull/1381#discussion_r401318959
 
 

 ##########
 File path: 
solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java
 ##########
 @@ -120,67 +123,66 @@ protected void validate() throws FeatureException {
           ": Q or FQ must be provided");
     }
   }
+
   /**
    * Weight for a SolrFeature
    **/
   public class SolrFeatureWeight extends FeatureWeight {
-    final private Weight solrQueryWeight;
-    final private Query query;
-    final private List<Query> queryAndFilters;
+    private final Weight solrQueryWeight;
 
-    public SolrFeatureWeight(IndexSearcher searcher,
-        SolrQueryRequest request, Query originalQuery, Map<String,String[]> 
efi) throws IOException {
+    public SolrFeatureWeight(SolrIndexSearcher searcher,
+                             SolrQueryRequest request, Query originalQuery, 
Map<String, String[]> efi) throws IOException {
       super(SolrFeature.this, searcher, request, originalQuery, efi);
       try {
-        String solrQuery = q;
-        final List<String> fqs = fq;
-
-        if ((solrQuery == null) || solrQuery.isEmpty()) {
-          solrQuery = "*:*";
-        }
-
-        solrQuery = macroExpander.expand(solrQuery);
-        if (solrQuery == null) {
-          throw new FeatureException(this.getClass().getSimpleName()+" 
requires efi parameter that was not passed in request.");
-        }
-
-        final SolrQueryRequest req = makeRequest(request.getCore(), solrQuery,
-            fqs, df);
+        final SolrQueryRequest req = makeRequest(request.getCore(), q, fq, df);
         if (req == null) {
           throw new IOException("ERROR: No parameters provided");
         }
 
+        // Build the scoring query
+        Query scoreQuery;
+        String qStr = q;
+        if (qStr == null || qStr.isEmpty()) {
+          scoreQuery = null; // ultimately behaves like MatchAllDocs
+        } else {
+          qStr = macroExpander.expand(qStr);
+          if (qStr == null) {
+            throw new FeatureException(this.getClass().getSimpleName() + " 
requires efi parameter that was not passed in request.");
+          }
+          scoreQuery = QParser.getParser(qStr, req).getQuery();
+          // note: QParser can return a null Query sometimes, such as if the 
query is a stopword or just symbols
+          if (scoreQuery == null) {
+            scoreQuery = new MatchNoDocsQuery(); // debatable; all or none?
+          }
+        }
+
         // Build the filter queries
-        queryAndFilters = new ArrayList<Query>(); // If there are no fqs we 
just want an empty list
-        if (fqs != null) {
-          for (String fq : fqs) {
-            if ((fq != null) && (fq.trim().length() != 0)) {
-              fq = macroExpander.expand(fq);
-              if (fq == null) {
-                throw new FeatureException(this.getClass().getSimpleName()+" 
requires efi parameter that was not passed in request.");
+        Query filterDocSetQuery = null;
+        if (fq != null) {
+          List<Query> filterQueries = new ArrayList<>(); // If there are no 
fqs we just want an empty list
+          for (String fqStr : fq) {
+            if (fqStr != null) {
+              fqStr = macroExpander.expand(fqStr);
+              if (fqStr == null) {
+                throw new FeatureException(this.getClass().getSimpleName() + " 
requires efi parameter that was not passed in request.");
               }
-              final QParser fqp = QParser.getParser(fq, req);
-              final Query filterQuery = fqp.getQuery();
+              final Query filterQuery = QParser.getParser(fqStr, 
req).getQuery();
               if (filterQuery != null) {
-                queryAndFilters.add(filterQuery);
+                filterQueries.add(filterQuery);
               }
             }
           }
+
+          DocSet filtersDocSet = searcher.getDocSet(filterQueries); // execute
 
 Review comment:
   Good eye.  It's possible filterQueries might be empty if say someone added a 
blank fq.  And I verified in a debugger get getDocSet does the right thing.  
It's taking a long time to do its job however (accumulating a complete docset, 
bit by bit).  In another issue I'm targeting at 9.0, I optimized this case to a 
live-docs DocSet, among other things relating to TwoPhaseIterator.  I should 
pull that out and make it its own improvement, along with a bit more 
documentation to getDocSet to indicate that's what the semantics are if an 
empty list is passed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to