Solr
  1. Solr
  2. SOLR-3039

ExtendedDismaxQParser should allow for extension of parsing-related behavior

    Details

      Description

      ExtendedDismaxQParser.parse does not currently allow for things like query pre-processing prior to its parsing, specifying the parser to be used, and whether particular clause should be included in the query being parsed. Furthermore, ExtendedDismaxQParser and inner ExtendedSolrQueryParser cannot be subclassed. By resolving this issue, we'll provide a way for Solr implementations to extend the parser and parsing related behavior.

      1. SOLR-3039.patch
        1 kB
        Danny Dvinov
      2. SOLR-3039.patch
        74 kB
        Danny Dvinov
      3. SOLR-3039.patch
        74 kB
        Danny Dvinov
      4. SOLR-3039.patch
        93 kB
        Danny Dvinov

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Closed Closed
          366d 22h 47m 1 Danny Dvinov 15/Jan/13 20:19
          Danny Dvinov made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Fix Version/s 4.1 [ 12321141 ]
          Resolution Duplicate [ 3 ]
          Danny Dvinov made changes -
          Link This issue is duplicated by SOLR-4208 [ SOLR-4208 ]
          Hide
          Danny Dvinov added a comment -

          Thank you, Otis Gospodnetic and Tomás Fernández Löbbe, looked at Tomas' patch, and looks like it pretty much covers everything I was suggesting (and further improves the refactoring!). The only thing we won't be able to do out of the box is decide on whether or not a particular clause should be included/excluded while in splitIntoClauses (includeClause method from this suggested patch), but I suppose this is minor, and we could either override entire splitIntoClauses or just apply a minor patch to accomplish this. Thank you for looking into this!

          Show
          Danny Dvinov added a comment - Thank you, Otis Gospodnetic and Tomás Fernández Löbbe , looked at Tomas' patch, and looks like it pretty much covers everything I was suggesting (and further improves the refactoring!). The only thing we won't be able to do out of the box is decide on whether or not a particular clause should be included/excluded while in splitIntoClauses (includeClause method from this suggested patch), but I suppose this is minor, and we could either override entire splitIntoClauses or just apply a minor patch to accomplish this. Thank you for looking into this!
          Hide
          Otis Gospodnetic added a comment -

          Danny Dvinov maybe you can close after you've confirmed?

          Show
          Otis Gospodnetic added a comment - Danny Dvinov maybe you can close after you've confirmed?
          Hide
          Tomás Fernández Löbbe added a comment -

          Yes, it looks like this patch is based on an older revision, prior to SOLR-4208. Most of the things that are suggested here were implemented and committed.
          I didn't add a "preProecessUserQuery(...)", I thought about this, but I think now it is something easier to extend now (either wrapping the parse method, overriding the "getString()" method, when parsing the configuration, when getting the different clauses, depending on the needs). I didn't thought it was necessary to add an empty hook for this.

          Show
          Tomás Fernández Löbbe added a comment - Yes, it looks like this patch is based on an older revision, prior to SOLR-4208 . Most of the things that are suggested here were implemented and committed. I didn't add a "preProecessUserQuery(...)", I thought about this, but I think now it is something easier to extend now (either wrapping the parse method, overriding the "getString()" method, when parsing the configuration, when getting the different clauses, depending on the needs). I didn't thought it was necessary to add an empty hook for this.
          Hide
          Otis Gospodnetic added a comment -

          Haven't looked at the patch but I suspect it might be in conflict with Tomás Fernández Löbbe's SOLR-4208. Oh, maybe not, SOLR-4208 was committed... assuming Danny based his latest patch on the more recent trunk revision? I'm eyeing this with LUCENE-4499 in mind.

          Show
          Otis Gospodnetic added a comment - Haven't looked at the patch but I suspect it might be in conflict with Tomás Fernández Löbbe 's SOLR-4208 . Oh, maybe not, SOLR-4208 was committed... assuming Danny based his latest patch on the more recent trunk revision? I'm eyeing this with LUCENE-4499 in mind.
          Hide
          Danny Dvinov added a comment -

          Please feel free to make comments/suggestions!

          Show
          Danny Dvinov added a comment - Please feel free to make comments/suggestions!
          Danny Dvinov made changes -
          Comment [ All comments are more than welcome btw! ]
          Danny Dvinov made changes -
          Attachment SOLR-3039.patch [ 12548815 ]
          Hide
          Danny Dvinov added a comment -

          Changes include:

          • Making ExtendedDismaxQParser public to allow implementations that can extend it (and pulling it out into a separate file, ExtendedDismaxQParser.java).
          • Making ExtendedSolrQueryParser protected to allow to allow implementations that can extend it.
          • Introducing 3 protected methods on ExtendedDismaxQParser to allow implementations that extend ExtendedDismaxQParser to override some of the behavior:
            1. ExtendedSolrQueryParser getQueryParser to allow specifying the parser used to parse the query.
            2. boolean includeClause(Clause clause) to allow overriding whether given clause should be part of the {@link org.apache.lucene.search.Query}

            being parsed.
            3. String preProcessUserQuery(String queryString) to allow pre-processing the query string prior to parsing.

          Show
          Danny Dvinov added a comment - Changes include: Making ExtendedDismaxQParser public to allow implementations that can extend it (and pulling it out into a separate file, ExtendedDismaxQParser.java). Making ExtendedSolrQueryParser protected to allow to allow implementations that can extend it. Introducing 3 protected methods on ExtendedDismaxQParser to allow implementations that extend ExtendedDismaxQParser to override some of the behavior: 1. ExtendedSolrQueryParser getQueryParser to allow specifying the parser used to parse the query. 2. boolean includeClause(Clause clause) to allow overriding whether given clause should be part of the {@link org.apache.lucene.search.Query} being parsed. 3. String preProcessUserQuery(String queryString) to allow pre-processing the query string prior to parsing.
          Danny Dvinov made changes -
          Affects Version/s 5.0 [ 12321664 ]
          Summary ExtendedDismaxQParser should allow pre-processing of the user query prior to parsing ExtendedDismaxQParser should allow for extension of parsing-related behavior
          Affects Version/s 4.0-ALPHA [ 12314992 ]
          Description ExtendedDismaxQParser.parse does not currently allow for query pre-processing prior to its parsing routine. Provide a way for Solr implementations to pre-process the query whenever needed. This can be useful when query needs to be adjusted to account for things like expanding wildcarded terms within phrase queries, processing terms to allow multi-token synonym matches (in cases when no index-time synonym filtering is done), and whatever other cases that could benefit from altering the query prior to parsing. ExtendedDismaxQParser.parse does not currently allow for things like query pre-processing prior to its parsing, specifying the parser to be used, and whether particular clause should be included in the query being parsed. Furthermore, ExtendedDismaxQParser and inner ExtendedSolrQueryParser cannot be subclassed. By resolving this issue, we'll provide a way for Solr implementations to extend the parser and parsing related behavior.
          Labels edismax multi-token parser parsing phrase synonyms wildcard edismax parser parsing
          Fix Version/s 5.0 [ 12321664 ]
          Fix Version/s 4.1 [ 12321141 ]
          Component/s query parsers [ 12317802 ]
          Component/s Schema and Analysis [ 12312520 ]
          Robert Muir made changes -
          Fix Version/s 4.1 [ 12321141 ]
          Fix Version/s 4.0 [ 12314992 ]
          Danny Dvinov made changes -
          Summary ExtendedDismaxQParser should allow expanding wildcarded terms within phrase queries ExtendedDismaxQParser should allow pre-processing of the user query prior to parsing
          Description ExtendedDismaxQParser.parse does not currently expand wildcarded terms within phrase queries. Provide a way for Solr implementations to expand wildcarded terms in such cases, while still benefiting from the rest of ExtendedDismaxQParser functionality. ExtendedDismaxQParser.parse does not currently allow for query pre-processing prior to its parsing routine. Provide a way for Solr implementations to pre-process the query whenever needed. This can be useful when query needs to be adjusted to account for things like expanding wildcarded terms within phrase queries, processing terms to allow multi-token synonym matches (in cases when no index-time synonym filtering is done), and whatever other cases that could benefit from altering the query prior to parsing.
          Labels parser phrase wildcard edismax multi-token parser parsing phrase synonyms wildcard
          Danny Dvinov made changes -
          Attachment SOLR-3039.patch [ 12520475 ]
          Hide
          Danny Dvinov added a comment -

          Refactored the previous change to be more generic to allow pre-processing user query prior to parsing for whatever reasons one might have (as opposed to only for expanding wildcarded terms).

          Show
          Danny Dvinov added a comment - Refactored the previous change to be more generic to allow pre-processing user query prior to parsing for whatever reasons one might have (as opposed to only for expanding wildcarded terms).
          Danny Dvinov made changes -
          Attachment SOLR-3039.patch [ 12510607 ]
          Hide
          Danny Dvinov added a comment -

          Made ExtendedDismaxQParser public for implementations that will be extending it.

          Show
          Danny Dvinov added a comment - Made ExtendedDismaxQParser public for implementations that will be extending it.
          Danny Dvinov made changes -
          Field Original Value New Value
          Attachment SOLR-3039.patch [ 12510599 ]
          Danny Dvinov created issue -

            People

            • Assignee:
              Unassigned
              Reporter:
              Danny Dvinov
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 24h
                24h
                Remaining:
                Remaining Estimate - 24h
                24h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development