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

Add support for CollapseQParser with PointFields

    Details

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

      Description

      Followup task of SOLR-8396

      1. SOLR-9994.patch
        39 kB
        Varun Thacker
      2. SOLR-9994.patch
        42 kB
        Cao Manh Dat
      3. SOLR-9994.patch
        39 kB
        Varun Thacker

        Issue Links

          Activity

          Hide
          varunthacker Varun Thacker added a comment -
          • Patch which adds Points support to the collapse query parser
          • In the tests we change the fields from "test_ti"/"test_tl"/"test_tf" to "test_i"/"test_l"/"test_f" dynamic fields.
          • These dynamic fields are randomly picked between the Trie and Point field variants by SolrTestCaseJ4.
          Show
          varunthacker Varun Thacker added a comment - Patch which adds Points support to the collapse query parser In the tests we change the fields from "test_ti"/"test_tl"/"test_tf" to "test_i"/"test_l"/"test_f" dynamic fields. These dynamic fields are randomly picked between the Trie and Point field variants by SolrTestCaseJ4.
          Hide
          dsmiley David Smiley added a comment -

          At first I was confused what Points has to do with CollapseQParser since collapsing should be on DocValues, not on an index (be it Terms or Points) but now I understand that it's still DocValues, it's just that the collapse doesn't know about these new field types. I wonder if these features that need to detect the DocValues type might be improved by introducing some method on FieldType that returns the Int/Long/Float/Double interpretation of the numeric DocValues, assuming the field even has numeric DocValues? The result would be reducing instanceof checks (usually a good thing) which also allows for more flexibility of user defined special numeric fields. Heck you could even collapse on an enum field then.

          Show
          dsmiley David Smiley added a comment - At first I was confused what Points has to do with CollapseQParser since collapsing should be on DocValues, not on an index (be it Terms or Points) but now I understand that it's still DocValues, it's just that the collapse doesn't know about these new field types. I wonder if these features that need to detect the DocValues type might be improved by introducing some method on FieldType that returns the Int/Long/Float/Double interpretation of the numeric DocValues, assuming the field even has numeric DocValues? The result would be reducing instanceof checks (usually a good thing) which also allows for more flexibility of user defined special numeric fields. Heck you could even collapse on an enum field then.
          Hide
          caomanhdat Cao Manh Dat added a comment -

          Update patch, enable PointFIelds on TestRandomCollapseQParserPlugin and remove unused imports.
          David Smiley That's a good idea. But should we create another ticket for the improvement?
          Varun Thacker Do you think the patch in good shape for commit?

          Show
          caomanhdat Cao Manh Dat added a comment - Update patch, enable PointFIelds on TestRandomCollapseQParserPlugin and remove unused imports. David Smiley That's a good idea. But should we create another ticket for the improvement? Varun Thacker Do you think the patch in good shape for commit?
          Hide
          dsmiley David Smiley added a comment -

          David Smiley That's a good idea. But should we create another ticket for the improvement?

          Yeah it would be a separate issue.

          Show
          dsmiley David Smiley added a comment - David Smiley That's a good idea. But should we create another ticket for the improvement? Yeah it would be a separate issue.
          Hide
          varunthacker Varun Thacker added a comment -

          Hi Dat,

          I took your latest patch and made a couple of very small changes.

          The changes were spaces before the if statements and also removing periods in the logging statements.

          Looks good to me otherwise. +1 to commit

          Show
          varunthacker Varun Thacker added a comment - Hi Dat, I took your latest patch and made a couple of very small changes. The changes were spaces before the if statements and also removing periods in the logging statements. Looks good to me otherwise. +1 to commit
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 444db592cb53dcecc063139a1c5fc5a088ca1079 in lucene-solr's branch refs/heads/branch_6x from Cao Manh Dat
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=444db59 ]

          SOLR-9994: Add support for CollapseQParser with PointFields

          Show
          jira-bot ASF subversion and git services added a comment - Commit 444db592cb53dcecc063139a1c5fc5a088ca1079 in lucene-solr's branch refs/heads/branch_6x from Cao Manh Dat [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=444db59 ] SOLR-9994 : Add support for CollapseQParser with PointFields
          Hide
          caomanhdat Cao Manh Dat added a comment -

          Made a mistake on commit message at branch master , so here are the log

          Commit b7042c1f6e449d7eb33a9daaabda0e0d2a53e95b in lucene-solr's branch refs/heads/master from Cao Manh Dat
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;a=commit;h=b7042c1f6e449d7eb33a9daaabda0e0d2a53e95b ]
          Add support for CollapseQParser with PointFields

          Show
          caomanhdat Cao Manh Dat added a comment - Made a mistake on commit message at branch master , so here are the log Commit b7042c1f6e449d7eb33a9daaabda0e0d2a53e95b in lucene-solr's branch refs/heads/master from Cao Manh Dat [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;a=commit;h=b7042c1f6e449d7eb33a9daaabda0e0d2a53e95b ] Add support for CollapseQParser with PointFields

            People

            • Assignee:
              caomanhdat Cao Manh Dat
              Reporter:
              tomasflobbe Tomás Fernández Löbbe
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development