[jira] [Commented] (LUCENE-6845) Merge Spans and SpanScorer
[ https://issues.apache.org/jira/browse/LUCENE-6845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14968845#comment-14968845 ] ASF subversion and git services commented on LUCENE-6845: - Commit 1709964 from [~romseygeek] in branch 'dev/trunk' [ https://svn.apache.org/r1709964 ] LUCENE-6845: Merge SpanScorer into Spans > Merge Spans and SpanScorer > -- > > Key: LUCENE-6845 > URL: https://issues.apache.org/jira/browse/LUCENE-6845 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: Trunk, 5.4 > > Attachments: LUCENE-6845.patch, LUCENE-6845_norenames.patch, > LUCENE-6845_norenames.patch, LUCENE-6845_norenames.patch.txt > > > SpanScorer and Spans currently share the burden of scoring span queries, with > SpanScorer delegating to Spans for most operations. Spans is essentially a > Scorer, just with the ability to iterate through positions as well, and no > SimScorer to use for scoring. This seems overly complicated. We should > merge the two classes into one. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-6845) Merge Spans and SpanScorer
[ https://issues.apache.org/jira/browse/LUCENE-6845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14968990#comment-14968990 ] ASF subversion and git services commented on LUCENE-6845: - Commit 1709993 from [~romseygeek] in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1709993 ] LUCENE-6845: Merge SpanScorer into Spans > Merge Spans and SpanScorer > -- > > Key: LUCENE-6845 > URL: https://issues.apache.org/jira/browse/LUCENE-6845 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: Trunk, 5.4 > > Attachments: LUCENE-6845.patch, LUCENE-6845_norenames.patch, > LUCENE-6845_norenames.patch, LUCENE-6845_norenames.patch.txt > > > SpanScorer and Spans currently share the burden of scoring span queries, with > SpanScorer delegating to Spans for most operations. Spans is essentially a > Scorer, just with the ability to iterate through positions as well, and no > SimScorer to use for scoring. This seems overly complicated. We should > merge the two classes into one. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-6845) Merge Spans and SpanScorer
[ https://issues.apache.org/jira/browse/LUCENE-6845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14967275#comment-14967275 ] David Smiley commented on LUCENE-6845: -- bq. we already changed the Spans interface in 5.3 by adding Spans.collect(), so users upgrading from 5.2 from 5.3 would have had to recompile I don't see why recompiling would be needed if you merely consume a Spans (don't have a custom implementation) and don't call the newly added method. Any way, I'm +1 to all this. > Merge Spans and SpanScorer > -- > > Key: LUCENE-6845 > URL: https://issues.apache.org/jira/browse/LUCENE-6845 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: Trunk, 5.4 > > Attachments: LUCENE-6845.patch, LUCENE-6845_norenames.patch, > LUCENE-6845_norenames.patch.txt > > > SpanScorer and Spans currently share the burden of scoring span queries, with > SpanScorer delegating to Spans for most operations. Spans is essentially a > Scorer, just with the ability to iterate through positions as well, and no > SimScorer to use for scoring. This seems overly complicated. We should > merge the two classes into one. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-6845) Merge Spans and SpanScorer
[ https://issues.apache.org/jira/browse/LUCENE-6845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14967736#comment-14967736 ] David Smiley commented on LUCENE-6845: -- And the patch is now half the size :-) (thus low-impact on consumers). I do very much like your rename proposals; it's just that those should be on trunk-only, I think. Perhaps in a follow-up commit that doesn't get back-ported. > Merge Spans and SpanScorer > -- > > Key: LUCENE-6845 > URL: https://issues.apache.org/jira/browse/LUCENE-6845 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: Trunk, 5.4 > > Attachments: LUCENE-6845.patch, LUCENE-6845_norenames.patch, > LUCENE-6845_norenames.patch, LUCENE-6845_norenames.patch.txt > > > SpanScorer and Spans currently share the burden of scoring span queries, with > SpanScorer delegating to Spans for most operations. Spans is essentially a > Scorer, just with the ability to iterate through positions as well, and no > SimScorer to use for scoring. This seems overly complicated. We should > merge the two classes into one. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-6845) Merge Spans and SpanScorer
[ https://issues.apache.org/jira/browse/LUCENE-6845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14966707#comment-14966707 ] Alan Woodward commented on LUCENE-6845: --- bq. What is the purpose of SpanNotQuery’s getSpanScorer wrapping the includeSpans with a FilterSpans that has an accept that always returns YES? This is a slightly hacky workaround for the fact that only the top-level Spans implementation actually does any scoring. This means that degenerate cases like single-clause SpanOrQueries or SpanNotQueries with null exclusion clauses can't just return the relevant child Spans, because it won't have a SimScorer set on it. Instead, we need to wrap it with a Spans containing a SimScorer. This is an abuse of FilterSpans though - I'll create a specialised ScoringWrapperSpans instead to use here. If we're really worried about backwards compatibility, we could keep Spans as an empty subclass of SpanScorer. But I really don't think it's necessary - we already changed the Spans interface in 5.3 by adding Spans.collect(), so users upgrading from 5.2 from 5.3 would have had to recompile. > Merge Spans and SpanScorer > -- > > Key: LUCENE-6845 > URL: https://issues.apache.org/jira/browse/LUCENE-6845 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: Trunk, 5.4 > > Attachments: LUCENE-6845.patch, LUCENE-6845_norenames.patch > > > SpanScorer and Spans currently share the burden of scoring span queries, with > SpanScorer delegating to Spans for most operations. Spans is essentially a > Scorer, just with the ability to iterate through positions as well, and no > SimScorer to use for scoring. This seems overly complicated. We should > merge the two classes into one. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-6845) Merge Spans and SpanScorer
[ https://issues.apache.org/jira/browse/LUCENE-6845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14965473#comment-14965473 ] David Smiley commented on LUCENE-6845: -- I looked it over; I like the conceptual simplification this bring. One thing confused me though: What is the purpose of SpanNotQuery’s getSpanScorer wrapping the includeSpans with a FilterSpans that has an accept that always returns YES? I see this in SpanOrQuery too and perhaps others. The rename of Spans -> SpanScorer affects anyone consuming Spans, not just implementers of custom Spans. This is seen in your patch in WeightedSpanTermExtractor, for example. I'm on the fence if this is acceptable or not in a point release; I wonder what others think. Even if we decide to keep backward compatibility, I think the substance of your patch could still land in 5x by having Spans not be renamed to SpanScorer, and keep the existing method names as-is. > Merge Spans and SpanScorer > -- > > Key: LUCENE-6845 > URL: https://issues.apache.org/jira/browse/LUCENE-6845 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: Trunk, 5.4 > > Attachments: LUCENE-6845.patch, LUCENE-6845_norenames.patch > > > SpanScorer and Spans currently share the burden of scoring span queries, with > SpanScorer delegating to Spans for most operations. Spans is essentially a > Scorer, just with the ability to iterate through positions as well, and no > SimScorer to use for scoring. This seems overly complicated. We should > merge the two classes into one. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-6845) Merge Spans and SpanScorer
[ https://issues.apache.org/jira/browse/LUCENE-6845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14964027#comment-14964027 ] David Smiley commented on LUCENE-6845: -- Sounds great. But as I started to look at the patch; it became clear that it's hard to review with the renames to have the Scorer Suffix. Perhaps, temporarily, you could not do that to make it more reviewable? I'm anticipating I'm going to see stuff that suggests this should wait for 6.0. What do you think? > Merge Spans and SpanScorer > -- > > Key: LUCENE-6845 > URL: https://issues.apache.org/jira/browse/LUCENE-6845 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: Trunk, 5.4 > > Attachments: LUCENE-6845.patch > > > SpanScorer and Spans currently share the burden of scoring span queries, with > SpanScorer delegating to Spans for most operations. Spans is essentially a > Scorer, just with the ability to iterate through positions as well, and no > SimScorer to use for scoring. This seems overly complicated. We should > merge the two classes into one. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
