[jira] [Commented] (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-tabpanelfocusedCommentId=13155094#comment-13155094
]
Uwe Schindler commented on LUCENE-3464:
---
I will add a comment to the javadocs with a simple statement:
{noformat}
* pbNote:/b The default implementation of {@link
FilterIndexReader#doOpenIfChanged}
* throws {@link UnsupportedOperationException} (like the base class),
* so it's not possible to reopen a codeFilterIndexReader/code.
* To reopen, you have to first reopen the underlying reader
* and wrap it again with the custom filter.
{noformat}
In this case the responsibility is moved over to the consumer. Its the same
like with SlowMultiReaderWrapper: It does not support reopen, to support this,
reopen the underlying reader and make it slow again :-)
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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154434#comment-13154434
]
Robert Muir commented on LUCENE-3464:
-
{quote}
The problem: This cannot be implemented if the custom Filter is in a 3rd party
package, as it cannot call the protected doOpenIfChanged.
{quote}
I don't understand this statement, because FilterReader extends IndexReader so
it should be able to call protected IR methods.
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-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154437#comment-13154437 ] Simon Willnauer commented on LUCENE-3464: - bq. I don't understand this statement, because FilterReader extends IndexReader so it should be able to call protected IR methods. but not the delegates method though. 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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154440#comment-13154440
]
Uwe Schindler commented on LUCENE-3464:
---
This code does not compile (fails by saying IndexReader has no accessible
method doOpenIfChanged()):
{code:java}
package my.sophisticated.package;
@Override
protected IndexReader doOpenIfChanged() throws CorruptIndexException,
IOException {
final IndexReader n=in.doOpenIfChanged();
return (n==null) ? null : new MySophisticatedIndexReader(n);
}
{code}
This is the working workaround but looks wrong (and works around the
VirtualMethod issues):
{code:java}
@Override
protected IndexReader doOpenIfChanged() throws CorruptIndexException,
IOException {
final IndexReader n=IndexReader.openIfChanged(in);
return (n==null) ? null : new MySophisticatedIndexReader(n);
}
{code}
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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154442#comment-13154442 ] Uwe Schindler commented on LUCENE-3464: --- bq. Thats no problem, it just calls super.doOpenIfChanged, see my proof of concept. This throws UOE as FilterIndexReader (correctly!!!) does not implement doOpenIfChanged. If it would implement if a FilterIndexReader without any doOpenIfChanged impl would be horribly broken as it would return an unfiltered reader!!! 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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154441#comment-13154441 ] Robert Muir commented on LUCENE-3464: - Uwe, see my patch, it uses its own sophisticated packaged and works fine. 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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=1315#comment-1315 ] Robert Muir commented on LUCENE-3464: - In my patch FIR returns whatever the underlying reader does. If you want this to be UOE *if doReopen is not overridden*, well, you know how to do that :) 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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154445#comment-13154445 ] Uwe Schindler commented on LUCENE-3464: --- Robert read my comment about the reason why FilterIndexReader should never delegate to doOpenIfChanged(), as it would return an unfiltered reader on reopening, which would be a hidden bug for an implementor! 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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154448#comment-13154448 ] Uwe Schindler commented on LUCENE-3464: --- Robert: If you wrap a standard SegmentReader, it supports doOpenIfChanged. If I wrap it with my own custom FilterIndexReader that does not implement doOpenIfChanged, it will silently pass the return value of SegmentReader.doOpenIfChanged() which is no longer filtered. By throwing UOE in the default FilterIndexReader the user will see this when he tries to reopen. 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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154449#comment-13154449 ] Robert Muir commented on LUCENE-3464: - As I said before, this is simple, FIR's impl only delegates if its overriden. otherwise throws UOE. 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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154456#comment-13154456 ] Uwe Schindler commented on LUCENE-3464: --- bq. As I said before, this is simple, FIR's impl only delegates if its overriden. otherwise throws UOE. Not in your proof of concept. And this proof of concept is no proof as it modifies FilteredIndexReader, so it would not work with 3.5.0RC1. Please note: For the above reason: FilterIndexReader in the past never delegated reopen() - for this exact reason (there was an issue open why delegating is wrong - have to look it up). 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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154463#comment-13154463
]
Robert Muir commented on LUCENE-3464:
-
{quote}
Not in your proof of concept. And this proof of concept is no proof as it
modifies FilteredIndexReader, so it would not work with 3.5.0RC1.
{quote}
I'm not arguing that nothing needs to be done: I'm just saying that the fact
its protected isnt really a problem.
For 3.5.0 there is already a workaround.
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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13154468#comment-13154468 ] Robert Muir commented on LUCENE-3464: - And just again: I want to point out this is the same fundamental problem as LUCENE-2828. This is why abstract classes are NOT the solution to backwards compatibility, because delegators over abstract classes get all jacked up. 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, LUCENE-3464_see_its_just_fine.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13120014#comment-13120014 ] Uwe Schindler commented on LUCENE-3464: --- This sophisticated backwards seems fine. The most common use-case is: One has a FilterIndexReader. All FilterIndexReaders should override reopen(), as the reopened reader has to be wrapped, too. In that case the filter would class super.reopen (or delegate.reopen), this would then trigger backwards. IndexReader.openIfChanged() would also work on this filtered reader, as it would detect the FilterIR to be old-style and would call its reopen method (so the call chain would be: IR.openIfChanged(filter) - filter.reopen() - wrappedReader.reopen() - IR.openIfChanged(wrappedReader)) I have to think about other use-cases, but the most common one seems to be fine. 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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114492#comment-13114492 ] Simon Willnauer commented on LUCENE-3464: - just a couple of variants: tryReopen() reopenOnChange() just and idea, we could also have a higherlevel api here on IR that refreshes the reader something like IR IR#refresh(IR prefious, boolean closePrevious) 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114521#comment-13114521 ] Martijn van Groningen commented on LUCENE-3464: --- +1 reopenOnChange() Names that contain maybe or try sound vague to me as a user. 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114656#comment-13114656 ] Doron Cohen commented on LUCENE-3464: - I liked reopen()... (but also like returning null in case there's nothing newer...) If the name is going to change, two additional names to consider: * newest() * newer() For newest() I think current behavior of returning this makes sense when this is the newest. For newer() returning null in that case seems right. One problem I have with these names is that they both seem to hide the fact that things are going on down there, when it is required to open a new reader... 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114662#comment-13114662 ] Ryan McKinley commented on LUCENE-3464: --- maybe: reopenIfChanged() reopenOnChange suggests, it will reopen when something changes. 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114667#comment-13114667 ] Uwe Schindler commented on LUCENE-3464: --- We can even do the change in 3.x, if we keep the old method delegating to the new one. In 3.x we also need some sophisticated VirtualMethod usage then, as we have to take care of custom IR implementations that may only override the old one :( - unfortunately, reopen() is not final in IR and uses doReopen, like the other methods. reopenIfChanged() - my favourite! 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114748#comment-13114748 ] Michael McCandless commented on LUCENE-3464: I like reopenIfChanged too! bq. unfortunately, reopen() is not final in IR and uses doReopen, Can we just make it final when we backport this change? It's very expert to externally subclass IR and override reopen... 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114760#comment-13114760 ] Michael McCandless commented on LUCENE-3464: bq. Can we just make it final when we backport this change? Duh no we can't since we have core IR impls subclassing IR... 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114795#comment-13114795 ] Michael McCandless commented on LUCENE-3464: Another trappiness I've seen on IR.reopen is... the method implies that the reopen will happen in place. And I've seen users try to simply do IR.reopen(). Maybe, the public API we expose should be something like IR.openIfChanged(oldReader), a static method? This would open a new reader only if there are changes vs the old one, and would try to share resources (ie shared segments) with the old reader, when possible. We'd have to add protected IR methods that really try to do the reopen. 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13115030#comment-13115030 ] Ryan McKinley commented on LUCENE-3464: --- bq. the method implies that the reopen will happen in place. And I've seen users try to simply do IR.reopen(). funny, that's what i thought it did! If you have to use the results value, should it be: getFreshReader(oldReader) or something? without the 'get' it seems like it operates on the reader itself, not the return value. 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.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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (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-tabpanelfocusedCommentId=13114354#comment-13114354 ] Mark Miller commented on LUCENE-3464: - bq. reopenIfNeeded +1 - can't think of anything better yet 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 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. For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
