lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chuck Williams <ch...@allthingslocal.com>
Subject Re: HighlighterTest failure
Date Tue, 26 Apr 2005 04:59:53 GMT
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


Mime
View raw message