[jira] [Comment Edited] (LUCENE-8292) Fix FilterLeafReader.FilterTermsEnum to delegate all seekExact methods
[ https://issues.apache.org/jira/browse/LUCENE-8292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16475579#comment-16475579 ] Bruno Roustant edited comment on LUCENE-8292 at 5/15/18 9:57 AM: - Actually there is also another related issue with this FilterLeafReader#FilterTermsEnum delegate pattern. It does not delegate termState() nor seekExact(ByteRef, TermState) methods. Which means the termState is never used, so the term queries repeat twice the same seek (seekCeil) instead of using the termState to improve performance (normally the termState is kept by TermContext#build()). Practical example: When one configures a timeout for queries, internally an ExitableDirectoryReader is created. And its ExitableTermsEnum, which extends FilterTermsEnum, makes all term queries repeat twice the same seekCeil(). was (Author: bruno.roustant): Actually there is also another related issue with this FilterLeafReader#FilterTermsEnum delegate pattern. It does not delegate termState() nor seekExact(ByteRef, TermState) methods. Which means the termState is never used, so the term queries repeat twice the same seek (seekCeil) instead of using the termState to improve performance (normally the termState is kept by TermContext#build()). Practical example: When one configures a timeout for queries, internally a ExitableDirectoryReader is created. And its ExitableTermsEnum, which extends FilterTermsEnum, makes all term queries repeat twice the same seekCeil(). > Fix FilterLeafReader.FilterTermsEnum to delegate all seekExact methods > -- > > Key: LUCENE-8292 > URL: https://issues.apache.org/jira/browse/LUCENE-8292 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Affects Versions: 7.2.1 >Reporter: Bruno Roustant >Priority: Major > Fix For: trunk > > Attachments: > 0001-Fix-FilterLeafReader.FilterTermsEnum-to-delegate-see.patch, > LUCENE-8292.patch > > > FilterLeafReader#FilterTermsEnum wraps another TermsEnum and delegates many > methods. > It misses some seekExact() methods, thus it is not possible to the delegate > to override these methods to have specific behavior (unlike the TermsEnum API > which allows that). > The fix is straightforward: simply override these seekExact() methods and > delegate. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Comment Edited] (LUCENE-8292) Fix FilterLeafReader.FilterTermsEnum to delegate all seekExact methods
[
https://issues.apache.org/jira/browse/LUCENE-8292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16465875#comment-16465875
]
David Smiley edited comment on LUCENE-8292 at 5/7/18 12:55 PM:
---
Maybe it would be helpful if we simply add a _comment_ to FilterTermsEnum of
the methods it does not override but could still be by subclasses that wish to?
e.g.:
{code:java}
//inherited methods with default impls that could still be overridden:
// public AttributeSource attributes()
// public boolean seekExact(BytesRef text) throws IOException
// public void seekExact(BytesRef term, TermState state) throws IOException
// public TermState termState() throws IOException
{code}
The main down-side is that it could be easy to forget to maintain this comment
as changes happen over time. WDYT [~jpountz]?
was (Author: dsmiley):
Maybe it would be helpful if we simply add a _commented_ to FilterTermsEnum of
the methods it does not override but could still be by subclasses that wish to?
e.g.:
{code:java}
//inherited methods with default impls that could still be overridden:
// public AttributeSource attributes()
// public boolean seekExact(BytesRef text) throws IOException
// public void seekExact(BytesRef term, TermState state) throws IOException
// public TermState termState() throws IOException
{code}
The main down-side is that it could be easy to forget to maintain this comment
as changes happen over time. WDYT [~jpountz]?
> Fix FilterLeafReader.FilterTermsEnum to delegate all seekExact methods
> --
>
> Key: LUCENE-8292
> URL: https://issues.apache.org/jira/browse/LUCENE-8292
> Project: Lucene - Core
> Issue Type: Bug
> Components: core/index
>Affects Versions: 7.2.1
>Reporter: Bruno Roustant
>Priority: Major
> Fix For: trunk
>
> Attachments:
> 0001-Fix-FilterLeafReader.FilterTermsEnum-to-delegate-see.patch,
> LUCENE-8292.patch
>
>
> FilterLeafReader#FilterTermsEnum wraps another TermsEnum and delegates many
> methods.
> It misses some seekExact() methods, thus it is not possible to the delegate
> to override these methods to have specific behavior (unlike the TermsEnum API
> which allows that).
> The fix is straightforward: simply override these seekExact() methods and
> delegate.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Comment Edited] (LUCENE-8292) Fix FilterLeafReader.FilterTermsEnum to delegate all seekExact methods
[ https://issues.apache.org/jira/browse/LUCENE-8292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16462409#comment-16462409 ] Bruno Roustant edited comment on LUCENE-8292 at 5/3/18 1:08 PM: Another option would be to modify the TermsEnum.seekExact() method and make it final, or have the javadoc be explicit that it should not be overridden. (though I don't like this option) was (Author: bruno.roustant): Another option would be to modify the TermsEnum.seekExact() method and make it final, or have the javadoc be explicit that it should not be overridden. > Fix FilterLeafReader.FilterTermsEnum to delegate all seekExact methods > -- > > Key: LUCENE-8292 > URL: https://issues.apache.org/jira/browse/LUCENE-8292 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Affects Versions: 7.2.1 >Reporter: Bruno Roustant >Priority: Major > Fix For: trunk > > Attachments: > 0001-Fix-FilterLeafReader.FilterTermsEnum-to-delegate-see.patch, > LUCENE-8292.patch > > > FilterLeafReader#FilterTermsEnum wraps another TermsEnum and delegates many > methods. > It misses some seekExact() methods, thus it is not possible to the delegate > to override these methods to have specific behavior (unlike the TermsEnum API > which allows that). > The fix is straightforward: simply override these seekExact() methods and > delegate. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Comment Edited] (LUCENE-8292) Fix FilterLeafReader.FilterTermsEnum to delegate all seekExact methods
[ https://issues.apache.org/jira/browse/LUCENE-8292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16462394#comment-16462394 ] Bruno Roustant edited comment on LUCENE-8292 at 5/3/18 1:03 PM: When looking at TermsEnum API, what I understand is that seekExact() defaults to calling seekCeil(), but if needed (not for correctness but for performance consideration) we can override it to have a specialized seek that searches only the exact term and does not have to position to the next term if not found. This may have an impact for some TermsEnum extensions (a really noticeable impact in my case, that's why I noticed this issue). To me the current behavior of FilterTermsEnum is not correct with regard to TermsEnum API. (And I noticed that AssertingLeafReader overrides seekExact()). Adding these two methods in FilterTermsEnum fixes correctness, even if I agree it makes more room for bugs. was (Author: bruno.roustant): When looking at TermsEnum API, what I understand is that seekExact() defaults to calling seekCeil(), but if needed (not for correctness but for performance consideration) we can override it to have a specialized seek that searches only the exact term and does not have to position to the next term if not found. This may have an impact for some TermsEnum extensions (a really noticeable impact in my case, that's why I noticed this issue). To me the current behavior of FilterTermsEnum is not correct with regard to TermsEnum API. (And I noticed that AssertingLeafReader overrides seekExact()). Adding this two methods in FilterTermsEnum fixes correctness, even if I agree it makes more room for bugs. > Fix FilterLeafReader.FilterTermsEnum to delegate all seekExact methods > -- > > Key: LUCENE-8292 > URL: https://issues.apache.org/jira/browse/LUCENE-8292 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Affects Versions: 7.2.1 >Reporter: Bruno Roustant >Priority: Major > Fix For: trunk > > Attachments: > 0001-Fix-FilterLeafReader.FilterTermsEnum-to-delegate-see.patch, > LUCENE-8292.patch > > > FilterLeafReader#FilterTermsEnum wraps another TermsEnum and delegates many > methods. > It misses some seekExact() methods, thus it is not possible to the delegate > to override these methods to have specific behavior (unlike the TermsEnum API > which allows that). > The fix is straightforward: simply override these seekExact() methods and > delegate. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
