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

Remove defaultSearchField and solrQueryParser from schema

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.6
    • Component/s: Schema and Analysis
    • Labels:
      None

      Description

      The two tags <defautlSearchField> and solrQueryParser were deprecated in Solr3.6 (SOLR-2724). This umbrella issue will guide the eventual removal in 7.x by first deprecating and complaining in 6.6.

      1. SOLR-7041.patch
        77 kB
        Jan Høydahl
      2. SOLR-7041-defaultOperator.patch
        48 kB
        Jan Høydahl
      3. SOLR-7041-df.patch
        69 kB
        Jan Høydahl

        Issue Links

          Activity

          Hide
          janhoy Jan Høydahl added a comment -

          I know 5.0.0 is on its way out the door - just want to record this JIRA as I cannot recall nuking this has been discussed for 5.0 and it may be a simple patch.

          See also https://cwiki.apache.org/confluence/display/solr/Other+Schema+Elements

          Show
          janhoy Jan Høydahl added a comment - I know 5.0.0 is on its way out the door - just want to record this JIRA as I cannot recall nuking this has been discussed for 5.0 and it may be a simple patch. See also https://cwiki.apache.org/confluence/display/solr/Other+Schema+Elements
          Hide
          janhoy Jan Høydahl added a comment -

          I see the two used in a bunch of test-schemas. Also, the methods getDefaultSearchFieldName() and getQueryParserDefaultOperator() in IndexSchema.java are not deprecated in current trunk.

          If we don't take the time to rip it all out for 5.0, I propose we

          • remove the commented-out parts from example schemas
          • deprecate the two methods in IndexSchema
          • remove mentions in RefGuide
          • start logging a WARN if schema parser finds any of these in use

          Another in-between option is to fail-fast if luceneMatchVersion >= 5.0 and log warning if less (indicates people brought their old config).

          Show
          janhoy Jan Høydahl added a comment - I see the two used in a bunch of test-schemas. Also, the methods getDefaultSearchFieldName() and getQueryParserDefaultOperator() in IndexSchema.java are not deprecated in current trunk. If we don't take the time to rip it all out for 5.0, I propose we remove the commented-out parts from example schemas deprecate the two methods in IndexSchema remove mentions in RefGuide start logging a WARN if schema parser finds any of these in use Another in-between option is to fail-fast if luceneMatchVersion >= 5.0 and log warning if less (indicates people brought their old config).
          Hide
          romseygeek Alan Woodward added a comment -

          +1 to deprecating properly in 5.0, and removing in trunk.

          Show
          romseygeek Alan Woodward added a comment - +1 to deprecating properly in 5.0, and removing in trunk.
          Hide
          mmurphy3141 Mike Murphy added a comment -

          defaultSearchField is very useful, can we please keep this?

          Show
          mmurphy3141 Mike Murphy added a comment - defaultSearchField is very useful, can we please keep this?
          Hide
          dsmiley David Smiley added a comment -

          Mike, just use 'df' or 'qf' as appropriate. defaultSearchField in schema.xml is trappy.

          Show
          dsmiley David Smiley added a comment - Mike, just use 'df' or 'qf' as appropriate. defaultSearchField in schema.xml is trappy.
          Hide
          janhoy Jan Høydahl added a comment -

          I updated the refGuide to hide away the deprecated tags somewhat more.

          Show
          janhoy Jan Høydahl added a comment - I updated the refGuide to hide away the deprecated tags somewhat more.
          Hide
          janhoy Jan Høydahl added a comment -

          +1 to start complaining in logs from 5.1 and nuke for good, i.e. fail hard in Trunk.

          Show
          janhoy Jan Høydahl added a comment - +1 to start complaining in logs from 5.1 and nuke for good, i.e. fail hard in Trunk.
          Hide
          janhoy Jan Høydahl added a comment -

          Attaching a work-in-progress patch that used to apply to SVN r1692690.
          There is still a long way to go to convert all tests to not using these deprecated tags.

          Show
          janhoy Jan Høydahl added a comment - Attaching a work-in-progress patch that used to apply to SVN r1692690. There is still a long way to go to convert all tests to not using these deprecated tags.
          Hide
          janhoy Jan Høydahl added a comment -

          This would be good to finalize before 7.0.0
          But there are a ton of tests that rely on these, so the last time I tried I ended up nowhere

          Show
          janhoy Jan Høydahl added a comment - This would be good to finalize before 7.0.0 But there are a ton of tests that rely on these, so the last time I tried I ended up nowhere
          Hide
          janhoy Jan Høydahl added a comment -

          Current state of this is that you have been getting a WARN in logs since 3.6 when using these, but they still work. The attached patch (old) changes that warn log into throwing an exception (dependent on luceneMatchVersion). But for that to be committable all tests must also stop using these.

          Since these have been deprecated for a long time, perhaps we don't need to let the removal happen on exactly version 7.0.0, but can allow it to take place in any minor version? Do we need a CHANGES entry in 6.6.0 that this is the last version where you can expect it to still work?

          Show
          janhoy Jan Høydahl added a comment - Current state of this is that you have been getting a WARN in logs since 3.6 when using these, but they still work. The attached patch (old) changes that warn log into throwing an exception (dependent on luceneMatchVersion). But for that to be committable all tests must also stop using these. Since these have been deprecated for a long time, perhaps we don't need to let the removal happen on exactly version 7.0.0, but can allow it to take place in any minor version? Do we need a CHANGES entry in 6.6.0 that this is the last version where you can expect it to still work?
          Hide
          erickerickson Erick Erickson added a comment -

          +1 to put the warning in CHANGES 6.6.0 that these won't work any more, removing them from the tests and committing the exception when the tests pass.

          Show
          erickerickson Erick Erickson added a comment - +1 to put the warning in CHANGES 6.6.0 that these won't work any more, removing them from the tests and committing the exception when the tests pass.
          Hide
          janhoy Jan Høydahl added a comment -

          This patch SOLR-7041-defaultOperator.patch removes all use of defaultOperator in test schemas as well as a lot of unnecessary defaultSearchField usage.

          I'm going to commit this as the first step to get some progress.

          Converting test files from using <defaultSearchField> in schemas into using df in solrconfig is harder, since the same config is used by many test schemas with many different default fields. So either we fork more configs with the only difference being df, or we instrument the beforeClass() method of the tests in question that do not use "text" as df to redefine the /select handler using config API.

          Show
          janhoy Jan Høydahl added a comment - This patch SOLR-7041 -defaultOperator.patch removes all use of defaultOperator in test schemas as well as a lot of unnecessary defaultSearchField usage. I'm going to commit this as the first step to get some progress. Converting test files from using <defaultSearchField> in schemas into using df in solrconfig is harder, since the same config is used by many test schemas with many different default fields. So either we fork more configs with the only difference being df, or we instrument the beforeClass() method of the tests in question that do not use "text" as df to redefine the /select  handler using config API.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 61f64829d84d5a6b8c8bdff0e1a1f32c5e0a86f6 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=61f6482 ]

          SOLR-7041: Remove a lot of defaultOperator and defaultSearchField from test configs (still more work to do)

          Show
          jira-bot ASF subversion and git services added a comment - Commit 61f64829d84d5a6b8c8bdff0e1a1f32c5e0a86f6 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=61f6482 ] SOLR-7041 : Remove a lot of defaultOperator and defaultSearchField from test configs (still more work to do)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 66bf7a8e32ed5c541a30b72df709ec5290c88715 in lucene-solr's branch refs/heads/branch_6x from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=66bf7a8 ]

          SOLR-7041: Remove a lot of defaultOperator and defaultSearchField from test configs (still more work to do)

          (cherry picked from commit 8ecdd67)

          Show
          jira-bot ASF subversion and git services added a comment - Commit 66bf7a8e32ed5c541a30b72df709ec5290c88715 in lucene-solr's branch refs/heads/branch_6x from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=66bf7a8 ] SOLR-7041 : Remove a lot of defaultOperator and defaultSearchField from test configs (still more work to do) (cherry picked from commit 8ecdd67)
          Hide
          janhoy Jan Høydahl added a comment -

          Further work will happen in the sub tasks, to split the elephant a bit

          Show
          janhoy Jan Høydahl added a comment - Further work will happen in the sub tasks, to split the elephant a bit
          Hide
          ppujari Pradeep added a comment -

          Bad. removing defaultOperator and defaultSearchField is bad idea.

          Show
          ppujari Pradeep added a comment - Bad. removing defaultOperator and defaultSearchField is bad idea.
          Hide
          dsmiley David Smiley added a comment -

          Pradeep you can read about how they came to be deprecated in SOLR-2724

          Show
          dsmiley David Smiley added a comment - Pradeep you can read about how they came to be deprecated in SOLR-2724
          Hide
          janhoy Jan Høydahl added a comment -

          Skimmed through SOLR-2724. It's a long time since these discussions and also a long time since the 3.6 deprecation of these schema features.

          What is the sentiment now, in particular from Hoss Man and Yonik Seeley? The code handling defaultOperator or defaultSearchField in schema will go away in 7.0, and we'll fail-fast if we encounter these from 6.6 (silently ignoring would be worse than failing).

          Personally I haven't used these for years. But the main reason I started working on this is that it feels good to actually remove deprecated stuff and simplify the code, config and complexity of the product. But I don't have a problem either with just leaving it in there forever, as seemed to be what some of you (strongly) wanted way back then. If I get serious, well-founded push-back now, I'll unassign myself and leave the fate of this change in the hands of David Smiley

          Show
          janhoy Jan Høydahl added a comment - Skimmed through SOLR-2724 . It's a long time since these discussions and also a long time since the 3.6 deprecation of these schema features. What is the sentiment now, in particular from Hoss Man and Yonik Seeley ? The code handling defaultOperator or defaultSearchField in schema will go away in 7.0, and we'll fail-fast if we encounter these from 6.6 (silently ignoring would be worse than failing). Personally I haven't used these for years. But the main reason I started working on this is that it feels good to actually remove deprecated stuff and simplify the code, config and complexity of the product. But I don't have a problem either with just leaving it in there forever, as seemed to be what some of you (strongly) wanted way back then. If I get serious, well-founded push-back now, I'll unassign myself and leave the fate of this change in the hands of David Smiley
          Hide
          hossman Hoss Man added a comment -

          they've been logging a WARN for so long I don't have strong opinions against removing them in 7.0 ... i'm not sure that i ever had strong opinions about removing them, i just raised the question/concern that there didn't seem to be a lot of benefit in doing so since that code didn't seem to particularly be standing in the way of any new code/features, but (at the time anyway) removing them was confusing/problematic to existing users.

          Show
          hossman Hoss Man added a comment - they've been logging a WARN for so long I don't have strong opinions against removing them in 7.0 ... i'm not sure that i ever had strong opinions about removing them, i just raised the question/concern that there didn't seem to be a lot of benefit in doing so since that code didn't seem to particularly be standing in the way of any new code/features, but (at the time anyway) removing them was confusing/problematic to existing users.
          Hide
          janhoy Jan Høydahl added a comment -

          Ok, here's a patch SOLR-7041-df.patch that cuts over all tests from relying on defaultSearchField in schemas to explicitly state df in either the test's solrconfig.xml or in the query request for the test.

          The patch also removes an orhpan schema file not in use anywhere, and removes a test that explicitly validates the presence of <defaultSearchField> in schema.

          With this, it should be possible to commit SOLR-10587 and later SOLR-10585

          Show
          janhoy Jan Høydahl added a comment - Ok, here's a patch SOLR-7041 -df.patch that cuts over all tests from relying on defaultSearchField in schemas to explicitly state df in either the test's solrconfig.xml or in the query request for the test. The patch also removes an orhpan schema file not in use anywhere, and removes a test that explicitly validates the presence of <defaultSearchField> in schema. With this, it should be possible to commit SOLR-10587 and later SOLR-10585
          Hide
          dsmiley David Smiley added a comment -

          +1 patch LGTM

          Show
          dsmiley David Smiley added a comment - +1 patch LGTM
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit e776cbe4464e52a28ceffd9fa46d7c47ed44bb57 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e776cbe ]

          SOLR-7041: Cut over tests from <defaultSearchField> in schema to df on requests

          Show
          jira-bot ASF subversion and git services added a comment - Commit e776cbe4464e52a28ceffd9fa46d7c47ed44bb57 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e776cbe ] SOLR-7041 : Cut over tests from <defaultSearchField> in schema to df on requests
          Hide
          janhoy Jan Høydahl added a comment -

          Closing this umbrella issue since all four sub tasks are now committed.

          Show
          janhoy Jan Høydahl added a comment - Closing this umbrella issue since all four sub tasks are now committed.

            People

            • Assignee:
              janhoy Jan Høydahl
              Reporter:
              janhoy Jan Høydahl
            • Votes:
              3 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development