Lucene - Core
  1. Lucene - Core
  2. LUCENE-1784

Make BooleanWeight and DisjunctionMaxWeight protected

    Details

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

      Description

      Currently, BooleanWeight is private, yet it has 2 protected members (similarity, weights) which are unaccessible from custom code

      i have some use cases where it would be very useful to crawl a BooleanWeight to get at the sub Weight objects

      however, since BooleanWeight is private, i have no way of doing this

      If BooleanWeight is protected, then i can subclass BooleanQuery to hook in and wrap BooleanWeight with a subclass to facilitate this walking of the Weight objects

      Would also want DisjunctionMaxWeight to be protected, along with its "weights" member

      Would be even better if these Weights were made public with accessors to their sub "weights" objects (then no subclassing would be necessary on my part)

      this should be really trivial and would be great if it can get into 2.9

      more generally, it would be nice if all Weight classes were public with nice accessors to relevant "sub weights"/etc so custom code can get its hooks in where and when desired

        Activity

        Mark Thomas made changes -
        Workflow Default workflow, editable Closed status [ 12562918 ] jira [ 12583798 ]
        Mark Thomas made changes -
        Workflow jira [ 12472441 ] Default workflow, editable Closed status [ 12562918 ]
        Mark Miller made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Michael McCandless made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 2.9 [ 12312682 ]
        Resolution Fixed [ 1 ]
        Hide
        Michael McCandless added a comment -

        Thanks Tim!

        Show
        Michael McCandless added a comment - Thanks Tim!
        Michael McCandless made changes -
        Assignee Michael McCandless [ mikemccand ]
        Hide
        Michael McCandless added a comment -

        Thanks Tim! I plan to commit soon...

        Show
        Michael McCandless added a comment - Thanks Tim! I plan to commit soon...
        Tim Smith made changes -
        Attachment LUCENE-1784.patch [ 12416075 ]
        Hide
        Tim Smith added a comment -

        New patch that adds some javadoc marking these as EXPERT and "subject to change"

        Show
        Tim Smith added a comment - New patch that adds some javadoc marking these as EXPERT and "subject to change"
        Tim Smith made changes -
        Attachment LUCENE-1784.patch [ 12415742 ]
        Hide
        Tim Smith added a comment -

        ok, will work up a patch on 2.9 making the classes (and members) protected with javadoc noting the volatility of the classes

        people will only be able to get at these classes if they first override BooleanQuery/DisjunctionMaxQuery, and then in these overridden implementation also override BooleanWeight/DisjunctionMaxWeight (casual user will never do this anyway)

        Show
        Tim Smith added a comment - ok, will work up a patch on 2.9 making the classes (and members) protected with javadoc noting the volatility of the classes people will only be able to get at these classes if they first override BooleanQuery/DisjunctionMaxQuery, and then in these overridden implementation also override BooleanWeight/DisjunctionMaxWeight (casual user will never do this anyway)
        Hide
        Michael McCandless added a comment -

        I like the "make Boolean/DisjunctionMaxWeight protected and note in the javadocs these APIs can suddenly change" approach best... could you work up a patch, but on 2.9 not 2.4?

        Show
        Michael McCandless added a comment - I like the "make Boolean/DisjunctionMaxWeight protected and note in the javadocs these APIs can suddenly change" approach best... could you work up a patch, but on 2.9 not 2.4?
        Hide
        Tim Smith added a comment -

        Here's another approach that may be more desirable:

        add a TraversableWeight interface:

        public interface TraversableWeight extends Weight {
          List/*<Weight>*/ getWeights(); 
        }
        

        then, DisjunctionMaxWeight and BooleanWeight could implement TraversableWeight

        this way, as i traverse the weights, I can check (weight instanceof TraversableWeight) and then traverse the child weights as needed
        i can call weight.getQuery() to see if it was a BooleanQuery or a DisjunctionMaxQuery

        If this is more acceptable, i can work up a patch

        Show
        Tim Smith added a comment - Here's another approach that may be more desirable: add a TraversableWeight interface: public interface TraversableWeight extends Weight { List/*<Weight>*/ getWeights(); } then, DisjunctionMaxWeight and BooleanWeight could implement TraversableWeight this way, as i traverse the weights, I can check (weight instanceof TraversableWeight) and then traverse the child weights as needed i can call weight.getQuery() to see if it was a BooleanQuery or a DisjunctionMaxQuery If this is more acceptable, i can work up a patch
        Hide
        Tim Smith added a comment -

        it could be noted that the class will change in future versions (no back-compat guaranteed)

        i need the ability to be able to walk the weights recursively in order to do some rather nasty post-query analysis

        another alternative would be to add a "getWeights() method to the Weight/QueryWeight interface/class in order to allow generically walking query weights
        this would then allow me to walk the weights as needed. Sadly i cannot walk the queries themselves because what i need is something that isn't resolved until Weight creation time (and this could be very expensive to recreate from the query)

        i guess this idea is somewhat complicated by the fact that Weight is an interface (and you then get compile time back compat issues)

        if the ability to walk the weights is not exposed, i'll just have to hack it in (copy source of these 2 classes and expose them as protected) and i prefer to not do this (much prefer using a pristine lucene distro)

        Show
        Tim Smith added a comment - it could be noted that the class will change in future versions (no back-compat guaranteed) i need the ability to be able to walk the weights recursively in order to do some rather nasty post-query analysis another alternative would be to add a "getWeights() method to the Weight/QueryWeight interface/class in order to allow generically walking query weights this would then allow me to walk the weights as needed. Sadly i cannot walk the queries themselves because what i need is something that isn't resolved until Weight creation time (and this could be very expensive to recreate from the query) i guess this idea is somewhat complicated by the fact that Weight is an interface (and you then get compile time back compat issues) if the ability to walk the weights is not exposed, i'll just have to hack it in (copy source of these 2 classes and expose them as protected) and i prefer to not do this (much prefer using a pristine lucene distro)
        Hide
        Michael McCandless added a comment -

        I'm a little nervous in general about opening up the Weight impls for Lucene's core Queries, simply because in 2.9 (very much fresh on our minds right now) there have been very sizable changes to how Weight works in general, and how certain queries (esp. BooleanQuery) create scorers. Opening up these classes further makes carrying out our back-compat policy even more challenging, and they are already amazingly challenging. In particular, the back-compat implications of external subclasses to Lucene's core classes are the trickiest to get right.

        Show
        Michael McCandless added a comment - I'm a little nervous in general about opening up the Weight impls for Lucene's core Queries, simply because in 2.9 (very much fresh on our minds right now) there have been very sizable changes to how Weight works in general, and how certain queries (esp. BooleanQuery) create scorers. Opening up these classes further makes carrying out our back-compat policy even more challenging, and they are already amazingly challenging. In particular, the back-compat implications of external subclasses to Lucene's core classes are the trickiest to get right.
        Tim Smith made changes -
        Field Original Value New Value
        Attachment LUCENE-1784.patch [ 12415742 ]
        Hide
        Tim Smith added a comment -

        Patch that makes BooleanWeight and DisjunctionMaxWeight protected

        also makes weights on DisjunctionMaxWeight protected

        Show
        Tim Smith added a comment - Patch that makes BooleanWeight and DisjunctionMaxWeight protected also makes weights on DisjunctionMaxWeight protected
        Tim Smith created issue -

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Tim Smith
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development