[jira] Issue Comment Edited: (LUCENE-2694) MTQ rewrite + weight/scorer init should be single pass

2010-12-18 Thread Uwe Schindler (JIRA) 

[ 
https://issues.apache.org/jira/browse/LUCENE-2694?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12972769#action_12972769
 ] 

Uwe Schindler edited comment on LUCENE-2694 at 12/18/10 5:43 AM:
-

I have also some things:
- We currently don't support seeking a FilteredTermsEnum, this is disallowed by 
UnsupportedOperationException (we may change this, but its complicated, Robert 
and me are thinking about it, but for now its disallowed, as it would break the 
enum logic). So the TermState seek method in FilteredTermsEnum should also 
throw UOE:
{code}
/** This enum does not support seeking!
 * @throws UnsupportedOperationException
 */
@Override
public SeekStatus seek(BytesRef term, boolean useCache) throws IOException {
  throw new UnsupportedOperationException(getClass().getName()+" does not 
support seeking");
}
{code}
- Additionally, can the next() implementation in FilteredTermsEnum use 
TermState? It does lots of seeking on the underlying (filtered) TermsEnum. This 
is the reason why sekking on the FilteredTermsEnum is not allowed. Filtering is 
done here on the accept() methods.
- For what is setNextReader in TermCollector? I don't like that, but you seems 
to need it for the PerReaderTermState. The collector should really only work on 
the enum not on any reader. At least the

Thats what I have seen on first patch review, will now apply patch and look 
closer into it :-) But the first point is important, FilteredTermsEnum 
currently should not support seeking.

  was (Author: thetaphi):
I have also some things:
- We currently don't support seeking a FilteredTermsEnum, this is disallowed by 
UnsupportedOperationException (we may change this, but its complicated, Robert 
and me are thinking about it, but for now its disallowed, as it would break the 
enum logic). So the TermState seek method in FilteredTermsEnum should also 
throw UOE:
{code}
/** This enum does not support seeking!
 * @throws UnsupportedOperationException
 */
@Override
public SeekStatus seek(BytesRef term, boolean useCache) throws IOException {
  throw new UnsupportedOperationException(getClass().getName()+" does not 
support seeking");
}
{code}
- For what is setNextReader in TermCollector? I don't like that, but you seems 
to need it for the PerReaderTermState. The collector should really only work on 
the enum not on any reader.

Thats what I have seen on first patch review, will now apply patch and look 
closer into it :-) But the first point is important, FilteredTermsEnum 
currently should not support seeking.
  
> MTQ rewrite + weight/scorer init should be single pass
> --
>
> Key: LUCENE-2694
> URL: https://issues.apache.org/jira/browse/LUCENE-2694
> Project: Lucene - Java
>  Issue Type: Improvement
>  Components: Search
>Reporter: Michael McCandless
>Assignee: Michael McCandless
> Fix For: 4.0
>
> Attachments: LUCENE-2694.patch, LUCENE-2694.patch, LUCENE-2694.patch, 
> LUCENE-2694.patch
>
>
> Spinoff of LUCENE-2690 (see the hacked patch on that issue)...
> Once we fix MTQ rewrite to be per-segment, we should take it further and make 
> weight/scorer init also run in the same single pass as rewrite.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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

[jira] Issue Comment Edited: (LUCENE-2694) MTQ rewrite + weight/scorer init should be single pass

2010-11-18 Thread Uwe Schindler (JIRA) 

[ 
https://issues.apache.org/jira/browse/LUCENE-2694?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12933505#action_12933505
 ] 

Uwe Schindler edited comment on LUCENE-2694 at 11/18/10 1:10 PM:
-

Havent looked closely into the patch (still need to understand the whole 
thing), just some comments from attribute policeman in general:
- The TermStateAttributeImpl.copyTo should throw ClassCastEx if attributes are 
not conform (compare other impls), so the if statement should not be there. 
AttributeSource takes care of copying. This is not used, but for completeness.
- the convenience addClause() method in abstract base class should be final! 
Else you could incorrectly override the wrong one. We already have code 
duplication in your patch because of this. When you make it final you will see!
- why is the attribute using a SetOnce? Attributes generally should be 
modifiable multiple times. Now you have to call clear() first. This may change 
in future when we have set-once attributes, but for now that violates the 
contract :-)
- Is the docFreq no already part of the state so TermCollectingRewrite does not 
need to expose it separately?
- What happens in the term collectors when the same term with different states 
are merged in the PQ/TermsHash/...?

  was (Author: thetaphi):
Havent looked closely into the patch (still need to understand the whole 
thing), just some comments from attribute policeman in general:
- The TermStateAttributeImpl.copyTo should throw ClassCastEx if attributes are 
not conform (compare other impls), so the if statement should not be there. 
AttributeSource takes care of copying. This is not used, ut for completeness.
- the convenience addClause() method in abstract base class should be final! 
Else you could incorrectly override the wrong one.
- why is the attribute using a SetOnce? Attributes generally should be 
modifiable multiple times. Now you have to call clear() first. This may change 
in future when we have set-once attributes, but for now that violates the 
contract :-)
- Is the docFreq no already part of the state so TermCollectingRewrite does not 
need to expose it separately?
- What happens in the term collectors when the same term with different states 
are merged in the PQ/TermsHash/...?
  
> MTQ rewrite + weight/scorer init should be single pass
> --
>
> Key: LUCENE-2694
> URL: https://issues.apache.org/jira/browse/LUCENE-2694
> Project: Lucene - Java
>  Issue Type: Improvement
>  Components: Search
>Reporter: Michael McCandless
>Assignee: Michael McCandless
> Fix For: 4.0
>
> Attachments: LUCENE-2694.patch
>
>
> Spinoff of LUCENE-2690 (see the hacked patch on that issue)...
> Once we fix MTQ rewrite to be per-segment, we should take it further and make 
> weight/scorer init also run in the same single pass as rewrite.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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