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

Introduce score local parameter for {!parent} query parser

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 4.8
    • Fix Version/s: 5.3
    • Component/s: None
    • Labels:
      None

      Description

      I propose to have ability to configure scoring mode by optional local parameter

      Syntax for parent queries is:

      {!parent which=type:parent score=none|avg|max|total|min}...
      Capital case for score values is also accepted
      

      Child query enables scoring by default.

      1. SOLR-5882.patch
        14 kB
        Mikhail Khludnev
      2. SOLR-5882.patch
        5 kB
        Andrey Kudryavtsev

        Issue Links

          Activity

          Hide
          werder Andrey Kudryavtsev added a comment -

          Initial patch

          Show
          werder Andrey Kudryavtsev added a comment - Initial patch
          Hide
          ashfo ash fo added a comment -

          How do I apply this patch on windows? I am using Bitnami Solr 4.10.2 on windows. Thanks.

          Show
          ashfo ash fo added a comment - How do I apply this patch on windows? I am using Bitnami Solr 4.10.2 on windows. Thanks.
          Hide
          werder Andrey Kudryavtsev added a comment -

          Use one of Windows GUI utilities for example. More details - http://stackoverflow.com/questions/517257/how-do-i-apply-a-diff-patch-on-windows

          Show
          werder Andrey Kudryavtsev added a comment - Use one of Windows GUI utilities for example. More details - http://stackoverflow.com/questions/517257/how-do-i-apply-a-diff-patch-on-windows
          Hide
          alessandro.benedetti Alessandro Benedetti added a comment -

          Hi,
          Is there any idea when this patch will be included in Solr Code Base ?
          I find it to be a very interesting aspect to complete the Join feature!

          Cheers

          Show
          alessandro.benedetti Alessandro Benedetti added a comment - Hi, Is there any idea when this patch will be included in Solr Code Base ? I find it to be a very interesting aspect to complete the Join feature! Cheers
          Hide
          mkhludnev Mikhail Khludnev added a comment -

          Andrey Kudryavtsev would you mind to make sure that patch suites for trunk?

          Show
          mkhludnev Mikhail Khludnev added a comment - Andrey Kudryavtsev would you mind to make sure that patch suites for trunk?
          Hide
          mkhludnev Mikhail Khludnev added a comment -

          QParser test found a bug in ToParentBlockJoinQuery.BlockJoinScorer.nextDoc() it occurs on ScoreMode.Min

          minScore = Math.min(childFreq, minScore); // shouldn't it be childScore, not Freq?
          

          I attach a patch includes this fix and test coverage.

          Show
          mkhludnev Mikhail Khludnev added a comment - QParser test found a bug in ToParentBlockJoinQuery.BlockJoinScorer.nextDoc() it occurs on ScoreMode.Min minScore = Math .min(childFreq, minScore); // shouldn't it be childScore, not Freq? I attach a patch includes this fix and test coverage.
          Hide
          mkhludnev Mikhail Khludnev added a comment -

          attaching patch.

          • introduce {!parent score=..}
          • it turns out, {!child} already returns scores always
          • introduced util for parsing score enum, it also made query-time join parser more correct, it throws SolrException on wrong enum name.
          • fix ScoreMode.Min at ToParentBlockJoin and test it

          I'm kindly ask to review. Due to the last item, I want to commit it before 5.3 cut.

          Show
          mkhludnev Mikhail Khludnev added a comment - attaching patch. introduce {!parent score=..} it turns out, {!child} already returns scores always introduced util for parsing score enum, it also made query-time join parser more correct, it throws SolrException on wrong enum name. fix ScoreMode.Min at ToParentBlockJoin and test it I'm kindly ask to review. Due to the last item, I want to commit it before 5.3 cut.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Mikhail Khludnev - does a score mode specified in uppercase throw an exception? Shouldn't that be case-insensitive?

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Mikhail Khludnev - does a score mode specified in uppercase throw an exception? Shouldn't that be case-insensitive?
          Hide
          mkhludnev Mikhail Khludnev added a comment -

          Shalin Shekhar Mangar it fails on upper case, it accepts only Capital and lowercase. Here it mimics SOLR-6234 behavior. I just didn't find a guide how to do that. Is there any examples of case insensitive behavior of qparsers?
          If you prefer I can rework ScoreModeParser which can make both qparsers case insensitive for scoremode, it's not a big deal.

          Show
          mkhludnev Mikhail Khludnev added a comment - Shalin Shekhar Mangar it fails on upper case, it accepts only Capital and lowercase. Here it mimics SOLR-6234 behavior. I just didn't find a guide how to do that. Is there any examples of case insensitive behavior of qparsers? If you prefer I can rework ScoreModeParser which can make both qparsers case insensitive for scoremode, it's not a big deal.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Okay, I don't have a serious objection to this but I just find it weird to require case-sensitivity for such parameter names. Perhaps we can use lowercase only?

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Okay, I don't have a serious objection to this but I just find it weird to require case-sensitivity for such parameter names. Perhaps we can use lowercase only?
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Oh and rest of the patch looks good to me, thanks!

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Oh and rest of the patch looks good to me, thanks!
          Hide
          mkhludnev Mikhail Khludnev added a comment -

          lowercase works. and it's up to user to leave the first letter Capital like in enum's javadoc

          Show
          mkhludnev Mikhail Khludnev added a comment - lowercase works. and it's up to user to leave the first letter Capital like in enum's javadoc
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Okay, cool. +1 to commit!

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Okay, cool. +1 to commit!
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1693926 from mkhl@apache.org in branch 'dev/trunk'
          [ https://svn.apache.org/r1693926 ]

          SOLR-5882: introducing local param

          {!parent score=..}

          ..
          fixing ScoreMode.Min for ToParentBlockJoinQuery
          fixing ScoreMode parsing exception in ScoreJoinQParserPlugin

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1693926 from mkhl@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1693926 ] SOLR-5882 : introducing local param {!parent score=..} .. fixing ScoreMode.Min for ToParentBlockJoinQuery fixing ScoreMode parsing exception in ScoreJoinQParserPlugin
          Hide
          yseeley@gmail.com Yonik Seeley added a comment -

          > Okay, I don't have a serious objection to this but I just find it weird to require case-sensitivity for such parameter names. Perhaps we can use lowercase only?

          lowercase works. and it's up to user to leave the first letter Capital like in enum's javadoc

          I agree with Shalin, and I don't think the enum value is really relevant (again, interface vs implementation... plus someone could always change the enum).
          So when this is documented in the ref guide, let's use lowecase.

          Show
          yseeley@gmail.com Yonik Seeley added a comment - > Okay, I don't have a serious objection to this but I just find it weird to require case-sensitivity for such parameter names. Perhaps we can use lowercase only? lowercase works. and it's up to user to leave the first letter Capital like in enum's javadoc I agree with Shalin, and I don't think the enum value is really relevant (again, interface vs implementation... plus someone could always change the enum). So when this is documented in the ref guide, let's use lowecase.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1693941 from mkhl@apache.org in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1693941 ]

          SOLR-5882: introducing local param

          {!parent score=..}

          ..
          fixing ScoreMode.Min for ToParentBlockJoinQuery
          fixing ScoreMode parsing exception in ScoreJoinQParserPlugin

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1693941 from mkhl@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1693941 ] SOLR-5882 : introducing local param {!parent score=..} .. fixing ScoreMode.Min for ToParentBlockJoinQuery fixing ScoreMode parsing exception in ScoreJoinQParserPlugin
          Hide
          mkhludnev Mikhail Khludnev added a comment -

          Cassandra Targett note, I mentioned it in the ref guide. It should be fine, because I copypasted your paragraph.

          Show
          mkhludnev Mikhail Khludnev added a comment - Cassandra Targett note, I mentioned it in the ref guide. It should be fine, because I copypasted your paragraph.
          Hide
          mkhludnev Mikhail Khludnev added a comment -
          Show
          mkhludnev Mikhail Khludnev added a comment - Thanks, Andrey Kudryavtsev !
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Bulk close for 5.3.0 release

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release
          Hide
          vstrugatsky Vladimir Strugatsky added a comment -

          It appears that although "score" is available in the parent document, "boost" function has no effect when using the

          {!parent}

          query parser.

          Show
          vstrugatsky Vladimir Strugatsky added a comment - It appears that although "score" is available in the parent document, "boost" function has no effect when using the {!parent} query parser.
          Hide
          mkhludnev Mikhail Khludnev added a comment -

          It should, I believe. Could you provide a specific example including parsed query from debugQuery=true?

          Show
          mkhludnev Mikhail Khludnev added a comment - It should, I believe. Could you provide a specific example including parsed query from debugQuery=true?
          Hide
          vstrugatsky Vladimir Strugatsky added a comment -

          This is running against Solr 5.3.1. Thanks for looking into this:

              "params": {
                "debugQuery": "true",
                "fl": "*,score,[child parentFilter=isParent:true limit=1]",
                "q": "{!parent which=isParent:true score=max}text:copy",
                "boost": "if(termfreq(ownerStatus,\"Supporting\"),0.5,1.0",
                "defType": "edismax"
              }
          

          ...

            "debug": {
              "rawquerystring": "{!parent which=isParent:true score=max}text:copy",
              "querystring": "{!parent which=isParent:true score=max}text:copy",
              "parsedquery": "ToParentBlockJoinQuery(ToParentBlockJoinQuery (text:copi))",
              "parsedquery_toString": "ToParentBlockJoinQuery (text:copi)",
              "explain": {
                "CI_XXX_583": "\n2.1501067 = Score based on child doc range from 486 to 493\n",
          ......
          
          Show
          vstrugatsky Vladimir Strugatsky added a comment - This is running against Solr 5.3.1. Thanks for looking into this: "params" : { "debugQuery" : " true " , "fl" : "*,score,[child parentFilter=isParent: true limit=1]" , "q" : "{!parent which=isParent: true score=max}text:copy" , "boost" : " if (termfreq(ownerStatus,\" Supporting\ "),0.5,1.0" , "defType" : "edismax" } ... "debug" : { "rawquerystring" : "{!parent which=isParent: true score=max}text:copy" , "querystring" : "{!parent which=isParent: true score=max}text:copy" , "parsedquery" : "ToParentBlockJoinQuery(ToParentBlockJoinQuery (text:copi))" , "parsedquery_toString" : "ToParentBlockJoinQuery (text:copi)" , "explain" : { "CI_XXX_583" : "\n2.1501067 = Score based on child doc range from 486 to 493\n" , ......
          Hide
          mkhludnev Mikhail Khludnev added a comment -
          Show
          mkhludnev Mikhail Khludnev added a comment - boost parameter is e?dismax's feature. See https://cwiki.apache.org/confluence/display/solr/The+Extended+DisMax+Query+Parser
          Hide
          vstrugatsky Vladimir Strugatsky added a comment -

          I have a missing parenthesis at the end of the boost - this is a typo, but the boost has no effect with or without the typo.

          Show
          vstrugatsky Vladimir Strugatsky added a comment - I have a missing parenthesis at the end of the boost - this is a typo, but the boost has no effect with or without the typo.
          Hide
          vstrugatsky Vladimir Strugatsky added a comment -

          Indeed, I've tried

                "q": "{!parent which=isParent:true score=max} +_query_:\"{!edismax qf=text}copy\"",
                "boost": "if(termfreq(statusFacet,\"Supporting\"),0.3,1.0)",
          

          with success, but this breaks highlighting per https://issues.apache.org/jira/browse/SOLR-2632. It appears that the combination of query, boost and highlighting is problematic.

          Show
          vstrugatsky Vladimir Strugatsky added a comment - Indeed, I've tried "q" : "{!parent which=isParent: true score=max} +_query_:\" {!edismax qf=text}copy\"", "boost" : " if (termfreq(statusFacet,\" Supporting\ "),0.3,1.0)" , with success, but this breaks highlighting per https://issues.apache.org/jira/browse/SOLR-2632 . It appears that the combination of query , boost and highlighting is problematic.

            People

            • Assignee:
              mkhludnev Mikhail Khludnev
              Reporter:
              werder Andrey Kudryavtsev
            • Votes:
              11 Vote for this issue
              Watchers:
              18 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development