Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.9
    • Fix Version/s: 2.9
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Since we now have NumericRangeQuery (LUCENE-1701) we should rename RangeQuery to TextRangeQuery to make it clear that TextRangeQuery (TermRangeQuery? StringRangeQuery) is based entirely on text comparison.

      And, existing users on upgrading to 2.9 and using RangeQuery for [slow] numeric searching would realize they now have a good option for numeric range searching.

      1. LUCENE-1713.patch
        140 kB
        Uwe Schindler
      2. LUCENE-1713.patch
        136 kB
        Uwe Schindler
      3. LUCENE-1713.patch
        102 kB
        Uwe Schindler
      4. LUCENE-1713-backwards-branch.patch
        41 kB
        Uwe Schindler
      5. RangeFilter.java
        4 kB
        Uwe Schindler
      6. RangeFilter.java
        4 kB
        Uwe Schindler
      7. RangeQuery.java
        6 kB
        Uwe Schindler
      8. RangeQuery.java
        3 kB
        Uwe Schindler

        Issue Links

          Activity

          Michael McCandless created issue -
          Hide
          Hoss Man added a comment -

          Suggestion: TermRangeQuery to be clear about hte fact that it deals with raw Terms in TermEnum order (and doesn't do any fancy collation or anything else people might expect when dealing with "Strings" or "Text")

          Show
          Hoss Man added a comment - Suggestion: TermRangeQuery to be clear about hte fact that it deals with raw Terms in TermEnum order (and doesn't do any fancy collation or anything else people might expect when dealing with "Strings" or "Text")
          Hide
          Michael McCandless added a comment -

          Suggestion: TermRangeQuery to be clear about hte fact that it deals with raw Terms in TermEnum order (and doesn't do any fancy collation or anything else people might expect when dealing with "Strings" or "Text")

          +1

          Show
          Michael McCandless added a comment - Suggestion: TermRangeQuery to be clear about hte fact that it deals with raw Terms in TermEnum order (and doesn't do any fancy collation or anything else people might expect when dealing with "Strings" or "Text") +1
          Hide
          Uwe Schindler added a comment -

          Additionally rename FieldCacheRangeFilter, RangeFilter.

          Maybe a good addition to FieldCacheRangeFilter would be to do the same currently only possible with StringIndex for NumericFields?

          Show
          Uwe Schindler added a comment - Additionally rename FieldCacheRangeFilter, RangeFilter. Maybe a good addition to FieldCacheRangeFilter would be to do the same currently only possible with StringIndex for NumericFields?
          Hide
          Uwe Schindler added a comment - - edited

          Maybe a good addition to FieldCacheRangeFilter would be to do the same currently only possible with StringIndex for NumericFields?

          see LUCENE-1461

          Show
          Uwe Schindler added a comment - - edited Maybe a good addition to FieldCacheRangeFilter would be to do the same currently only possible with StringIndex for NumericFields? see LUCENE-1461
          Uwe Schindler made changes -
          Field Original Value New Value
          Link This issue relates to LUCENE-1487 [ LUCENE-1487 ]
          Uwe Schindler made changes -
          Link This issue is related to LUCENE-1461 [ LUCENE-1461 ]
          Uwe Schindler made changes -
          Link This issue relates to LUCENE-1487 [ LUCENE-1487 ]
          Hide
          Uwe Schindler added a comment -

          OK, so:

          • RangeFilter -> TermRangeFilter
          • RangeQuery -> TermRangeQuery

          And under LUCENE-1461:

          • FieldCacheRangeFilter -> FieldCacheRangeFilter (on picking up support for numerics...)
          Show
          Uwe Schindler added a comment - OK, so: RangeFilter -> TermRangeFilter RangeQuery -> TermRangeQuery And under LUCENE-1461 : FieldCacheRangeFilter -> FieldCacheRangeFilter (on picking up support for numerics...)
          Uwe Schindler made changes -
          Comment [ OK, so:

            * RangeFilter -> TermRangeFilter
            * RangeQuery -> TermRangeQuery

          And under LUCENE-1487:

            * FieldCacheTermsFilter -> FieldCacheRangeFilter (on picking up support for numerics...) ]
          Uwe Schindler made changes -
          Summary Rename RangeQuery -> TextRangeQuery Rename RangeQuery -> TermRangeQuery
          Hide
          Uwe Schindler added a comment -

          Should I prepare a patch later?

          I would simply rename the files in core and then change the tests and rest to find any references to the old RangeQuery/Filter (compile failures). After that I create a deprecated subclass of TermRange* with the old name and all ctors from the old class. The new class will only supply the new ctors (no deprecated ones using Term, mapping is done in the subclass).

          Show
          Uwe Schindler added a comment - Should I prepare a patch later? I would simply rename the files in core and then change the tests and rest to find any references to the old RangeQuery/Filter (compile failures). After that I create a deprecated subclass of TermRange* with the old name and all ctors from the old class. The new class will only supply the new ctors (no deprecated ones using Term, mapping is done in the subclass).
          Hide
          Simon Willnauer added a comment -

          For an issue like this we should go for "commit without patch" to keep the history of the classes. If you create a patch and apply it without useing svn copy && svn commit you will loose history which is not worth it in this case. Just a hint.

          simon

          Show
          Simon Willnauer added a comment - For an issue like this we should go for "commit without patch" to keep the history of the classes. If you create a patch and apply it without useing svn copy && svn commit you will loose history which is not worth it in this case. Just a hint. simon
          Hide
          Michael McCandless added a comment -

          Yes feel free to take this one Uwe, thanks! That approach (and using svn move/copy) sounds good.

          Show
          Michael McCandless added a comment - Yes feel free to take this one Uwe, thanks! That approach (and using svn move/copy) sounds good.
          Hide
          Uwe Schindler added a comment -

          I know! We had this problem a lot of times.

          The patch will be only as reference. When I do it, I would assign and also commit with svn copy/move and so on.

          Show
          Uwe Schindler added a comment - I know! We had this problem a lot of times. The patch will be only as reference. When I do it, I would assign and also commit with svn copy/move and so on.
          Hide
          Uwe Schindler added a comment -

          Yes feel free to take this one Uwe, thanks! That approach (and using svn move/copy) sounds good.

          I was the "bad-boy" who invented NumericRangeQuery, so I have to take this one and clean up the rest of Lucene for this change.

          Show
          Uwe Schindler added a comment - Yes feel free to take this one Uwe, thanks! That approach (and using svn move/copy) sounds good. I was the "bad-boy" who invented NumericRangeQuery, so I have to take this one and clean up the rest of Lucene for this change.
          Uwe Schindler made changes -
          Assignee Uwe Schindler [ thetaphi ]
          Hide
          Uwe Schindler added a comment -

          Here the patch for reference. The deprecated old classes are attached as separate files for a second commit step. Please note, that it will not apply to your checkout, because it needs a SVN rename of RangeQuery, RangeFilter and RangeTermEnum to Term*.

          This patch also converts all tests and contrib to use the new classes and non-deprecated ctors. In TermRange* all deprecated methods are removed and only stay alive in the old classes. The new TermRangeQuery use constant score mode per default (as planned for 3.0), but the old deprecated classes use boolean rewrite per default. I will edit the tests after this commit, to also test boolean rewrite (because of the new defaults, only constant score rewrite ist tested)

          I will commit this soon (approx 2 hours) in two steps (first the patch, then svn add of the new deprecated classes) to not prevent people from doing things.

          All tests, especially bw-tests pass.

          Show
          Uwe Schindler added a comment - Here the patch for reference. The deprecated old classes are attached as separate files for a second commit step. Please note, that it will not apply to your checkout, because it needs a SVN rename of RangeQuery, RangeFilter and RangeTermEnum to Term*. This patch also converts all tests and contrib to use the new classes and non-deprecated ctors. In TermRange* all deprecated methods are removed and only stay alive in the old classes. The new TermRangeQuery use constant score mode per default (as planned for 3.0), but the old deprecated classes use boolean rewrite per default. I will edit the tests after this commit, to also test boolean rewrite (because of the new defaults, only constant score rewrite ist tested) I will commit this soon (approx 2 hours) in two steps (first the patch, then svn add of the new deprecated classes) to not prevent people from doing things. All tests, especially bw-tests pass.
          Uwe Schindler made changes -
          Attachment LUCENE-1713.patch [ 12412397 ]
          Attachment RangeQuery.java [ 12412398 ]
          Attachment RangeFilter.java [ 12412399 ]
          Uwe Schindler made changes -
          Attachment LUCENE-1713.patch [ 12412397 ]
          Hide
          Uwe Schindler added a comment -

          correct patch.

          Show
          Uwe Schindler added a comment - correct patch.
          Uwe Schindler made changes -
          Attachment LUCENE-1713.patch [ 12412400 ]
          Hide
          Uwe Schindler added a comment -

          After comparing the old 2.4.1 javadocs with the new ones, I found out, that RangeQueries in 2.4.1 only had Term ctors and no (field,String,String) ctors. As the new classes are separated (deprecated one and new one), I will change it to not have this strange getLowerTermText methods in the new class.

          So I will prepare a new patch and do all tests again, no commits for now. The current patch had also some minor problems with BW compatibility and a missing collator.

          Show
          Uwe Schindler added a comment - After comparing the old 2.4.1 javadocs with the new ones, I found out, that RangeQueries in 2.4.1 only had Term ctors and no (field,String,String) ctors. As the new classes are separated (deprecated one and new one), I will change it to not have this strange getLowerTermText methods in the new class. So I will prepare a new patch and do all tests again, no commits for now. The current patch had also some minor problems with BW compatibility and a missing collator.
          Hide
          Uwe Schindler added a comment -

          New patch for reference (before all Range* must be svn renamed to TermRange*):

          • Restores original API of RangeQuery and RangeFilter (the backards-tag must be changed to use the original 2.4 tests, see java-dev-mail from me regarding this)
          • RangeQuery is a deprecated dummy class deleagting everything to TermRangeQuery, but is not a subclass, because different semantics of ctors and some getter methods.
          • new classes use cleaner API (no such strange methods like getLowerTermText() because of overriding)
          Show
          Uwe Schindler added a comment - New patch for reference (before all Range* must be svn renamed to TermRange*): Restores original API of RangeQuery and RangeFilter (the backards-tag must be changed to use the original 2.4 tests, see java-dev-mail from me regarding this) RangeQuery is a deprecated dummy class deleagting everything to TermRangeQuery, but is not a subclass, because different semantics of ctors and some getter methods. new classes use cleaner API (no such strange methods like getLowerTermText() because of overriding)
          Uwe Schindler made changes -
          Attachment LUCENE-1713.patch [ 12412429 ]
          Attachment LUCENE-1713-backwards-branch.patch [ 12412430 ]
          Attachment RangeQuery.java [ 12412431 ]
          Uwe Schindler made changes -
          Attachment RangeFilter.java [ 12412432 ]
          Hide
          Uwe Schindler added a comment -

          After committing this, I will open an issue for Solr, to change the references to the not yet released RangeQuery/Filter ctors taking String args to the new classes.

          Show
          Uwe Schindler added a comment - After committing this, I will open an issue for Solr, to change the references to the not yet released RangeQuery/Filter ctors taking String args to the new classes.
          Hide
          Uwe Schindler added a comment -

          Last update.

          The patch also contains regenerated QueryParser classes (see LUCENE-1646).

          Show
          Uwe Schindler added a comment - Last update. The patch also contains regenerated QueryParser classes (see LUCENE-1646 ).
          Uwe Schindler made changes -
          Attachment LUCENE-1713.patch [ 12412545 ]
          Hide
          Uwe Schindler added a comment -

          Committed revisions 791175 and 791176.

          Show
          Uwe Schindler added a comment - Committed revisions 791175 and 791176.
          Uwe Schindler made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Uwe Schindler made changes -
          Link This issue is related to SOLR-1261 [ SOLR-1261 ]
          Mark Thomas made changes -
          Workflow jira [ 12466605 ] Default workflow, editable Closed status [ 12563139 ]
          Mark Thomas made changes -
          Workflow Default workflow, editable Closed status [ 12563139 ] jira [ 12584221 ]

            People

            • Assignee:
              Uwe Schindler
              Reporter:
              Michael McCandless
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development