Lucene - Core
  1. Lucene - Core
  2. LUCENE-4882

FacetsAccumulator.java:185 throws NullPointerException if it's given an empty CategoryPath.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 4.2
    • Fix Version/s: 4.3, 5.0
    • Component/s: modules/facet
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      When I wanted to count root categories, I used to pass "new CategoryPath(new String[0])" to a CountFacetRequest.

      Since upgrading lucene from 4.1 to 4.2, that threw ArrayIndexOfOutBoundsException, so I passed CategoryPath.EMPTY to a CountFacetRequest instead, and this time I got NullPointerException.

      It all originates from FacetsAccumulator.java:185

      Does someone conspire to prevent others from counting root categories?

        Issue Links

          Activity

          Hide
          Shai Erera added a comment -

          Does someone conspire to prevent others from counting root categories?

          Hehe, no, no conspiracy. I'll look into it!

          Show
          Shai Erera added a comment - Does someone conspire to prevent others from counting root categories? Hehe, no, no conspiracy. I'll look into it!
          Hide
          Shai Erera added a comment -

          Patch adds a test and fix. I'll commit later.

          Show
          Shai Erera added a comment - Patch adds a test and fix. I'll commit later.
          Hide
          crocket added a comment -

          Thanks for a quick response, man.

          Show
          crocket added a comment - Thanks for a quick response, man.
          Hide
          Shai Erera added a comment -

          Committed a fix to trunk and 4x. Thanks for reporting crocket!

          If you cannot wait until 4.3 (and cannot work with 4x directly), you can use StandardFacetsAccumulator as an alternative, but note that it's slower than FacetsAccumulator. Or, create your own FacetsAccumulator and copy accumulate + fix.

          Show
          Shai Erera added a comment - Committed a fix to trunk and 4x. Thanks for reporting crocket! If you cannot wait until 4.3 (and cannot work with 4x directly), you can use StandardFacetsAccumulator as an alternative, but note that it's slower than FacetsAccumulator. Or, create your own FacetsAccumulator and copy accumulate + fix.
          Hide
          crocket added a comment -

          What about 4.2.1?

          And when will 4.3 be released?

          Show
          crocket added a comment - What about 4.2.1? And when will 4.3 be released?
          Hide
          Shai Erera added a comment -

          Unfortunately it won't make it into 4.2.1, and it's likely that 4.3 will be released before 4.2.2 (though it will take some time since we just cut 4.2).

          Show
          Shai Erera added a comment - Unfortunately it won't make it into 4.2.1, and it's likely that 4.3 will be released before 4.2.2 (though it will take some time since we just cut 4.2).
          Hide
          crocket added a comment - - edited

          I decided to stick to 4.2 for a while.

          Should I override methods in both FacetsAccumulator.java and CountingListBuilder.java to make it work?

          Show
          crocket added a comment - - edited I decided to stick to 4.2 for a while. Should I override methods in both FacetsAccumulator.java and CountingListBuilder.java to make it work?
          Hide
          Shai Erera added a comment -

          The fix that I added to CountingListBuilder is only for the case where you index facets such as "a", "b", which is done in tests only. Usually, your facets will look like dimension/level1[/level2/level3...], in which case you're not affected by the fix in CLB. I would just extends FacetsAccumulator with a TODO "remove when 4.3 is out"...

          Show
          Shai Erera added a comment - The fix that I added to CountingListBuilder is only for the case where you index facets such as "a", "b", which is done in tests only. Usually, your facets will look like dimension/level1 [/level2/level3...] , in which case you're not affected by the fix in CLB. I would just extends FacetsAccumulator with a TODO "remove when 4.3 is out"...
          Hide
          crocket added a comment - - edited

          1) I have a facet "me" that doesn't have a subcategory. Does it mean I need to modify CountingListBuilder as well as FacetsAccumulator or just use StandardFacetsAccumulator?

          2) I tried to use StandardFacetsAccumulator, but FacetsCollector.getFacetResults still throws "java.lang.ArrayIndexOutOfBoundsException: 0"
          FacetsAccumulator sfa=StandardFacetsAccumulator.create(fsp, searcher.getIndexReader(), taxoReader);
          FacetsCollector fc = FacetsCollector.create(sfa);

          Does it mean I need to apply your patch to my project?

          Show
          crocket added a comment - - edited 1) I have a facet "me" that doesn't have a subcategory. Does it mean I need to modify CountingListBuilder as well as FacetsAccumulator or just use StandardFacetsAccumulator? 2) I tried to use StandardFacetsAccumulator, but FacetsCollector.getFacetResults still throws "java.lang.ArrayIndexOutOfBoundsException: 0" FacetsAccumulator sfa=StandardFacetsAccumulator.create(fsp, searcher.getIndexReader(), taxoReader); FacetsCollector fc = FacetsCollector.create(sfa); Does it mean I need to apply your patch to my project?
          Hide
          Shai Erera added a comment -

          Ahh. Well, you can still do without modifying CLB, by specifying OrdinalPolicy.ALL_PARENTS for you category lists. That's a change we've done in 4.2, that the root dimension ordinal is not indexed by default (== OP.ALL_BUT_DIMENSION) to save some space as well as CPU cycles. The downside (besides the bug!) is that you don't get the count of the dimension. Performance-wise, this improved some, but not critical. So you can choose between overriding FacetIndexParams.getCategoryListParams() to always return a CLP which specifies OP.ALL_PARENTS for all categories, or extend CLB and apply the fix locally. In both cases, I would put the TODO .

          Show
          Shai Erera added a comment - Ahh. Well, you can still do without modifying CLB, by specifying OrdinalPolicy.ALL_PARENTS for you category lists. That's a change we've done in 4.2, that the root dimension ordinal is not indexed by default (== OP.ALL_BUT_DIMENSION) to save some space as well as CPU cycles. The downside (besides the bug!) is that you don't get the count of the dimension. Performance-wise, this improved some, but not critical. So you can choose between overriding FacetIndexParams.getCategoryListParams() to always return a CLP which specifies OP.ALL_PARENTS for all categories, or extend CLB and apply the fix locally. In both cases, I would put the TODO .
          Hide
          Shai Erera added a comment -

          I tried to use StandardFacetsAccumulator, but FacetsCollector.getFacetResults still throws "java.lang.ArrayIndexOutOfBoundsException: 0"

          Strange, the test I added to TestFacetsCollector passed with StandardFacetsAccumulator. Also, I don't see that SFA has a static create() method – are you sure you're using the right version of the code? Or perhaps it was a copy/paste bug?

          Show
          Shai Erera added a comment - I tried to use StandardFacetsAccumulator, but FacetsCollector.getFacetResults still throws "java.lang.ArrayIndexOutOfBoundsException: 0" Strange, the test I added to TestFacetsCollector passed with StandardFacetsAccumulator. Also, I don't see that SFA has a static create() method – are you sure you're using the right version of the code? Or perhaps it was a copy/paste bug?
          Hide
          crocket added a comment - - edited

          It turned out that StandardFacetsAccumulator inherited create from FacetsAccumulator.
          Thus, when I invoked StandardFacetsAccumulator.create, FacetsAccumulator.create was called actually.

          After replacing StandardFacetsAccumulator.create with new StandardFacetsAccumulator, it worked.

          I'll replace StandardFacetsAccumulator with something else when 4.3 comes around.

          I guess it is safe to close the issue.

          Show
          crocket added a comment - - edited It turned out that StandardFacetsAccumulator inherited create from FacetsAccumulator. Thus, when I invoked StandardFacetsAccumulator.create, FacetsAccumulator.create was called actually. After replacing StandardFacetsAccumulator.create with new StandardFacetsAccumulator, it worked. I'll replace StandardFacetsAccumulator with something else when 4.3 comes around. I guess it is safe to close the issue.
          Hide
          Shai Erera added a comment - - edited

          It turned out that StandardFacetsAccumulator inherited create from FacetsAccumulator.

          Ahh, right. That's why in eclipse I mark these things as warnings because it's dangerous to call static methods in these cases. At any rate, this is just a temporary workaround until 4.3. Thanks for bringing closure!

          Show
          Shai Erera added a comment - - edited It turned out that StandardFacetsAccumulator inherited create from FacetsAccumulator. Ahh, right. That's why in eclipse I mark these things as warnings because it's dangerous to call static methods in these cases. At any rate, this is just a temporary workaround until 4.3. Thanks for bringing closure!
          Hide
          Uwe Schindler added a comment -

          Closed after release.

          Show
          Uwe Schindler added a comment - Closed after release.

            People

            • Assignee:
              Shai Erera
              Reporter:
              crocket
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development