You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Uwe Schindler (Issue Comment Edited) (JIRA)" <ji...@apache.org> on 2011/11/21 21:04:51 UTC
[jira] [Issue Comment Edited] (LUCENE-3464) Rename
IndexReader.reopen to make it clear that reopen may not happen
[ https://issues.apache.org/jira/browse/LUCENE-3464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13154430#comment-13154430 ]
Uwe Schindler edited comment on LUCENE-3464 at 11/21/11 8:03 PM:
-----------------------------------------------------------------
I am reopening this issue as there is some API problem which makes openIfChanged not consistently useable with FilterIndexReader:
If you have a FilterIndexReader that did in the past not implement reopen(...), the base class IndexReader throwed UOE. This was fine, as a FilterIndexReader cannot support reopen unless specifically supported. A FilterIndexReader of course must reopen the delegate reader and then wrap it again to Filter. This was done by overriding reopen() methods, checking if the delegate returned another reader and if yes, wrapping it.
I tried to transform code that implement this pattern to Lucene 3.5RC1 but failed to do it in the clean way: Reopen was replaced by a static IR.openIfChanged(IR oldReader) that delegates to the specific IndexReaders implementation of doOpenIfChanged (which is protected).
To implement the above pattern, doOpenIfChanged must be overridden in FilterIndexReader (again the default *must* thorw UOE, otherwise reopening a filtered reader returns a non-filtered one). This method must call delegate's doOpenIfChanged and if it returns != null, wrap by our FilterIndexReader implementation. The problem: This cannot be implemented if the custom Filter is in a 3rd party package, as it cannot call the protected doOpenIfChanged. The workaround is to use IndexReader.openIfChanged(delegate), but this look borken and violates the pattern.
The good thing with the workaround is that the VirtualMethod sophisticated backwards works correctly. We must at least document this behaviour in FilterIndexReader or fix the API.
was (Author: thetaphi):
I am reopening this issue as there is some API problem which makes openIfChanged not consistently useable with FilterIndexReader:
If you have a FilterIndexReader that did in the past not implement reopen(...), the base class IndexReader throwed UOE. This was fine, as a FilterIndexReader cannot support reopen unless specifically supported. A FilterIndexReader of course must reopen the delegate reader and then wrap it again to Filter. This was done by overriding reopen() methods, checking if the delegate returned another reader and if yes, wrapping it.
I tried to transform code that implement this pattern to Lucene 3.5RC1 but failed to do it in the clean way: Reopen was replaced by a static IR.openIfChanged(IR oldReader) that delegates to the specific IndexReaders implementation of doOpenIfChanged (which is protected).
To implement the above pattern, doOpenIfChanged must be overridden in FilterIndexReader (again the default *must* thorw UOE, otherwise reopening a filtered reader returns a non-filtered one). This method must call delegate's doOpenIfChanged and if it returns != null, wrap by our FilterIndexReader implementation. The problem: This cannot be implemented if the custom Filter is in a 3rd party package, as it cannot call the protected doOpenIfChanged. The workaround is to use IndexReader.openIfChanged(delegate), but this look borken and violates the pattern.
The good this with the workaround is that the VirtualMethod sophisticated backwards works correctly. We must at least document this behaviour in FilterIndexReader or fix the API.
> Rename IndexReader.reopen to make it clear that reopen may not happen
> ---------------------------------------------------------------------
>
> Key: LUCENE-3464
> URL: https://issues.apache.org/jira/browse/LUCENE-3464
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Michael McCandless
> Assignee: Michael McCandless
> Fix For: 3.5, 4.0
>
> Attachments: LUCENE-3464.3x.patch, LUCENE-3464.patch, LUCENE-3464.patch
>
>
> Spinoff from LUCENE-3454 where Shai noted this inconsistency.
> IR.reopen sounds like an unconditional operation, which has trapped users in the past into always closing the old reader instead of only closing it if the returned reader is new.
> I think this hidden maybe-ness is trappy and we should rename it (maybeReopen? reopenIfNeeded?).
> In addition, instead of returning "this" when the reopen didn't happen, I think we should return null to enforce proper usage of the maybe-ness of this API.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org