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