Lucene - Core
  1. Lucene - Core
  2. LUCENE-2757

Refactor RewriteMethods out of MultiTermQuery

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.1, 4.0-ALPHA
    • Fix Version/s: 3.1, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Policeman work - as usual

      1. LUCENE-2757.patch
        96 kB
        Uwe Schindler
      2. LUCENE-2757-branch_3x.patch
        77 kB
        Uwe Schindler
      3. LUCENE-2757-cleanup.patch
        4 kB
        Uwe Schindler
      4. LUCENE-2757-refactor1.patch
        35 kB
        Uwe Schindler
      5. LUCENE-2757-refactor1.patch
        34 kB
        Uwe Schindler

        Issue Links

          Activity

          Hide
          Uwe Schindler added a comment -

          Here is the current state of my work. It contains the spans patch in modified form. This first step changes the class structure to have more abstract classes and generics. At the end I will move the abstract classes and their implementations to @lucene.internal classes.

          MTQ will still have all rewrite modes as final constants (except span), but implementations will go away.

          For backwards compatibility i would keep empty subclasses of the 3.0 classes that were public (like the autorewrite) like the empty TermAttributeImpl skeleton.

          Show
          Uwe Schindler added a comment - Here is the current state of my work. It contains the spans patch in modified form. This first step changes the class structure to have more abstract classes and generics. At the end I will move the abstract classes and their implementations to @lucene.internal classes. MTQ will still have all rewrite modes as final constants (except span), but implementations will go away. For backwards compatibility i would keep empty subclasses of the 3.0 classes that were public (like the autorewrite) like the empty TermAttributeImpl skeleton.
          Hide
          Uwe Schindler added a comment -

          I will go to bed now, here more fixes (BQ max clause count in ParallelArraysCollector, wrong superclass for TopTermsRewrite).

          Will work on factoring out rewrite modes tomorrow!

          Show
          Uwe Schindler added a comment - I will go to bed now, here more fixes (BQ max clause count in ParallelArraysCollector, wrong superclass for TopTermsRewrite). Will work on factoring out rewrite modes tomorrow!
          Hide
          Uwe Schindler added a comment -

          Here the complete heavy refactoring. All tests in core and contrib pass, javadocs build fine:

          • Some trivial implementations of rewrite methods stay in MTQ itsself
          • All rewrite constants stay in MTQ
          • Span Rewrites are in the span wrapper class only. To be type safe, the span rewrites have a super class and the wrapper class only accepts this superclass as rewrite method. The SpanRewriteMethod also uses covariant return types to be type-safe.
          • SpanRewrites and wrapper class got missing equals/hashCode
          Show
          Uwe Schindler added a comment - Here the complete heavy refactoring. All tests in core and contrib pass, javadocs build fine: Some trivial implementations of rewrite methods stay in MTQ itsself All rewrite constants stay in MTQ Span Rewrites are in the span wrapper class only. To be type safe, the span rewrites have a super class and the wrapper class only accepts this superclass as rewrite method. The SpanRewriteMethod also uses covariant return types to be type-safe. SpanRewrites and wrapper class got missing equals/hashCode
          Hide
          Uwe Schindler added a comment -

          Backporting will be funny, but doable. Because lots of code is now separated from the enum handling, we only have to really backport the two methods TopTermsRewrite and ScoringRewrite and their superclass. Spans and contrib stays unchanged and can be simply merged.

          Show
          Uwe Schindler added a comment - Backporting will be funny, but doable. Because lots of code is now separated from the enum handling, we only have to really backport the two methods TopTermsRewrite and ScoringRewrite and their superclass. Spans and contrib stays unchanged and can be simply merged.
          Hide
          Robert Muir added a comment -

          +1, this is really needed... its difficult to navigate MultiTermQuery and this will make a huge difference.

          Show
          Robert Muir added a comment - +1, this is really needed... its difficult to navigate MultiTermQuery and this will make a huge difference.
          Hide
          Uwe Schindler added a comment -

          I will commit this soon and then do backporting!

          Show
          Uwe Schindler added a comment - I will commit this soon and then do backporting!
          Hide
          Uwe Schindler added a comment -

          Committed trunk revision: 1035096

          Now backporting...

          Show
          Uwe Schindler added a comment - Committed trunk revision: 1035096 Now backporting...
          Hide
          Uwe Schindler added a comment - - edited

          Here the merge patch for 3.x

          The merge had a difficulty:

          • MTQ has no fixed field name (as in 3.x MTQ can even rewrite to a query with terms from different fields). So SpanQuery.getField() cannot simply be delegated in the wrapper. A hack was added that uses reflection to find the field name from the wrapped MTQ.

          The other parts were straightforward:

          • merge patch, for MTQ conflict choose "overwrite with their's", keep a backup of old MTQ
          • copypaste all code from old MTQ rewrite modes to the new separate file with rewrite modes.
          • remove docFreq parameter from collect() method.
          Show
          Uwe Schindler added a comment - - edited Here the merge patch for 3.x The merge had a difficulty: MTQ has no fixed field name (as in 3.x MTQ can even rewrite to a query with terms from different fields). So SpanQuery.getField() cannot simply be delegated in the wrapper. A hack was added that uses reflection to find the field name from the wrapped MTQ. The other parts were straightforward: merge patch, for MTQ conflict choose "overwrite with their's", keep a backup of old MTQ copypaste all code from old MTQ rewrite modes to the new separate file with rewrite modes. remove docFreq parameter from collect() method.
          Hide
          Uwe Schindler added a comment -

          Committed 3.x revision: 1035118

          Show
          Uwe Schindler added a comment - Committed 3.x revision: 1035118
          Hide
          Uwe Schindler added a comment -

          Here is a fix for a "nocommit" in trunk's code. I did not mark it as nocommit, but it was one. Now the hacky instanceof check if boolean max clauses must be checked was removed and the whole thing done like in the TopTermRewrite.

          There is also an optimization, that the max clause count is only checked, when an entry is really added to the BytesRefHash.

          Will commit now.

          Show
          Uwe Schindler added a comment - Here is a fix for a "nocommit" in trunk's code. I did not mark it as nocommit, but it was one. Now the hacky instanceof check if boolean max clauses must be checked was removed and the whole thing done like in the TopTermRewrite. There is also an optimization, that the max clause count is only checked, when an entry is really added to the BytesRefHash. Will commit now.
          Hide
          Uwe Schindler added a comment -

          Committed cleanup in trunk revision: 1035173

          Show
          Uwe Schindler added a comment - Committed cleanup in trunk revision: 1035173
          Hide
          Grant Ingersoll added a comment -

          Bulk close for 3.1

          Show
          Grant Ingersoll added a comment - Bulk close for 3.1

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development