Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9987

Implement support for multi-valued DocValues in PointFields

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.5, 7.0
    • Component/s: None
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None

      Description

      This is not currently supported, and since PointFields can't use FieldCache, faceting, stats, etc is not supported on multi-valued point fields. Followup task of SOLR-8396

      1. SOLR-9987.patch
        180 kB
        Tomás Fernández Löbbe
      2. SOLR-9987.patch
        155 kB
        Tomás Fernández Löbbe

        Issue Links

          Activity

          Hide
          tomasflobbe Tomás Fernández Löbbe added a comment -

          As discussed in SOLR-8396, we should use SortedNumericDocValues. There are already some (ignored) tests in TestPointFields for this feature, once it's available.

          Show
          tomasflobbe Tomás Fernández Löbbe added a comment - As discussed in SOLR-8396 , we should use SortedNumericDocValues . There are already some (ignored) tests in TestPointFields for this feature, once it's available.
          Hide
          tomasflobbe Tomás Fernández Löbbe added a comment -

          Here is a first patch. Some nocommits and some tests are failing. This requires LUCENE-7673

          Show
          tomasflobbe Tomás Fernández Löbbe added a comment - Here is a first patch. Some nocommits and some tests are failing. This requires LUCENE-7673
          Hide
          tomasflobbe Tomás Fernández Löbbe added a comment -

          Here is another patch, with some more tests and support for multivalued stats (same feature support as single value). I think this is ready to commit

          Show
          tomasflobbe Tomás Fernández Löbbe added a comment - Here is another patch, with some more tests and support for multivalued stats (same feature support as single value). I think this is ready to commit
          Hide
          dsmiley David Smiley added a comment -

          Wow you did a lot of work for this! Thanks for doing this work.

          As I skimmed through with curiosity, one thing caught my eye:

          +//            We should do this, but mincount=0 is currently the default
          +//            if (ft.isPointField() && mincount <= 0) {
          +//              throw new SolrException(ErrorCode.BAD_REQUEST, FacetParams.FACET_MINCOUNT + " <= 0 is not supported on point types");
          +//            }
          

          Judging from this and nearby code changes, if minCount is 0 then you make it 1 instead (for PointFields only); is this right? This seems wrong to me. I suspect we all agree that the default of 0 is unfortunate but ignoring it and pretending it's not in certain cases but not others seems wrong. Instead, I propose that we make the default '1' for 7.0. Obviously such a change would be in another issue.

          Show
          dsmiley David Smiley added a comment - Wow you did a lot of work for this! Thanks for doing this work. As I skimmed through with curiosity, one thing caught my eye: + // We should do this , but mincount=0 is currently the default + // if (ft.isPointField() && mincount <= 0) { + // throw new SolrException(ErrorCode.BAD_REQUEST, FacetParams.FACET_MINCOUNT + " <= 0 is not supported on point types" ); + // } Judging from this and nearby code changes, if minCount is 0 then you make it 1 instead (for PointFields only); is this right? This seems wrong to me. I suspect we all agree that the default of 0 is unfortunate but ignoring it and pretending it's not in certain cases but not others seems wrong. Instead, I propose that we make the default '1' for 7.0. Obviously such a change would be in another issue.
          Hide
          tomasflobbe Tomás Fernández Löbbe added a comment -

          Thanks for reviewing David. The issue is that facet.mincount=0 doesn't work currently (SOLR-10033), and it's not easy, since we don't have TermsEnum for PointFields. So yes, if the user sets facet.mincount=0 in the request I'm currently blindly changing it to 1, which is the same thing that happens today with TrieFields that have indexed="false" and docValues="true" (since SOLR-5260, before that we would throw an exception).
          An extra issue is that currently by default in distributed facet requests, shard requests are always set to facet.mincount=0 (see SOLR-8988 and SOLR-9152). I don't want to put this exception back until the defaults change.

          Show
          tomasflobbe Tomás Fernández Löbbe added a comment - Thanks for reviewing David. The issue is that facet.mincount=0 doesn't work currently ( SOLR-10033 ), and it's not easy, since we don't have TermsEnum for PointFields. So yes, if the user sets facet.mincount=0 in the request I'm currently blindly changing it to 1, which is the same thing that happens today with TrieFields that have indexed="false" and docValues="true" (since SOLR-5260 , before that we would throw an exception). An extra issue is that currently by default in distributed facet requests, shard requests are always set to facet.mincount=0 (see SOLR-8988 and SOLR-9152 ). I don't want to put this exception back until the defaults change.
          Hide
          dsmiley David Smiley added a comment -

          Ugh; ok. I was a watcher on SOLR-5260 but missed the implication.

          +1 to commit then.

          Show
          dsmiley David Smiley added a comment - Ugh; ok. I was a watcher on SOLR-5260 but missed the implication. +1 to commit then.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 7dcf9de41f6435a741910a6367ef9fece11a588b in lucene-solr's branch refs/heads/master from Tomas Fernandez Lobbe
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7dcf9de ]

          SOLR-9987: Implement support for multi-valued DocValues in PointFields
          CC SOLR-8396

          Show
          jira-bot ASF subversion and git services added a comment - Commit 7dcf9de41f6435a741910a6367ef9fece11a588b in lucene-solr's branch refs/heads/master from Tomas Fernandez Lobbe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7dcf9de ] SOLR-9987 : Implement support for multi-valued DocValues in PointFields CC SOLR-8396
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit bc10fa67b641d0cfb9bd1954378019d4fc343ae8 in lucene-solr's branch refs/heads/branch_6x from Tomas Fernandez Lobbe
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=bc10fa6 ]

          SOLR-9987: Implement support for multi-valued DocValues in PointFields
          CC SOLR-8396

          Show
          jira-bot ASF subversion and git services added a comment - Commit bc10fa67b641d0cfb9bd1954378019d4fc343ae8 in lucene-solr's branch refs/heads/branch_6x from Tomas Fernandez Lobbe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=bc10fa6 ] SOLR-9987 : Implement support for multi-valued DocValues in PointFields CC SOLR-8396
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 81b4288a2133dce87e0ac92da5f6e37dc28176f6 in lucene-solr's branch refs/heads/branch_6x from Tomas Fernandez Lobbe
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=81b4288 ]

          SOLR-8396, SOLR-9987, SOLR-10011: Move CHANGES entries from 7.0 to 6.5

          Show
          jira-bot ASF subversion and git services added a comment - Commit 81b4288a2133dce87e0ac92da5f6e37dc28176f6 in lucene-solr's branch refs/heads/branch_6x from Tomas Fernandez Lobbe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=81b4288 ] SOLR-8396 , SOLR-9987 , SOLR-10011 : Move CHANGES entries from 7.0 to 6.5

            People

            • Assignee:
              tomasflobbe Tomás Fernández Löbbe
              Reporter:
              tomasflobbe Tomás Fernández Löbbe
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development