lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (JIRA)" <j...@apache.org>
Subject [jira] Issue Comment Edited: (LUCENE-2784) Change all FilteredTermsEnum impls into TermsEnum decorators
Date Mon, 29 Nov 2010 18:35:18 GMT

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

Uwe Schindler edited comment on LUCENE-2784 at 11/29/10 1:34 PM:
-----------------------------------------------------------------

That patch looks very good!

{noformat}
+      if (term == null) // TODO: is this really necessary? we should be positioned already?
{noformat}

The check is really necessary, as on the first call of nextSeekTerm on the uninitialized TermsEnum
(before first next()), the current term is null. If we would change this and place it on the
first term of the underlying enum we would break everything: TermsEnum must be unpositioned
initially before next() [this was the broken thing before 4.0] and we would have an unneeded
seek on a never needed term.

So you can remove that comment from patch!

      was (Author: thetaphi):
    That patch looks very good!

{noformat}
+      if (term == null) // TODO: is this really necessary? we should be positioned already?
{noformat}

The check is really necessary, as on the first call of nextSeekTerm on the uninitialized TermsEnum
(before first next()), the current term is null. If we would change this and place it on the
first term of the underlying enum we would break everything: TermsEnum must be unpositioned
initially before next() [this was the broken thing before 4.0] and we would have an unneeded
seek on a never needed term.

So you can remove that line from patch!
  
> Change all FilteredTermsEnum impls into TermsEnum decorators
> ------------------------------------------------------------
>
>                 Key: LUCENE-2784
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2784
>             Project: Lucene - Java
>          Issue Type: Improvement
>            Reporter: Robert Muir
>            Assignee: Uwe Schindler
>             Fix For: 4.0
>
>         Attachments: LUCENE-2784.patch, LUCENE-2784.patch
>
>
> Currently, FilteredTermsEnum has two ctors:
> * FilteredTermsEnum(IndexReader reader, String field)
> * FilteredTermsEnum(TermsEnum tenum)
> But most of our concrete implementations (e.g. TermsRangeEnum) use the IndexReader+field
ctor
> In my opinion we should remove this ctor, and switch over all FilteredTermsEnum implementations
to just take a TermsEnum.
> Advantages:
> * This simplifies FilteredTermsEnum and its subclasses, where they are more decorator-like
(perhaps in the future we could compose them)
> * Removes silly checks such as if (tenum == null) in every next()
> * Allows for consumers to pass in enumerators however they want: e.g. its their responsibility
if they want to use MultiFields or not, it shouldnt be buried in FilteredTermsEnum.
> I created a quick patch (all core+contrib+solr tests pass), but i think this opens up
more possibilities for refactoring improvements that haven't yet been done in the patch: we
should explore these too.

-- 
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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message