You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "alessandrobenedetti (via GitHub)" <gi...@apache.org> on 2023/02/21 15:05:48 UTC

[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1260: SOLR-788: transfer mlt query for component right

alessandrobenedetti commented on code in PR #1260:
URL: https://github.com/apache/solr/pull/1260#discussion_r1112953125


##########
solr/core/src/java/org/apache/solr/handler/component/MoreLikeThisComponent.java:
##########
@@ -95,23 +98,23 @@ public void process(ResponseBuilder rb) throws IOException {
             rb.rsp.add("moreLikeThis", new NamedList<DocList>());
             return;
           }
-
           MoreLikeThisHandler.MoreLikeThisHelper mlt =
               new MoreLikeThisHandler.MoreLikeThisHelper(params, searcher);
-
-          NamedList<BooleanQuery> bQuery = mlt.getMoreLikeTheseQuery(rb.getResults().docList);
-
-          NamedList<String> temp = new NamedList<>();
-          Iterator<Entry<String, BooleanQuery>> idToQueryIt = bQuery.iterator();
-
-          while (idToQueryIt.hasNext()) {
-            Entry<String, BooleanQuery> idToQuery = idToQueryIt.next();
-            String s = idToQuery.getValue().toString();
-
-            log.debug("MLT Query:{}", s);
-            temp.add(idToQuery.getKey(), idToQuery.getValue().toString());
+          NamedList<NamedList<?>> temp = new NamedList<>();

Review Comment:
   we can leverage this contribution for some minor refactoring?
   rename 'temp' to 'docIdsAndMltQueries' , or something like that?



##########
solr/core/src/java/org/apache/solr/handler/component/MoreLikeThisComponent.java:
##########
@@ -95,23 +98,23 @@ public void process(ResponseBuilder rb) throws IOException {
             rb.rsp.add("moreLikeThis", new NamedList<DocList>());
             return;
           }
-
           MoreLikeThisHandler.MoreLikeThisHelper mlt =
               new MoreLikeThisHandler.MoreLikeThisHelper(params, searcher);
-
-          NamedList<BooleanQuery> bQuery = mlt.getMoreLikeTheseQuery(rb.getResults().docList);
-
-          NamedList<String> temp = new NamedList<>();
-          Iterator<Entry<String, BooleanQuery>> idToQueryIt = bQuery.iterator();
-
-          while (idToQueryIt.hasNext()) {
-            Entry<String, BooleanQuery> idToQuery = idToQueryIt.next();
-            String s = idToQuery.getValue().toString();
-
-            log.debug("MLT Query:{}", s);
-            temp.add(idToQuery.getKey(), idToQuery.getValue().toString());
+          NamedList<NamedList<?>> temp = new NamedList<>();
+          for (DocIterator iterator = rb.getResults().docList.iterator(); iterator.hasNext(); ) {

Review Comment:
   DocIterator searchResults ?
   DocIterator results ?
   
   never been a fan of names that are basically the type of the object 



##########
solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java:
##########
@@ -466,37 +456,19 @@ public DocListAndSet getMoreLikeThis(
       }
       return results;
     }
-
-    public NamedList<BooleanQuery> getMoreLikeTheseQuery(DocList docs) throws IOException {
-      IndexSchema schema = searcher.getSchema();
-      NamedList<BooleanQuery> result = new NamedList<>();
-      DocIterator iterator = docs.iterator();
-      while (iterator.hasNext()) {
-        int id = iterator.nextDoc();
-        String uniqueId = schema.printableUniqueKey(reader.document(id));
-
-        BooleanQuery mltquery = (BooleanQuery) mlt.like(id);
-        if (mltquery.clauses().size() == 0) {
-          return result;
-        }
-        mltquery = (BooleanQuery) getBoostedQuery(mltquery);
-
-        // exclude current document from results
-        BooleanQuery.Builder mltQuery = new BooleanQuery.Builder();
-        mltQuery.add(mltquery, BooleanClause.Occur.MUST);
-
-        mltQuery.add(
-            new TermQuery(new Term(uniqueKeyField.getName(), uniqueId)),
-            BooleanClause.Occur.MUST_NOT);
-        result.add(uniqueId, mltQuery.build());
-      }
-
-      return result;
-    }
-
-    private void fillInterestingTermsFromMLTQuery(Query query, List<InterestingTerm> terms) {
-      Collection<BooleanClause> clauses = ((BooleanQuery) query).clauses();
+    /**
+     * Yields terms with boosts from the boosted MLT query.
+     *
+     * @param maxTerms how many terms to return, negative value let to return all

Review Comment:
   maybe:
   @param maxTerms how many terms to return, a negative value means all terms are returned
   ?



##########
solr/core/src/java/org/apache/solr/handler/component/MoreLikeThisComponent.java:
##########
@@ -95,23 +98,23 @@ public void process(ResponseBuilder rb) throws IOException {
             rb.rsp.add("moreLikeThis", new NamedList<DocList>());
             return;
           }
-
           MoreLikeThisHandler.MoreLikeThisHelper mlt =
               new MoreLikeThisHandler.MoreLikeThisHelper(params, searcher);
-
-          NamedList<BooleanQuery> bQuery = mlt.getMoreLikeTheseQuery(rb.getResults().docList);
-
-          NamedList<String> temp = new NamedList<>();
-          Iterator<Entry<String, BooleanQuery>> idToQueryIt = bQuery.iterator();
-
-          while (idToQueryIt.hasNext()) {
-            Entry<String, BooleanQuery> idToQuery = idToQueryIt.next();
-            String s = idToQuery.getValue().toString();
-
-            log.debug("MLT Query:{}", s);
-            temp.add(idToQuery.getKey(), idToQuery.getValue().toString());
+          NamedList<NamedList<?>> temp = new NamedList<>();
+          for (DocIterator iterator = rb.getResults().docList.iterator(); iterator.hasNext(); ) {
+            int id = iterator.nextDoc();

Review Comment:
   docId maybe?



##########
solr/core/src/test/org/apache/solr/handler/component/DistributedMLTComponentTest.java:
##########
@@ -389,5 +389,29 @@ public void test() throws Exception {
       Long actual = ((SolrDocumentList) entry.getValue()).getNumFound();
       assertEquals("MLT mismatch for id=" + key, expected, actual);
     }
+    // test boost mlt.qf
+    query(
+        "q",
+        "lowerfilt:moon",
+        "fl",
+        id,
+        MoreLikeThisParams.MIN_TERM_FREQ,
+        2,
+        MoreLikeThisParams.MIN_DOC_FREQ,
+        1,
+        "sort",
+        "id_i1 desc",
+        "mlt",
+        "true",
+        "mlt.fl",
+        "lowerfilt1,lowerfilt",
+        "mlt.qf",
+        "lowerfilt1^1.2 lowerfilt^3.4",
+        "qt",
+        requestHandlerName,
+        "shards.qt",
+        requestHandlerName,
+        "mlt.count",
+        "20");

Review Comment:
   I was trying to understand why we have the 'query()' here, with no assertions. I can spend more time refreshing on those tests, but given you have been working recently on these, can you refresh my mind?



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