Solr
  1. Solr
  2. SOLR-8290

remove SchemaField.checkFieldCacheSource's unused QParser argument

    Details

    • Type: Wish Wish
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.4, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      From what I could see with a little looking around the argument was added in 2011 but not used then or since.

      1. SOLR-8290.patch
        5 kB
        Christine Poerschke

        Issue Links

          Activity

          Hide
          Christine Poerschke added a comment -

          patch against trunk

          Show
          Christine Poerschke added a comment - patch against trunk
          Hide
          ASF subversion and git services added a comment -

          Commit 1714766 from Christine Poerschke in branch 'dev/trunk'
          [ https://svn.apache.org/r1714766 ]

          SOLR-8290: remove SchemaField.checkFieldCacheSource's unused QParser argument

          Show
          ASF subversion and git services added a comment - Commit 1714766 from Christine Poerschke in branch 'dev/trunk' [ https://svn.apache.org/r1714766 ] SOLR-8290 : remove SchemaField.checkFieldCacheSource's unused QParser argument
          Hide
          ASF subversion and git services added a comment -

          Commit 1714785 from Christine Poerschke in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1714785 ]

          SOLR-8290: remove SchemaField.checkFieldCacheSource's unused QParser argument (merge in revision 1714766 from trunk)

          Show
          ASF subversion and git services added a comment - Commit 1714785 from Christine Poerschke in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1714785 ] SOLR-8290 : remove SchemaField.checkFieldCacheSource's unused QParser argument (merge in revision 1714766 from trunk)
          Hide
          Hoss Man added a comment -

          Some historical context for when/why this param was added in SOLR-2348...

          with that in mind, i think for now the most straight forward thing to do is to add a "checkFieldCacheSource(QParser)" method to SchemaField that would be cut/paste of checkSortability (with the error message wording changed) and make all of the (applicable) FieldType.getValueSource methods call it. In the future, it could evolve differently then checkSortability – either removing the "!multivalued" assertion completley, or introspecting the Qparser context in some way to determine that neccessary info has been provided to know how to use the (hypothetical) multivalued FieldCache (hard t ospeculate at this point)

          ...since it evidently hasn't proved useful since then, no objections to removing now (although i'm not sure what downside there was to leaving it in either)

          Show
          Hoss Man added a comment - Some historical context for when/why this param was added in SOLR-2348 ... with that in mind, i think for now the most straight forward thing to do is to add a "checkFieldCacheSource(QParser)" method to SchemaField that would be cut/paste of checkSortability (with the error message wording changed) and make all of the (applicable) FieldType.getValueSource methods call it. In the future, it could evolve differently then checkSortability – either removing the "!multivalued" assertion completley, or introspecting the Qparser context in some way to determine that neccessary info has been provided to know how to use the (hypothetical) multivalued FieldCache (hard t ospeculate at this point) ...since it evidently hasn't proved useful since then, no objections to removing now (although i'm not sure what downside there was to leaving it in either)
          Hide
          Christine Poerschke added a comment -

          Thanks for the historical context info. Motivation for the removal is that checkFieldCacheSource not using QParser and potentially SOLR-8305 replacing (ExternalFileField|LatLonType).getValueSource's QParser use then potentially FieldType.getRangeQuery need no longer take a QParser argument (several places are already passing null as the QParser argument for getRangeQuery).

          Show
          Christine Poerschke added a comment - Thanks for the historical context info. Motivation for the removal is that checkFieldCacheSource not using QParser and potentially SOLR-8305 replacing (ExternalFileField|LatLonType).getValueSource 's QParser use then potentially FieldType.getRangeQuery need no longer take a QParser argument (several places are already passing null as the QParser argument for getRangeQuery).

            People

            • Assignee:
              Christine Poerschke
              Reporter:
              Christine Poerschke
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development