Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2
    • Component/s: search
    • Labels:
      None

      Description

      using fq=xxx should be sufficient to make a valid query

      see:
      http://www.nabble.com/dismax-without-q%3D-tf3198431.html#a8880594

      1. SOLR-152-DismaxNoQ.patch
        5 kB
        Ryan McKinley
      2. SOLR-152-DismaxNoQ.patch
        6 kB
        Ryan McKinley

        Activity

        Hide
        Hoss Man added a comment -

        past discussion (along with my concerns about what it means semanticly to not have a "q") are in this thread...

        http://www.nabble.com/DisMax-and-null-query-strings-tf2321023.html#a6457790

        ...at the time what was discussed was having the absence of a "q" param would result in the "main" query being a MatchAllDocs – the intent being that having a "q" tells dismax what you want your superset to be, and not having a "q" tells it to default to everything. (since there is no easy syntax for telling dismax to match all like there is in the standard handler)

        This patch takes a different approach: defaulting to nothing (so no matter how many FQs you add, you still get nothing) ... that doesn't really seem that usefull to me – sure it eliminates the exception, but it doesn't gain the user functionality they didn't have before.

        (this behavior isn't obvious when looking at the test in the patch, because the test config has a default "bf" which is being used even when the "q" isn't)

        Show
        Hoss Man added a comment - past discussion (along with my concerns about what it means semanticly to not have a "q") are in this thread... http://www.nabble.com/DisMax-and-null-query-strings-tf2321023.html#a6457790 ...at the time what was discussed was having the absence of a "q" param would result in the "main" query being a MatchAllDocs – the intent being that having a "q" tells dismax what you want your superset to be, and not having a "q" tells it to default to everything. (since there is no easy syntax for telling dismax to match all like there is in the standard handler) This patch takes a different approach: defaulting to nothing (so no matter how many FQs you add, you still get nothing) ... that doesn't really seem that usefull to me – sure it eliminates the exception, but it doesn't gain the user functionality they didn't have before. (this behavior isn't obvious when looking at the test in the patch, because the test config has a default "bf" which is being used even when the "q" isn't)
        Hide
        Ryan McKinley added a comment -

        > (this behavior isn't obvious when looking at the test in the patch, because the test config has a default "bf" which is being used even when the "q" isn't)

        aaaaah. I've been struggling to figure out why it worked on one index and not another!

        I see your point on it not making sense... It is probably beter for the client to send a 'q' that will match everything if that is what it is after.

        But it should throw a 400 exception!

        Show
        Ryan McKinley added a comment - > (this behavior isn't obvious when looking at the test in the patch, because the test config has a default "bf" which is being used even when the "q" isn't) aaaaah. I've been struggling to figure out why it worked on one index and not another! I see your point on it not making sense... It is probably beter for the client to send a 'q' that will match everything if that is what it is after. But it should throw a 400 exception!
        Hide
        Hoss Man added a comment -

        > I see your point on it not making sense... It is probably beter for the client to send a 'q' that > will match everything if that is what it is after.

        i agree... but i don't think it makes sense to try and add some special syntax to the "q" ... it probably makes more sense to add a new "matchall" param – if there is no q, and matchall=true then use a MatchAllDocs instead – if neither are set, throw an exception

        (that way matchall can be set in the solrconfig as either a default or an invariant)

        > But it should throw a 400 exception!

        no disagreement there ... improving the error messages in error input cases is sometihng we need to do a lot of in Solr (most of hte focus in the past was on good behavior for good input)

        Show
        Hoss Man added a comment - > I see your point on it not making sense... It is probably beter for the client to send a 'q' that > will match everything if that is what it is after. i agree... but i don't think it makes sense to try and add some special syntax to the "q" ... it probably makes more sense to add a new "matchall" param – if there is no q, and matchall=true then use a MatchAllDocs instead – if neither are set, throw an exception (that way matchall can be set in the solrconfig as either a default or an invariant) > But it should throw a 400 exception! no disagreement there ... improving the error messages in error input cases is sometihng we need to do a lot of in Solr (most of hte focus in the past was on good behavior for good input)
        Hide
        Ryan McKinley added a comment -

        >
        > (that way matchall can be set in the solrconfig as either a default or an invariant)
        >

        thats good! then you can set:

        <requestHandler name="dismax" class="solr.DisMaxRequestHandler" >
        <lst name="defaults">
        <str name="q">
        matchall
        </str>
        ...

        problem solved.

        Show
        Ryan McKinley added a comment - > > (that way matchall can be set in the solrconfig as either a default or an invariant) > thats good! then you can set: <requestHandler name="dismax" class="solr.DisMaxRequestHandler" > <lst name="defaults"> <str name="q"> matchall </str> ... problem solved.
        Hide
        Hoss Man added a comment -

        don't you mean...

        <requestHandler name="dismax" class="solr.DisMaxRequestHandler" >
        <lst name="defaults">
        <str name="matchall">true</str>
        ...

        ?

        Show
        Hoss Man added a comment - don't you mean... <requestHandler name="dismax" class="solr.DisMaxRequestHandler" > <lst name="defaults"> <str name="matchall">true</str> ... ?
        Hide
        Ryan McKinley added a comment -

        danm, i was hoping you would not notice. There is no delete button around here.

        Show
        Ryan McKinley added a comment - danm, i was hoping you would not notice. There is no delete button around here.
        Hide
        Ryan McKinley added a comment -

        using the matchall parameter

        Show
        Ryan McKinley added a comment - using the matchall parameter
        Hide
        Hoss Man added a comment -

        i haven't looked at the latest patch yet, but it occured to me on the train this morning that a better mechanism then matchall=

        {true}

        |

        {false}

        would be altq=

        {lucene-standard-syntax-query}

        with the psuedoe code being...

        Query mainUserQuery = null;
        if ( params.get(Q) is not empty )

        { mainUserQuery = dismaxParse( params.get(Q) ) }

        else if ( params.get(ALTQ) is not empty )

        { mainUserQuery = SolrQueryParser.parse( params.get(ALTQ) ); }

        if ( null == mainUserQuery)

        { throw NoQueryStringException; }

        ...that way people that want a MatchAllDocs fallback can put altq=: in their defaults or UIR ... but if they want it with a different boost (because they are using BF and BQ) they can do that, or if their index is heavily partitioned by doctype and they only want certain types of docs to be included in the set when no Q is specified they can use "altq=doctype:(product blog)"

        Show
        Hoss Man added a comment - i haven't looked at the latest patch yet, but it occured to me on the train this morning that a better mechanism then matchall= {true} | {false} would be altq= {lucene-standard-syntax-query} with the psuedoe code being... Query mainUserQuery = null; if ( params.get(Q) is not empty ) { mainUserQuery = dismaxParse( params.get(Q) ) } else if ( params.get(ALTQ) is not empty ) { mainUserQuery = SolrQueryParser.parse( params.get(ALTQ) ); } if ( null == mainUserQuery) { throw NoQueryStringException; } ...that way people that want a MatchAllDocs fallback can put altq= : in their defaults or UIR ... but if they want it with a different boost (because they are using BF and BQ) they can do that, or if their index is heavily partitioned by doctype and they only want certain types of docs to be included in the set when no Q is specified they can use "altq=doctype:(product blog)"
        Hide
        Yonik Seeley added a comment -

        Or perhaps annotating the q parameter to avoid the explosion of parameters?

        q=<!lucene>:

        http://www.nabble.com/Using-HTTP-Post-for-Queries-tf3039973.html#a8483387

        Show
        Yonik Seeley added a comment - Or perhaps annotating the q parameter to avoid the explosion of parameters? q=<!lucene> : http://www.nabble.com/Using-HTTP-Post-for-Queries-tf3039973.html#a8483387
        Hide
        Hoss Man added a comment -

        >> q=<!lucene>:

        one of the major focuses of dismax is to take the hassle out of front end client code needing to "preprocess" user queries ... an approach like this would require that the client notice when no query is specified, and set teh q appropraitely (it would also probably require that the client allways put "<!dismax>" in front of most input to "escape" the possibility of a user searching on something like "<!lucene>"

        at a broader level: annotating the "q" doesn't really address the issue of letting the "solrconfig owner" specify what the behavior should be: an extra param can be baked into the config, but a prefix on a user query can not.

        Show
        Hoss Man added a comment - >> q=<!lucene> : one of the major focuses of dismax is to take the hassle out of front end client code needing to "preprocess" user queries ... an approach like this would require that the client notice when no query is specified, and set teh q appropraitely (it would also probably require that the client allways put "<!dismax>" in front of most input to "escape" the possibility of a user searching on something like "<!lucene>" at a broader level: annotating the "q" doesn't really address the issue of letting the "solrconfig owner" specify what the behavior should be: an extra param can be baked into the config, but a prefix on a user query can not.
        Hide
        Yonik Seeley added a comment -

        > an approach like this would require that the client notice when no query is specified, and set teh q appropraitely

        I'm not sure I understand... wouldn't defaulting still work the same way?

        <requestHandler name="dismax" class="solr.DisMaxRequestHandler" >
        <lst name="defaults">
        <str name="q">[!lucene]:</str>

        I thought you were proposing an altq that would work just like q, but it would be in lucene query syntax.
        What happens when one one wants XML syntax?
        But as you say, the dismax syntax is special... in that context, either an altq or q.type param would make sense. Then, the altq could actually have the optional annotations about what type of query it is (if/when we get around to that).

        Show
        Yonik Seeley added a comment - > an approach like this would require that the client notice when no query is specified, and set teh q appropraitely I'm not sure I understand... wouldn't defaulting still work the same way? <requestHandler name="dismax" class="solr.DisMaxRequestHandler" > <lst name="defaults"> <str name="q"> [!lucene] : </str> I thought you were proposing an altq that would work just like q, but it would be in lucene query syntax. What happens when one one wants XML syntax? But as you say, the dismax syntax is special... in that context, either an altq or q.type param would make sense. Then, the altq could actually have the optional annotations about what type of query it is (if/when we get around to that).
        Hide
        Hoss Man added a comment -

        >> at a broader level: annotating the "q" doesn't really address the issue of letting the "solrconfig owner" specify what >> the behavior should be: an extra param can be baked into the config, but a prefix on a user query can not.

        to clarify, the default in the solrconfig could of course be <str name="q"><!lucene>:</str> ... but if the client has to prepend <!dismax> on the front of user input anyway, it can't do it blindly (or it might send an "empty" query of "<!dismax>" and defeat the point of the default)

        with an extra param, the lcient doesn't even have to know anything special is happening when hte user provides an epty string – it doesn't even have to check to see if hte user has provided an empty string

        Show
        Hoss Man added a comment - >> at a broader level: annotating the "q" doesn't really address the issue of letting the "solrconfig owner" specify what >> the behavior should be: an extra param can be baked into the config, but a prefix on a user query can not. to clarify, the default in the solrconfig could of course be <str name="q"><!lucene> : </str> ... but if the client has to prepend <!dismax> on the front of user input anyway, it can't do it blindly (or it might send an "empty" query of "<!dismax>" and defeat the point of the default) with an extra param, the lcient doesn't even have to know anything special is happening when hte user provides an epty string – it doesn't even have to check to see if hte user has provided an empty string
        Hide
        Hoss Man added a comment -

        whoops ... looks like comment collision ... yeah, i'm thinking altq would be in "solr standard syntax" and if at soem point we add anotations about query syntax it would support them.

        (a q.type param wouldn't really solve this problem ... a default Q of ": in the solorconfig would requre a defualt QTYPE of "standard" or something ... whch means clients clients sending a "q" in the simple dismax syntaxwould have to allways specify q.type=dismax, unless q was blank in which case overriding the q.type would break the default q ... so we're back where we started: the client code needing ot look at the users iput and change something about the request if it's blank)

        Show
        Hoss Man added a comment - whoops ... looks like comment collision ... yeah, i'm thinking altq would be in "solr standard syntax" and if at soem point we add anotations about query syntax it would support them. (a q.type param wouldn't really solve this problem ... a default Q of " : in the solorconfig would requre a defualt QTYPE of "standard" or something ... whch means clients clients sending a "q" in the simple dismax syntaxwould have to allways specify q.type=dismax, unless q was blank in which case overriding the q.type would break the default q ... so we're back where we started: the client code needing ot look at the users iput and change something about the request if it's blank)
        Hide
        Hoss Man added a comment -

        I've committed a variation on Ryan's latest patch that:

        1) changes from the matchall=true syntax to a q.alt=: syntax
        2) adds documentaiton about the new param
        3) demonstrates the new param in the example solrconfig.xml

        Show
        Hoss Man added a comment - I've committed a variation on Ryan's latest patch that: 1) changes from the matchall=true syntax to a q.alt= : syntax 2) adds documentaiton about the new param 3) demonstrates the new param in the example solrconfig.xml
        Hide
        Hoss Man added a comment -

        This bug was modified as part of a bulk update using the criteria...

        • Marked ("Resolved" or "Closed") and "Fixed"
        • Had no "Fix Version" versions
        • Was listed in the CHANGES.txt for 1.2

        The Fix Version for all 39 issues found was set to 1.2, email notification
        was suppressed to prevent excessive email.

        For a list of all the issues modified, search jira comments for this
        (hopefully) unique string: 20080415hossman2

        Show
        Hoss Man added a comment - This bug was modified as part of a bulk update using the criteria... Marked ("Resolved" or "Closed") and "Fixed" Had no "Fix Version" versions Was listed in the CHANGES.txt for 1.2 The Fix Version for all 39 issues found was set to 1.2, email notification was suppressed to prevent excessive email. For a list of all the issues modified, search jira comments for this (hopefully) unique string: 20080415hossman2

          People

          • Assignee:
            Hoss Man
            Reporter:
            Ryan McKinley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development