[jira] Created: (LUCENE-1356) Allow easy extensions of TopDocCollector

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] Created: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
Allow easy extensions of TopDocCollector
----------------------------------------

                 Key: LUCENE-1356
                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
             Project: Lucene - Java
          Issue Type: Improvement
          Components: Index
            Reporter: Shai Erera
            Priority: Minor
         Attachments: 1356.patch

TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)

     [ https://issues.apache.org/jira/browse/LUCENE-1356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera updated LUCENE-1356:
-------------------------------

    Attachment: 1356.patch

The very trivial patch

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Priority: Minor
>         Attachments: 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Assigned: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

     [ https://issues.apache.org/jira/browse/LUCENE-1356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doron Cohen reassigned LUCENE-1356:
-----------------------------------

    Assignee: Doron Cohen

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>         Attachments: 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

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

Doron Cohen commented on LUCENE-1356:
-------------------------------------

Shai, Thanks for creating this issue and patch!

I noticed you also modified the private reusableSD to be protected.

This field is just for avoiding creating a new object at each insert to the priority queue.
Note that TopFieldDocCollector maintains its own reusable object for this matter, and it of a 'slightly' different type.

I am wondering if the right thing to do is to (1) leave that field private, or (2) make it protected but then make TopFieldDocCollector  use it too.
I'm inclined for option 1.

What do you think? Others?

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>         Attachments: 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

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

Shai Erera commented on LUCENE-1356:
------------------------------------

IMO, TopFieldDocCollector should be changed to use reusableSD. FieldDoc extends ScoreDoc. That's the reason I modified it to protected - for extensions of TopDocCollector who maintain in PQ ScoreDoc types (either ScoreDoc or extensions).
I don't see any advantage in marking it private, nor any disadvantage if any extension to TopDocCollector will maintain its own ScoreDoc instance.
It's just that we have TopDocCollector, TopFieldDocCollector and my extension to TDC which insert ScoreDoc instances into PQ, so it made sense to me to change it to protected.

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>         Attachments: 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

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

Shai Erera commented on LUCENE-1356:
------------------------------------

Re-thinking this - resuableSD should remain private to both TDC and TFDC. The reason is the two classes use it differently and don't share any implementation which involves this member (unlike totalHits and hq).

I was in the middle of adding javadoc to the protected members and constructor (which accepts numHits and PQ) when I noticed that numHits is completely ignored by this constructor --> TopDocCollector(int numHits, PriortityQueue hq).
The reason is that PQ is be probably configured to hold a maximum number of hits.

What bothers me with this constructor is that it may falsely lead users of the API to think that it limits their PQ with a maximum number of hits. I think we should remove that parameter and expose two constructors:
public TopDocCollector(int numHits) AND public TopDocCollector(PriorityQueue hq).

If you agree, I'll reflect that in the 2nd patch I want to create (which adds javadoc).

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>         Attachments: 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

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

Doron Cohen commented on LUCENE-1356:
-------------------------------------

You're right, this is confusing indeed.
Although it is not public or protected there may users code
(residing in same package) relying on this method so it can't
just be removed but rather just deprecated.

{quote}
If you agree, I'll reflect that in the 2nd patch I want to create (which adds javadoc).
{quote}
Yes thanks!

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>         Attachments: 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

     [ https://issues.apache.org/jira/browse/LUCENE-1356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera updated LUCENE-1356:
-------------------------------

    Attachment: 1356-2.patch

Marked the constructor as deprecated, created another one (protected) which accepts PQ only and modified TopFieldDocCollector to use the new c'tor instead of the deprecated one.
Also added javadoc.

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>             Fix For: 2.3.3
>
>         Attachments: 1356-2.patch, 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

     [ https://issues.apache.org/jira/browse/LUCENE-1356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera updated LUCENE-1356:
-------------------------------

    Fix Version/s: 2.3.3
    Lucene Fields: [New, Patch Available]  (was: [Patch Available, New])

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>             Fix For: 2.3.3
>
>         Attachments: 1356-2.patch, 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

     [ https://issues.apache.org/jira/browse/LUCENE-1356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mark Miller updated LUCENE-1356:
--------------------------------

    Fix Version/s: 2.4

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>             Fix For: 2.3.3, 2.4
>
>         Attachments: 1356-2.patch, 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

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

Michael McCandless commented on LUCENE-1356:
--------------------------------------------

Doron is this one ready to go in?

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>             Fix For: 2.3.3, 2.4
>
>         Attachments: 1356-2.patch, 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

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

Doron Cohen commented on LUCENE-1356:
-------------------------------------

It is, applies cleanly and seems correct.
Will commit as soon as tests complete.


> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>             Fix For: 2.3.3, 2.4
>
>         Attachments: 1356-2.patch, 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Resolved: (LUCENE-1356) Allow easy extensions of TopDocCollector

Sebastian Nagel (Jira)
In reply to this post by Sebastian Nagel (Jira)

     [ https://issues.apache.org/jira/browse/LUCENE-1356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doron Cohen resolved LUCENE-1356.
---------------------------------

       Resolution: Fixed
    Lucene Fields: [New, Patch Available]  (was: [Patch Available, New])

Thanks Shai !

> Allow easy extensions of TopDocCollector
> ----------------------------------------
>
>                 Key: LUCENE-1356
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1356
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Shai Erera
>            Assignee: Doron Cohen
>            Priority: Minor
>             Fix For: 2.3.3, 2.4
>
>         Attachments: 1356-2.patch, 1356.patch
>
>
> TopDocCollector's members and constructor are declared either private or package visible. It makes it hard to extend it as if you want to extend it you can reuse its *hq* and *totatlHits* members, but need to define your own. It also forces you to override getTotalHits() and topDocs().
> By changing its members and constructor (the one that accepts a PQ) to protected, we allow users to extend it in order to get a different view of 'top docs' (like TopFieldCollector does), but still enjoy its getTotalHits() and topDocs() method implementations.

--
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: [hidden email]
For additional commands, e-mail: [hidden email]