Re: svn commit: r510334 - in /lucene/solr/trunk/src/java/org/apache/solr/search: QueryParsing.java SolrQueryParser.java

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r510334 - in /lucene/solr/trunk/src/java/org/apache/solr/search: QueryParsing.java SolrQueryParser.java

Erik Hatcher
If you see anything fishy with this, by all means let me know.  This  
was an odd one... my custom request handler calls  
QueryParsing.parseQuery(String,String,IndexSchema) and it confused me  
for a while why our
default AND operator was not working.  I had implemented this  
feature, but for a different application that uses the standard  
request handler which calls QueryParsing.parseQuery
(String,String,SolrParams,IndexSchema).

I believe now having SolrQueryParser set the operator based on the  
schema configuration is much more sensible.

        Erik


On Feb 21, 2007, at 8:00 PM, [hidden email] wrote:

> Author: ehatcher
> Date: Wed Feb 21 17:00:23 2007
> New Revision: 510334
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=510334
> Log:
> QueryParsing.parseQuery(String,IndexSchema) and  
> QueryParsing.parseQuery(String,String,IndexSchema) now
> respects the schema.xml <solrQueryParser defaultOperator="..."/>  
> setting.  Before, only
> QueryParsing.parseQuery(String,String,SolrParams,IndexSchema)  
> respected the schema setting.
>
> SolrQueryParser itself now sets the default operator based on the  
> schema setting, and is overridden
> with the q.op param when the latter QueryParsing.parseQuery()  
> method is used.
>
>
> Modified:
>     lucene/solr/trunk/src/java/org/apache/solr/search/
> QueryParsing.java
>     lucene/solr/trunk/src/java/org/apache/solr/search/
> SolrQueryParser.java
>
> Modified: lucene/solr/trunk/src/java/org/apache/solr/search/
> QueryParsing.java
> URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/ 
> apache/solr/search/QueryParsing.java?
> view=diff&rev=510334&r1=510333&r2=510334
> ======================================================================
> ========
> --- lucene/solr/trunk/src/java/org/apache/solr/search/
> QueryParsing.java (original)
> +++ lucene/solr/trunk/src/java/org/apache/solr/search/
> QueryParsing.java Wed Feb 21 17:00:23 2007
> @@ -85,10 +85,11 @@
>     */
>    public static Query parseQuery(String qs, String defaultField,  
> SolrParams params, IndexSchema schema) {
>      try {
> -      String opParam = params.get(OP,  
> schema.getQueryParserDefaultOperator());
> -      QueryParser.Operator defaultOperator = "AND".equals
> (opParam) ? QueryParser.Operator.AND : QueryParser.Operator.OR;
>        SolrQueryParser parser = new SolrQueryParser(schema,  
> defaultField);
> -      parser.setDefaultOperator(defaultOperator);
> +      String opParam = params.get(OP);
> +      if (opParam != null) {
> +        parser.setDefaultOperator("AND".equals(opParam) ?  
> QueryParser.Operator.AND : QueryParser.Operator.OR);
> +      }
>        Query query = parser.parse(qs);
>
>        if (SolrCore.log.isLoggable(Level.FINEST)) {
>
> Modified: lucene/solr/trunk/src/java/org/apache/solr/search/
> SolrQueryParser.java
> URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/ 
> apache/solr/search/SolrQueryParser.java?
> view=diff&rev=510334&r1=510333&r2=510334
> ======================================================================
> ========
> --- lucene/solr/trunk/src/java/org/apache/solr/search/
> SolrQueryParser.java (original)
> +++ lucene/solr/trunk/src/java/org/apache/solr/search/
> SolrQueryParser.java Wed Feb 21 17:00:23 2007
> @@ -60,6 +60,8 @@
>      super(defaultField == null ? schema.getDefaultSearchFieldName
> () : defaultField, schema.getQueryAnalyzer());
>      this.schema = schema;
>      setLowercaseExpandedTerms(false);
> +    String operator = schema.getQueryParserDefaultOperator();
> +    setDefaultOperator("AND".equals(operator) ?  
> QueryParser.Operator.AND : QueryParser.Operator.OR);
>    }
>
>    protected Query getFieldQuery(String field, String queryText)  
> throws ParseException {
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r510334 - in /lucene/solr/trunk/src/java/org/apache/solr/search: QueryParsing.java SolrQueryParser.java

Chris Hostetter-3

: I believe now having SolrQueryParser set the operator based on the
: schema configuration is much more sensible.

I haven't looked at this in depth, but i *think* this breaks the
DisMaxQueryParser (i'm guessing AND isn't the default in our test config)

i remember discussing this before, and suggesting that SolrQueryParser
should use the schema only for understanding field types, so calling the
basic constructor gives you a barebones SOlrQUeryParser; and any other
options (like default op) could be set in a utility/factory method.

http://www.nabble.com/QueryParser-default-operator---AND-tf1977319.html#a5425365

: On Feb 21, 2007, at 8:00 PM, [hidden email] wrote:
:
: > Author: ehatcher
: > Date: Wed Feb 21 17:00:23 2007
: > New Revision: 510334
: >
: > URL: http://svn.apache.org/viewvc?view=rev&rev=510334
: > Log:
: > QueryParsing.parseQuery(String,IndexSchema) and
: > QueryParsing.parseQuery(String,String,IndexSchema) now
: > respects the schema.xml <solrQueryParser defaultOperator="..."/>
: > setting.  Before, only
: > QueryParsing.parseQuery(String,String,SolrParams,IndexSchema)
: > respected the schema setting.
: >
: > SolrQueryParser itself now sets the default operator based on the
: > schema setting, and is overridden
: > with the q.op param when the latter QueryParsing.parseQuery()
: > method is used.
: >
: >
: > Modified:
: >     lucene/solr/trunk/src/java/org/apache/solr/search/
: > QueryParsing.java
: >     lucene/solr/trunk/src/java/org/apache/solr/search/
: > SolrQueryParser.java
: >
: > Modified: lucene/solr/trunk/src/java/org/apache/solr/search/
: > QueryParsing.java
: > URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/
: > apache/solr/search/QueryParsing.java?
: > view=diff&rev=510334&r1=510333&r2=510334
: > ======================================================================
: > ========
: > --- lucene/solr/trunk/src/java/org/apache/solr/search/
: > QueryParsing.java (original)
: > +++ lucene/solr/trunk/src/java/org/apache/solr/search/
: > QueryParsing.java Wed Feb 21 17:00:23 2007
: > @@ -85,10 +85,11 @@
: >     */
: >    public static Query parseQuery(String qs, String defaultField,
: > SolrParams params, IndexSchema schema) {
: >      try {
: > -      String opParam = params.get(OP,
: > schema.getQueryParserDefaultOperator());
: > -      QueryParser.Operator defaultOperator = "AND".equals
: > (opParam) ? QueryParser.Operator.AND : QueryParser.Operator.OR;
: >        SolrQueryParser parser = new SolrQueryParser(schema,
: > defaultField);
: > -      parser.setDefaultOperator(defaultOperator);
: > +      String opParam = params.get(OP);
: > +      if (opParam != null) {
: > +        parser.setDefaultOperator("AND".equals(opParam) ?
: > QueryParser.Operator.AND : QueryParser.Operator.OR);
: > +      }
: >        Query query = parser.parse(qs);
: >
: >        if (SolrCore.log.isLoggable(Level.FINEST)) {
: >
: > Modified: lucene/solr/trunk/src/java/org/apache/solr/search/
: > SolrQueryParser.java
: > URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/
: > apache/solr/search/SolrQueryParser.java?
: > view=diff&rev=510334&r1=510333&r2=510334
: > ======================================================================
: > ========
: > --- lucene/solr/trunk/src/java/org/apache/solr/search/
: > SolrQueryParser.java (original)
: > +++ lucene/solr/trunk/src/java/org/apache/solr/search/
: > SolrQueryParser.java Wed Feb 21 17:00:23 2007
: > @@ -60,6 +60,8 @@
: >      super(defaultField == null ? schema.getDefaultSearchFieldName
: > () : defaultField, schema.getQueryAnalyzer());
: >      this.schema = schema;
: >      setLowercaseExpandedTerms(false);
: > +    String operator = schema.getQueryParserDefaultOperator();
: > +    setDefaultOperator("AND".equals(operator) ?
: > QueryParser.Operator.AND : QueryParser.Operator.OR);
: >    }
: >
: >    protected Query getFieldQuery(String field, String queryText)
: > throws ParseException {
: >
:



-Hoss

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r510334 - in /lucene/solr/trunk/src/java/org/apache/solr/search: QueryParsing.java SolrQueryParser.java

Erik Hatcher

On Feb 22, 2007, at 1:54 AM, Chris Hostetter wrote:
> : I believe now having SolrQueryParser set the operator based on the
> : schema configuration is much more sensible.
>
> I haven't looked at this in depth, but i *think* this breaks the
> DisMaxQueryParser (i'm guessing AND isn't the default in our test  
> config)

Does DisjunctionMaxQueryParser require the operator to be AND?   Does  
it always need to be set one way or the other?  If so, we could just  
override that setting in the constructor, eh?

> i remember discussing this before, and suggesting that SolrQueryParser
> should use the schema only for understanding field types, so  
> calling the
> basic constructor gives you a barebones SOlrQUeryParser; and any other
> options (like default op) could be set in a utility/factory method.
>
> http://www.nabble.com/QueryParser-default-operator---AND- 
> tf1977319.html#a5425365

Sure, the QueryParsing.parseQuery methods could set it, and I'll  
change it if needed, but I want to understand how this breaks  
DisjunctionMaxQueryParser first.  Can q.op be set specifically for  
the DisjunctionMaxQueryParser?

Thanks,
        Erik

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r510334 - in /lucene/solr/trunk/src/java/org/apache/solr/search: QueryParsing.java SolrQueryParser.java

Chris Hostetter-3

: Does DisjunctionMaxQueryParser require the operator to be AND?   Does
: it always need to be set one way or the other?  If so, we could just
: override that setting in the constructor, eh?

DisjunctionMaxQueryParser assumes that calling super(schema,defaultField)
will set the schema (for the purposes of field parsing), and the
defaultField, and have no other side affects.

: Sure, the QueryParsing.parseQuery methods could set it, and I'll
: change it if needed, but I want to understand how this breaks
: DisjunctionMaxQueryParser first.  Can q.op be set specifically for
: the DisjunctionMaxQueryParser?

DisjunctionMaxQueryParser uses "mm" to determine how many clauses
should match (so if you want all to be mandatory you set mm=100%) so it
doesn't use q.op or '<solrQueryParser defaultOperator="AND"/>'

with the change you made, someone setting <solrQueryParser
defaultOperator="AND"/> causes DisjunctionMaxQueryParser to treat every
clause as mandatory (ignoringthe mm)

we could make the constructor for DisjunctionMaxQueryParser explicitly
call setDefaultOperator(QueryParser.Operator.OR) ... that's probably wise
anyway, but in general I worry about putting "side effects" in the
constructor for SolrQueryParser that users may not no they need to "unset"
when constructing an instance.  (particulararly if/when we start adding
more options in the <solrQueryParser/> block)


What do you think of this patch (to the current trunk) ? ....


Index: src/java/org/apache/solr/schema/IndexSchema.java
===================================================================
--- src/java/org/apache/solr/schema/IndexSchema.java (revision 510622)
+++ src/java/org/apache/solr/schema/IndexSchema.java (working copy)
@@ -23,11 +23,13 @@
 import org.apache.lucene.document.Fieldable;
 import org.apache.lucene.search.DefaultSimilarity;
 import org.apache.lucene.search.Similarity;
+import org.apache.lucene.queryParser.QueryParser;
 import org.apache.solr.core.SolrException;
 import org.apache.solr.core.Config;
 import org.apache.solr.analysis.TokenFilterFactory;
 import org.apache.solr.analysis.TokenizerChain;
 import org.apache.solr.analysis.TokenizerFactory;
+import org.apache.solr.search.SolrQueryParser;
 import org.apache.solr.util.DOMUtil;
 import org.w3c.dom.Document;
 import org.w3c.dom.NamedNodeMap;
@@ -149,12 +151,33 @@
   private String defaultSearchFieldName=null;
   private String queryParserDefaultOperator = "OR";

-  /** Name of the default search field specified in the schema file */
+  /**
+   * A SolrQueryParser linked to this IndexSchema for field datatype
+   * information, and populated with default options from the
+   * &lt;solrQueryParser&gt; configuration for this IndexSchema.
+   *
+   * @param defaultField if non-null overrides the schema default
+   */
+  public SolrQueryParser getSolrQueryParser(String defaultField) {
+    SolrQueryParser qp = new SolrQueryParser(this,defaultField);
+    String operator = getQueryParserDefaultOperator();
+    qp.setDefaultOperator("AND".equals(operator) ?
+                          QueryParser.Operator.AND : QueryParser.Operator.OR);
+    return qp;
+  }
+
+  /**
+   * Name of the default search field specified in the schema file
+   * @deprecated use getSolrQueryParser().getField()
+   */
   public String getDefaultSearchFieldName() {
     return defaultSearchFieldName;
   }

-  /** default operator ("AND" or "OR") for QueryParser */
+  /**
+   * default operator ("AND" or "OR") for QueryParser
+   * @deprecated use getSolrQueryParser().getDefaultOperator()
+   */
   public String getQueryParserDefaultOperator() {
     return queryParserDefaultOperator;
   }
Index: src/java/org/apache/solr/search/SolrQueryParser.java
===================================================================
--- src/java/org/apache/solr/search/SolrQueryParser.java (revision 510622)
+++ src/java/org/apache/solr/search/SolrQueryParser.java (working copy)
@@ -52,16 +52,19 @@
   protected final IndexSchema schema;

   /**
-   *
+   * Constructs a SolrQueryParser using the schema to understand the
+   * formats and datatypes of each field.  Only the defaultSearchField
+   * will be used from the IndexSchema (unless overridden),
+   * &lt;solrQueryParser&gt; will not be used.
+   *
    * @param schema Used for default search field name if defaultField is null and field information is used for analysis
    * @param defaultField default field used for unspecified search terms.  if null, the schema default field is used
+   * @see IndexSchema.getSolrQueryParser
    */
   public SolrQueryParser(IndexSchema schema, String defaultField) {
     super(defaultField == null ? schema.getDefaultSearchFieldName() : defaultField, schema.getQueryAnalyzer());
     this.schema = schema;
     setLowercaseExpandedTerms(false);
-    String operator = schema.getQueryParserDefaultOperator();
-    setDefaultOperator("AND".equals(operator) ? QueryParser.Operator.AND : QueryParser.Operator.OR);
   }

   protected Query getFieldQuery(String field, String queryText) throws ParseException {
Index: src/java/org/apache/solr/search/QueryParsing.java
===================================================================
--- src/java/org/apache/solr/search/QueryParsing.java (revision 510622)
+++ src/java/org/apache/solr/search/QueryParsing.java (working copy)
@@ -62,7 +62,7 @@
    */
   public static Query parseQuery(String qs, String defaultField, IndexSchema schema) {
     try {
-      Query query = new SolrQueryParser(schema, defaultField).parse(qs);
+      Query query = schema.getSolrQueryParser(defaultField).parse(qs);

       if (SolrCore.log.isLoggable(Level.FINEST)) {
         SolrCore.log.finest("After QueryParser:" + query);
@@ -85,7 +85,7 @@
    */
   public static Query parseQuery(String qs, String defaultField, SolrParams params, IndexSchema schema) {
     try {
-      SolrQueryParser parser = new SolrQueryParser(schema, defaultField);
+      SolrQueryParser parser = schema.getSolrQueryParser(defaultField);
       String opParam = params.get(OP);
       if (opParam != null) {
         parser.setDefaultOperator("AND".equals(opParam) ? QueryParser.Operator.AND : QueryParser.Operator.OR);
Index: src/java/org/apache/solr/request/SimpleFacets.java
===================================================================
--- src/java/org/apache/solr/request/SimpleFacets.java (revision 510622)
+++ src/java/org/apache/solr/request/SimpleFacets.java (working copy)
@@ -106,7 +106,7 @@
      * If user doesn't want schema default for facet.query, they should be
      * explicit.
      */
-    SolrQueryParser qp = new SolrQueryParser(searcher.getSchema(),null);
+    SolrQueryParser qp = searcher.getSchema().getSolrQueryParser(null);

     String[] facetQs = params.getParams(SolrParams.FACET_QUERY);
     if (null != facetQs && 0 != facetQs.length) {
Index: src/java/org/apache/solr/request/DisMaxRequestHandler.java
===================================================================
--- src/java/org/apache/solr/request/DisMaxRequestHandler.java (revision 510622)
+++ src/java/org/apache/solr/request/DisMaxRequestHandler.java (working copy)
@@ -182,7 +182,7 @@
       int qslop = params.getInt(DMP.QS, 0);

       /* a generic parser for parsing regular lucene queries */
-      QueryParser p = new SolrQueryParser(schema, null);
+      QueryParser p = schema.getSolrQueryParser(null);

       /* a parser for dealing with user input, which will convert
        * things to DisjunctionMaxQueries
Index: src/java/org/apache/solr/util/SolrPluginUtils.java
===================================================================
--- src/java/org/apache/solr/util/SolrPluginUtils.java (revision 510622)
+++ src/java/org/apache/solr/util/SolrPluginUtils.java (working copy)
@@ -700,6 +700,8 @@

     public DisjunctionMaxQueryParser(IndexSchema s, String defaultField) {
       super(s,defaultField);
+      // don't trust that our parent class won't ever change it's default
+      setDefaultOperator(QueryParser.Operator.OR);
     }
     public DisjunctionMaxQueryParser(IndexSchema s) {
       this(s,null);
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r510334 - in /lucene/solr/trunk/src/java/org/apache/solr/search: QueryParsing.java SolrQueryParser.java

Erik Hatcher

On Feb 22, 2007, at 1:58 PM, Chris Hostetter wrote:
> : Does DisjunctionMaxQueryParser require the operator to be AND?    
> Does
> : it always need to be set one way or the other?  If so, we could just
> : override that setting in the constructor, eh?
>
> DisjunctionMaxQueryParser assumes that calling super
> (schema,defaultField)
> will set the schema (for the purposes of field parsing), and the
> defaultField, and have no other side affects.

And the default is OR, so we could explicitly set OR in the DMQP  
constructor and rest assured there was no side-effect.  Cool?

> with the change you made, someone setting <solrQueryParser
> defaultOperator="AND"/> causes DisjunctionMaxQueryParser to treat  
> every
> clause as mandatory (ignoringthe mm)

Gotcha.  That's clear.  So the above remedy will be ok, I think.

> we could make the constructor for DisjunctionMaxQueryParser explicitly
> call setDefaultOperator(QueryParser.Operator.OR) ... that's  
> probably wise
> anyway, but in general I worry about putting "side effects" in the
> constructor for SolrQueryParser that users may not no they need to  
> "unset"
> when constructing an instance.  (particulararly if/when we start  
> adding
> more options in the <solrQueryParser/> block)

Let's not overplan for the future... we already are aware that we'll  
want to have pluggable query parsers at the request parameter level,  
and have these switches configurable there.

>
> What do you think of this patch (to the current trunk) ? ....

Yeah, that looks good.  A fair bit more tinkering than I envisioned,  
though it looks great.

        Erik


>
>
> Index: src/java/org/apache/solr/schema/IndexSchema.java
> ===================================================================
> --- src/java/org/apache/solr/schema/IndexSchema.java (revision 510622)
> +++ src/java/org/apache/solr/schema/IndexSchema.java (working copy)
> @@ -23,11 +23,13 @@
>  import org.apache.lucene.document.Fieldable;
>  import org.apache.lucene.search.DefaultSimilarity;
>  import org.apache.lucene.search.Similarity;
> +import org.apache.lucene.queryParser.QueryParser;
>  import org.apache.solr.core.SolrException;
>  import org.apache.solr.core.Config;
>  import org.apache.solr.analysis.TokenFilterFactory;
>  import org.apache.solr.analysis.TokenizerChain;
>  import org.apache.solr.analysis.TokenizerFactory;
> +import org.apache.solr.search.SolrQueryParser;
>  import org.apache.solr.util.DOMUtil;
>  import org.w3c.dom.Document;
>  import org.w3c.dom.NamedNodeMap;
> @@ -149,12 +151,33 @@
>    private String defaultSearchFieldName=null;
>    private String queryParserDefaultOperator = "OR";
>
> -  /** Name of the default search field specified in the schema  
> file */
> +  /**
> +   * A SolrQueryParser linked to this IndexSchema for field datatype
> +   * information, and populated with default options from the
> +   * &lt;solrQueryParser&gt; configuration for this IndexSchema.
> +   *
> +   * @param defaultField if non-null overrides the schema default
> +   */
> +  public SolrQueryParser getSolrQueryParser(String defaultField) {
> +    SolrQueryParser qp = new SolrQueryParser(this,defaultField);
> +    String operator = getQueryParserDefaultOperator();
> +    qp.setDefaultOperator("AND".equals(operator) ?
> +                          QueryParser.Operator.AND :  
> QueryParser.Operator.OR);
> +    return qp;
> +  }
> +
> +  /**
> +   * Name of the default search field specified in the schema file
> +   * @deprecated use getSolrQueryParser().getField()
> +   */
>    public String getDefaultSearchFieldName() {
>      return defaultSearchFieldName;
>    }
>
> -  /** default operator ("AND" or "OR") for QueryParser */
> +  /**
> +   * default operator ("AND" or "OR") for QueryParser
> +   * @deprecated use getSolrQueryParser().getDefaultOperator()
> +   */
>    public String getQueryParserDefaultOperator() {
>      return queryParserDefaultOperator;
>    }
> Index: src/java/org/apache/solr/search/SolrQueryParser.java
> ===================================================================
> --- src/java/org/apache/solr/search/SolrQueryParser.java (revision  
> 510622)
> +++ src/java/org/apache/solr/search/SolrQueryParser.java (working  
> copy)
> @@ -52,16 +52,19 @@
>    protected final IndexSchema schema;
>
>    /**
> -   *
> +   * Constructs a SolrQueryParser using the schema to understand the
> +   * formats and datatypes of each field.  Only the  
> defaultSearchField
> +   * will be used from the IndexSchema (unless overridden),
> +   * &lt;solrQueryParser&gt; will not be used.
> +   *
>     * @param schema Used for default search field name if  
> defaultField is null and field information is used for analysis
>     * @param defaultField default field used for unspecified search  
> terms.  if null, the schema default field is used
> +   * @see IndexSchema.getSolrQueryParser
>     */
>    public SolrQueryParser(IndexSchema schema, String defaultField) {
>      super(defaultField == null ? schema.getDefaultSearchFieldName
> () : defaultField, schema.getQueryAnalyzer());
>      this.schema = schema;
>      setLowercaseExpandedTerms(false);
> -    String operator = schema.getQueryParserDefaultOperator();
> -    setDefaultOperator("AND".equals(operator) ?  
> QueryParser.Operator.AND : QueryParser.Operator.OR);
>    }
>
>    protected Query getFieldQuery(String field, String queryText)  
> throws ParseException {
> Index: src/java/org/apache/solr/search/QueryParsing.java
> ===================================================================
> --- src/java/org/apache/solr/search/QueryParsing.java (revision  
> 510622)
> +++ src/java/org/apache/solr/search/QueryParsing.java (working copy)
> @@ -62,7 +62,7 @@
>     */
>    public static Query parseQuery(String qs, String defaultField,  
> IndexSchema schema) {
>      try {
> -      Query query = new SolrQueryParser(schema, defaultField).parse
> (qs);
> +      Query query = schema.getSolrQueryParser(defaultField).parse
> (qs);
>
>        if (SolrCore.log.isLoggable(Level.FINEST)) {
>          SolrCore.log.finest("After QueryParser:" + query);
> @@ -85,7 +85,7 @@
>     */
>    public static Query parseQuery(String qs, String defaultField,  
> SolrParams params, IndexSchema schema) {
>      try {
> -      SolrQueryParser parser = new SolrQueryParser(schema,  
> defaultField);
> +      SolrQueryParser parser = schema.getSolrQueryParser
> (defaultField);
>        String opParam = params.get(OP);
>        if (opParam != null) {
>          parser.setDefaultOperator("AND".equals(opParam) ?  
> QueryParser.Operator.AND : QueryParser.Operator.OR);
> Index: src/java/org/apache/solr/request/SimpleFacets.java
> ===================================================================
> --- src/java/org/apache/solr/request/SimpleFacets.java (revision  
> 510622)
> +++ src/java/org/apache/solr/request/SimpleFacets.java (working copy)
> @@ -106,7 +106,7 @@
>       * If user doesn't want schema default for facet.query, they  
> should be
>       * explicit.
>       */
> -    SolrQueryParser qp = new SolrQueryParser(searcher.getSchema
> (),null);
> +    SolrQueryParser qp = searcher.getSchema().getSolrQueryParser
> (null);
>
>      String[] facetQs = params.getParams(SolrParams.FACET_QUERY);
>      if (null != facetQs && 0 != facetQs.length) {
> Index: src/java/org/apache/solr/request/DisMaxRequestHandler.java
> ===================================================================
> --- src/java/org/apache/solr/request/DisMaxRequestHandler.java
> (revision 510622)
> +++ src/java/org/apache/solr/request/DisMaxRequestHandler.java
> (working copy)
> @@ -182,7 +182,7 @@
>        int qslop = params.getInt(DMP.QS, 0);
>
>        /* a generic parser for parsing regular lucene queries */
> -      QueryParser p = new SolrQueryParser(schema, null);
> +      QueryParser p = schema.getSolrQueryParser(null);
>
>        /* a parser for dealing with user input, which will convert
>         * things to DisjunctionMaxQueries
> Index: src/java/org/apache/solr/util/SolrPluginUtils.java
> ===================================================================
> --- src/java/org/apache/solr/util/SolrPluginUtils.java (revision  
> 510622)
> +++ src/java/org/apache/solr/util/SolrPluginUtils.java (working copy)
> @@ -700,6 +700,8 @@
>
>      public DisjunctionMaxQueryParser(IndexSchema s, String  
> defaultField) {
>        super(s,defaultField);
> +      // don't trust that our parent class won't ever change it's  
> default
> +      setDefaultOperator(QueryParser.Operator.OR);
>      }
>      public DisjunctionMaxQueryParser(IndexSchema s) {
>        this(s,null);

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r510334 - in /lucene/solr/trunk/src/java/org/apache/solr/search: QueryParsing.java SolrQueryParser.java

Chris Hostetter-3

: And the default is OR, so we could explicitly set OR in the DMQP
: constructor and rest assured there was no side-effect.  Cool?

For DMQP yes -- not for any other plugin writers who have been using
SolrQueryParser up to now.

: Let's not overplan for the future... we already are aware that we'll
: want to have pluggable query parsers at the request parameter level,
: and have these switches configurable there.

i agree overplaning is bad ... i just want to ensure any current users of
"new SolrQueryParser" aren't suprised to find the behavior changing out
from under them -- any future changes for more pluggable query parsers
will probably affect the existing RequestHandlers -- but the role of
SolrQueryParser as a simple subclass of Lucene's QueryParser that knows
about field types should generally remain true for a long time.

: Yeah, that looks good.  A fair bit more tinkering than I envisioned,
: though it looks great.

I'll go ahead and commit (the really nice thing is that it makes more
schema configured query parser options easier to implement because they
won't require a log of public accessors on the IndexSchema).

-Hoss

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r510334 - in /lucene/solr/trunk/src/java/org/apache/solr/search: QueryParsing.java SolrQueryParser.java

Erik Hatcher

On Feb 22, 2007, at 4:50 PM, Chris Hostetter wrote:

> i agree overplaning is bad ... i just want to ensure any current  
> users of
> "new SolrQueryParser" aren't suprised to find the behavior changing  
> out
> from under them -- any future changes for more pluggable query parsers
> will probably affect the existing RequestHandlers -- but the role of
> SolrQueryParser as a simple subclass of Lucene's QueryParser that  
> knows
> about field types should generally remain true for a long time.

And the way a default of AND and the hype of DisMax queries  
(something I intend to spend some time with ASAP, just haven't yet)  
it should be a moot point.  Setting it to AND would imply you're  
likely using the standard request handler.

>
> : Yeah, that looks good.  A fair bit more tinkering than I envisioned,
> : though it looks great.
>
> I'll go ahead and commit (the really nice thing is that it makes more
> schema configured query parser options easier to implement because  
> they
> won't require a log of public accessors on the IndexSchema).

Nice.   Thanks for the quick review and more complete fix.

        Erik