HighlighterTest failure

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

HighlighterTest failure

Erik Hatcher
I get a failure running HighlighterTest from the Subversion trunk.  
Below are the details.

What's the fix?

Thanks,
        Erik


     [junit] Searching for: multi*
     [junit] ------------- ---------------- ---------------
     [junit] Testcase:  
testMultiSearcher(org.apache.lucene.search.highlight.HighlighterTest):  
   Caused an ERROR
     [junit] null
     [junit] java.lang.IllegalArgumentException
     [junit]     at  
org.apache.lucene.search.Query.combine(Query.java:113)
     [junit]     at  
org.apache.lucene.search.MultiSearcher.rewrite(MultiSearcher.java:298)
     [junit]     at org.apache.lucene.search.Query.weight(Query.java:92)
     [junit]     at org.apache.lucene.search.Hits.<init>(Hits.java:40)
     [junit]     at  
org.apache.lucene.search.Searcher.search(Searcher.java:40)
     [junit]     at  
org.apache.lucene.search.Searcher.search(Searcher.java:32)
     [junit]     at  
org.apache.lucene.search.highlight.HighlighterTest.testMultiSearcher(Hig
hlighterTest.java:436)
     [junit]     at sun.reflect.NativeMethodAccessorImpl.invoke0(Native  
Method)
     [junit]     at  
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.jav
a:39)
     [junit]     at  
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessor
Impl.java:25)


     [junit] Test org.apache.lucene.search.highlight.HighlighterTest  
FAILED

Which comes from this code in HighlighterTest:

                IndexSearcher searchers[]=new IndexSearcher[2];
                searchers[0] = new IndexSearcher(ramDir1);
                searchers[1] = new IndexSearcher(ramDir2);
                MultiSearcher multiSearcher=new MultiSearcher(searchers);
                query = QueryParser.parse("multi*", FIELD_NAME, new  
StandardAnalyzer());
                System.out.println("Searching for: " + query.toString(FIELD_NAME));
                //at this point the multisearcher calls combine(query[])
                hits = multiSearcher.search(query);


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: HighlighterTest failure

Chuck Williams
Erik Hatcher wrote:

> I get a failure running HighlighterTest from the Subversion trunk.
> Below are the details.
>
> What's the fix?

I don't have the code here to run, but the problem is that
MultiSearcher.rewrite(line 298) is calling Query.combine, which requires
all the combined queries to be equal. Obviously they will not be equal
in general for multi-term queries (PrefixQuery's in this case) since
those queries expand into different terms for the different searchers.
Just above the failing line in MultiSearcher.rewrite() it rewrites the
query it was passed from the parser in the HighlighterTest. This should
be a PrefixQuery, which should rewrite into two BooleanQuery's, which
should then lead to BooleanQuery.combine() and not Query.combine(). So
it appears that either the query parser did not produce a PrefixQuery,
or you don't have the current code?

Could you step through it and find out what kind of query is passes to
MultiSearcher.rewrite() and what comes back from the 2 rewrite() calls
made there for the two searchers?

Chuck


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: HighlighterTest failure

Erik Hatcher

On Apr 25, 2005, at 10:02 PM, Chuck Williams wrote:

> Erik Hatcher wrote:
>
>> I get a failure running HighlighterTest from the Subversion trunk.
>> Below are the details.
>>
>> What's the fix?
>
> I don't have the code here to run, but the problem is that
> MultiSearcher.rewrite(line 298) is calling Query.combine, which
> requires all the combined queries to be equal. Obviously they will not
> be equal in general for multi-term queries (PrefixQuery's in this
> case) since those queries expand into different terms for the
> different searchers. Just above the failing line in
> MultiSearcher.rewrite() it rewrites the query it was passed from the
> parser in the HighlighterTest. This should be a PrefixQuery, which
> should rewrite into two BooleanQuery's, which should then lead to
> BooleanQuery.combine() and not Query.combine(). So it appears that
> either the query parser did not produce a PrefixQuery, or you don't
> have the current code?
>
> Could you step through it and find out what kind of query is passes to
> MultiSearcher.rewrite() and what comes back from the 2 rewrite() calls
> made there for the two searchers?

PrefixQuery being passed to MultiSearcher.rewrite()... and the 2
rewrites each rewrite to the expanded Query (an optimized TermQuery in
this case).

I've got the current trunk code.

*shrugs*

        Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: HighlighterTest failure

Chuck Williams
Erik Hatcher wrote:

>
> On Apr 25, 2005, at 10:02 PM, Chuck Williams wrote:
>
>> Erik Hatcher wrote:
>>
>>> I get a failure running HighlighterTest from the Subversion trunk.
>>> Below are the details.
>>>
>>> What's the fix?
>>
>>
>> I don't have the code here to run, but the problem is that
>> MultiSearcher.rewrite(line 298) is calling Query.combine, which
>> requires all the combined queries to be equal. Obviously they will
>> not be equal in general for multi-term queries (PrefixQuery's in this
>> case) since those queries expand into different terms for the
>> different searchers. Just above the failing line in
>> MultiSearcher.rewrite() it rewrites the query it was passed from the
>> parser in the HighlighterTest. This should be a PrefixQuery, which
>> should rewrite into two BooleanQuery's, which should then lead to
>> BooleanQuery.combine() and not Query.combine(). So it appears that
>> either the query parser did not produce a PrefixQuery, or you don't
>> have the current code?
>>
>> Could you step through it and find out what kind of query is passes
>> to MultiSearcher.rewrite() and what comes back from the 2 rewrite()
>> calls made there for the two searchers?
>
>
> PrefixQuery being passed to MultiSearcher.rewrite()... and the 2
> rewrites each rewrite to the expanded Query (an optimized TermQuery in
> this case).
>
> I've got the current trunk code.
>
> *shrugs*

Erik, this patch will fix the problem. It passes all Lucene and
Highlighter tests, except for Otis's TestSort issue, which as I noted
earlier is a bug in the test. (It is hard for me to setup contrib
modules in my ide because it doesn't like the fact that they are
subfolders of trunk with independent build files so I have not tested
the other contribs). Change Query.combine() to this:

/** Expert: called when re-writing queries under MultiSearcher.
*
* <p>Only implemented by derived queries, with no
* {@link #createWeight(Searcher)} implementatation.
*/
public Query combine(Query[] queries) {
for (int i = 0; i < queries.length; i++) {
if (!this.equals(queries[i])) {
BooleanQuery result = new BooleanQuery(true);
for (int j = 0; j < queries.length; j++)
result.add(queries[j], BooleanClause.Occur.SHOULD);
return result;
}
}
return this;
}

Analysis: the root problem is that the current Query.combine(), which is
only used by MultiSearcher, requires that all queries rewrite to either
a) the same thing, or b) a combinable container type such as
BooleanQuery. This is incompatible with the single-clause optimization
that occurs in BooleanQuery.rewrite(). The fix is to have
Query.combine() combine distinct queries into an OR rather to reject
them. This is a reversal of the earlier optimizations in the rewrites
that is required to make sure the same query is distributed to each
sub-searcher -- this is exaclty what BooleanQuery.combine() and the
other custom combine() methods do. I would have liked to call rewrite()
on the result of the above inner loop, but unfortunately there is no
longer a pointer to the reader available. This deficiency could be
changed by modifying the API and updating all of the overriding methods
in the container types. There may be a better fix than the above, but I
think this is correct and it passes all the tests.

Doug and Wolf should review this.

Chuck


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]