jtibshirani commented on a change in pull request #160:
URL: https://github.com/apache/lucene/pull/160#discussion_r644158197



##########
File path: lucene/core/src/test/org/apache/lucene/search/TestSynonymQuery.java
##########
@@ -466,4 +467,26 @@ public void testRandomTopDocs() throws IOException {
     reader.close();
     dir.close();
   }
+
+  public void testRewrite() throws IOException {
+    // zero length SynonymQuery is rewritten
+    SynonymQuery q = new SynonymQuery.Builder("f").build();

Review comment:
       A small comment: in most other rewrite tests (like 
`TestBoostQuery#testRewrite`) we check the higher-level call 
`IndexSearcher#rewrite`. It'd be nice to do this to be consistent and to 
exercise the full rewrite logic.

##########
File path: 
lucene/sandbox/src/java/org/apache/lucene/sandbox/search/CombinedFieldQuery.java
##########
@@ -237,14 +236,6 @@ public Query rewrite(IndexReader reader) throws 
IOException {
     if (fieldTerms.length == 1) {

Review comment:
       I like the simplification below of removing the rewrite to synonym query 
(which is not perfectly accurate). I think we also need to fix or remove this 
check `if (fieldTerms.length == 1) { ... }`, since it's only accurate when the 
field weight is 1.0f.




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



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

Reply via email to