Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-7494

Explore making PointValues a per-field API like doc values

    Details

    • Type: Wish
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 7.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      This is a follow-up to LUCENE-7491. Maybe we could simplify things a bit by changing LeafReader.getPointValues() to LeafReader.getPointValues(String fieldName) and removing all String fieldName parameters from PointValues?

      1. LUCENE-7494.patch
        127 kB
        Adrien Grand

        Activity

        Hide
        mikemccand Michael McCandless added a comment -

        +1

        Show
        mikemccand Michael McCandless added a comment - +1
        Hide
        dsmiley David Smiley added a comment -

        +1.

        Related conceptually is another idea I've had kicking around: Does Fields need to exist? It seems like a pointless intermediary. Why not have LeafReader.getTerms(fieldName) instead? One loses the ability to get the count and iterate over indexed fields, but it's not clear what real use-cases are for that and such rare needs could figure that out with FieldInfos. If it sounds reasonable to you all I'll file a separate issue.

        Show
        dsmiley David Smiley added a comment - +1. Related conceptually is another idea I've had kicking around: Does Fields need to exist? It seems like a pointless intermediary. Why not have LeafReader.getTerms(fieldName) instead? One loses the ability to get the count and iterate over indexed fields, but it's not clear what real use-cases are for that and such rare needs could figure that out with FieldInfos. If it sounds reasonable to you all I'll file a separate issue.
        Hide
        mikemccand Michael McCandless added a comment -

        Fields does seem a bit silly, being such a tiny wrapper class, and it would make things more consistent if you also just passed a field name to the IndexReader API.

        Though, we also return Fields from getTermVectors, since that's essentially a tiny single-document inverted index. I'm not sure how we'd polish that usage away ... I guess we could just narrow it to a new (old!) TermVectors class.

        Show
        mikemccand Michael McCandless added a comment - Fields does seem a bit silly, being such a tiny wrapper class, and it would make things more consistent if you also just passed a field name to the IndexReader API. Though, we also return Fields from getTermVectors , since that's essentially a tiny single-document inverted index. I'm not sure how we'd polish that usage away ... I guess we could just narrow it to a new (old!) TermVectors class.
        Hide
        jpountz Adrien Grand added a comment -

        +1 to nuking Fields too

        Show
        jpountz Adrien Grand added a comment - +1 to nuking Fields too
        Hide
        jpountz Adrien Grand added a comment -

        Here is a patch that deals with points. I'd like to do changes to Fields / term vectors in a different issue.

        Show
        jpountz Adrien Grand added a comment - Here is a patch that deals with points. I'd like to do changes to Fields / term vectors in a different issue.
        Hide
        dsmiley David Smiley added a comment -

        +1 to the patch. I like how it, at least it seems, like there is a net reduction in code.

        Show
        dsmiley David Smiley added a comment - +1 to the patch. I like how it, at least it seems, like there is a net reduction in code.
        Hide
        jpountz Adrien Grand added a comment -

        The diff stats report a reduction of the number of lines indeed:

         51 files changed, 648 insertions(+), 958 deletions(-)
        
        Show
        jpountz Adrien Grand added a comment - The diff stats report a reduction of the number of lines indeed: 51 files changed, 648 insertions(+), 958 deletions(-)
        Hide
        mikemccand Michael McCandless added a comment -

        +1 to the patch, thanks Adrien Grand!

        Show
        mikemccand Michael McCandless added a comment - +1 to the patch, thanks Adrien Grand !
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit c22725f0b53a0d1a50cbc5a9d21ced29e6d7cd39 in lucene-solr's branch refs/heads/master from Adrien Grand
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c22725f ]

        LUCENE-7494: Give points a per-field API.

        Show
        jira-bot ASF subversion and git services added a comment - Commit c22725f0b53a0d1a50cbc5a9d21ced29e6d7cd39 in lucene-solr's branch refs/heads/master from Adrien Grand [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c22725f ] LUCENE-7494 : Give points a per-field API.

          People

          • Assignee:
            Unassigned
            Reporter:
            jpountz Adrien Grand
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development