Lucene - Core
  1. Lucene - Core
  2. LUCENE-4411

Depth requested in a facetRequest is reset when Sampling is in effect

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.6.1, 4.0, 6.0
    • Fix Version/s: 4.0, 4.1, 3.6.2, 6.0
    • Component/s: modules/facet
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      FacetRequest can be set a Depth parameter, which controls the depth of the result tree to be returned.
      When Sampling is enabled (and actually used) the Depth parameter gets reset to its default (1).

      1. LUCENE-4411.patch
        8 kB
        Gilad Barkai
      2. OversampleWithDepthTest.java
        5 kB
        Gilad Barkai
      3. OversampleWithDepthTest.java
        5 kB
        Gilad Barkai

        Activity

        Hide
        Gilad Barkai added a comment -

        A Test revealing the bug for Trunk

        Show
        Gilad Barkai added a comment - A Test revealing the bug for Trunk
        Hide
        Gilad Barkai added a comment -

        A Test revealing the bug for 3.6

        Show
        Gilad Barkai added a comment - A Test revealing the bug for 3.6
        Hide
        Gilad Barkai added a comment -

        When sampling is under effect, the original FacetRequest is replaces with a wrapper, which takes under account different sampling related parameters named OverSampledFacetRequest.
        This "wrapping" class modifies only a small set of parameters in the request and should otherwise delegate everything to the original one - but it does not.
        Some of the information that is lost from the original request: SortOrder, SortBy, number of results to lable, ResultMode and Depth.

        A patch would be available shortly.

        Show
        Gilad Barkai added a comment - When sampling is under effect, the original FacetRequest is replaces with a wrapper, which takes under account different sampling related parameters named OverSampledFacetRequest. This "wrapping" class modifies only a small set of parameters in the request and should otherwise delegate everything to the original one - but it does not. Some of the information that is lost from the original request: SortOrder, SortBy, number of results to lable, ResultMode and Depth. A patch would be available shortly.
        Hide
        Gilad Barkai added a comment -

        Attached a proposed fix + test.

        Delegating is impossible as FacetRequest's getters are final. The only way to 'delegate' the information is using the setters in the wrapping class (e.g setDepth(original.getDepth()).
        This solution does not seem the right one, but other approaches involves changing much more code and reducing the amount of protection the public API offers (e.g the user will find it easier to break something).

        The patch also introduce (the missing) delegation of the SortBy, SortOrder, numResultsToLable and ResultMode methods.

        Somewhat out of scope of the issue - I tried to figure out why the wrapping and keeping the original request is important:
        The count (number of categories to return) is final, set at construction. While Sampling is in effect, in order to better the chances of 'hitting' the true top-k results, the notion of oversampling is introduced, which asks for more than just K (e.g 3 * K results) - so another request should be made. The 'original' request is saves so the end-result would hold the original request, and not the over-sampled one (every FacetResult has its originating FacetRequest).

        Show
        Gilad Barkai added a comment - Attached a proposed fix + test. Delegating is impossible as FacetRequest 's getters are final . The only way to 'delegate' the information is using the setters in the wrapping class (e.g setDepth(original.getDepth()). This solution does not seem the right one, but other approaches involves changing much more code and reducing the amount of protection the public API offers (e.g the user will find it easier to break something). The patch also introduce (the missing) delegation of the SortBy , SortOrder , numResultsToLable and ResultMode methods. Somewhat out of scope of the issue - I tried to figure out why the wrapping and keeping the original request is important: The count (number of categories to return) is final , set at construction. While Sampling is in effect, in order to better the chances of 'hitting' the true top-k results, the notion of oversampling is introduced, which asks for more than just K (e.g 3 * K results) - so another request should be made. The 'original' request is saves so the end-result would hold the original request, and not the over-sampled one (every FacetResult has its originating FacetRequest ).
        Hide
        Shai Erera added a comment -

        Thanks Gilad ! Patch looks good, I'll commit it shortly. I'll also add a CHANGES entry.

        Show
        Shai Erera added a comment - Thanks Gilad ! Patch looks good, I'll commit it shortly. I'll also add a CHANGES entry.
        Hide
        Shai Erera added a comment -

        Committed to 3.6.2, 4x, and trunk.

        I also committed to lucene_solr_4_0 just in case we'll have another 4.0 candidate.

        Thanks Gilad !

        Show
        Shai Erera added a comment - Committed to 3.6.2, 4x, and trunk. I also committed to lucene_solr_4_0 just in case we'll have another 4.0 candidate. Thanks Gilad !
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Robert Muir
        http://svn.apache.org/viewvc?view=revision&revision=1392460

        LUCENE-4411: fix changes entry (this made it into 4.0)

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Robert Muir http://svn.apache.org/viewvc?view=revision&revision=1392460 LUCENE-4411 : fix changes entry (this made it into 4.0)
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Shai Erera
        http://svn.apache.org/viewvc?view=revision&revision=1389722

        LUCENE-4411: Depth requested in a FacetRequest is reset when Sampling is in effect

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Shai Erera http://svn.apache.org/viewvc?view=revision&revision=1389722 LUCENE-4411 : Depth requested in a FacetRequest is reset when Sampling is in effect
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Shai Erera
            Reporter:
            Gilad Barkai
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development