diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 5af2903eaa4..95a5f46418c 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -100,6 +100,9 @@ Bug Fixes considered if the input is equal causing the malformed weight to be identical as well. (Simon Willnauer) +* LUCENE-5151: Associations FacetsAggregators could enter an infinite loop when + some result documents were missing category associations. (Shai Erera) + API Changes * LUCENE-5094: Add ramBytesUsed() to MultiDocValues.OrdinalMap. diff --git a/lucene/facet/src/java/org/apache/lucene/facet/associations/SumFloatAssociationFacetsAggregator.java b/lucene/facet/src/java/org/apache/lucene/facet/associations/SumFloatAssociationFacetsAggregator.java index ce5270276a0..4ad33b032fc 100644 --- a/lucene/facet/src/java/org/apache/lucene/facet/associations/SumFloatAssociationFacetsAggregator.java +++ b/lucene/facet/src/java/org/apache/lucene/facet/associations/SumFloatAssociationFacetsAggregator.java @@ -54,23 +54,20 @@ public class SumFloatAssociationFacetsAggregator implements FacetsAggregator { int doc = 0; while (doc < length && (doc = matchingDocs.bits.nextSetBit(doc)) != -1) { dv.get(doc, bytes); - if (bytes.length == 0) { - continue; // no associations for this document + if (bytes.length > 0) { + // aggreate float association values for ordinals + int bytesUpto = bytes.offset + bytes.length; + int pos = bytes.offset; + while (pos < bytesUpto) { + int ordinal = ((bytes.bytes[pos++] & 0xFF) << 24) | ((bytes.bytes[pos++] & 0xFF) << 16) + | ((bytes.bytes[pos++] & 0xFF) << 8) | (bytes.bytes[pos++] & 0xFF); + + int value = ((bytes.bytes[pos++] & 0xFF) << 24) | ((bytes.bytes[pos++] & 0xFF) << 16) + | ((bytes.bytes[pos++] & 0xFF) << 8) | (bytes.bytes[pos++] & 0xFF); + + values[ordinal] += Float.intBitsToFloat(value); + } } - - // aggreate float association values for ordinals - int bytesUpto = bytes.offset + bytes.length; - int pos = bytes.offset; - while (pos < bytesUpto) { - int ordinal = ((bytes.bytes[pos++] & 0xFF) << 24) | ((bytes.bytes[pos++] & 0xFF) << 16) - | ((bytes.bytes[pos++] & 0xFF) << 8) | (bytes.bytes[pos++] & 0xFF); - - int value = ((bytes.bytes[pos++] & 0xFF) << 24) | ((bytes.bytes[pos++] & 0xFF) << 16) - | ((bytes.bytes[pos++] & 0xFF) << 8) | (bytes.bytes[pos++] & 0xFF); - - values[ordinal] += Float.intBitsToFloat(value); - } - ++doc; } } diff --git a/lucene/facet/src/java/org/apache/lucene/facet/associations/SumIntAssociationFacetsAggregator.java b/lucene/facet/src/java/org/apache/lucene/facet/associations/SumIntAssociationFacetsAggregator.java index 03d035e0a2f..13d7ea206ac 100644 --- a/lucene/facet/src/java/org/apache/lucene/facet/associations/SumIntAssociationFacetsAggregator.java +++ b/lucene/facet/src/java/org/apache/lucene/facet/associations/SumIntAssociationFacetsAggregator.java @@ -53,23 +53,20 @@ public class SumIntAssociationFacetsAggregator implements FacetsAggregator { int doc = 0; while (doc < length && (doc = matchingDocs.bits.nextSetBit(doc)) != -1) { dv.get(doc, bytes); - if (bytes.length == 0) { - continue; // no associations for this document + if (bytes.length > 0) { + // aggreate association values for ordinals + int bytesUpto = bytes.offset + bytes.length; + int pos = bytes.offset; + while (pos < bytesUpto) { + int ordinal = ((bytes.bytes[pos++] & 0xFF) << 24) | ((bytes.bytes[pos++] & 0xFF) << 16) + | ((bytes.bytes[pos++] & 0xFF) << 8) | (bytes.bytes[pos++] & 0xFF); + + int value = ((bytes.bytes[pos++] & 0xFF) << 24) | ((bytes.bytes[pos++] & 0xFF) << 16) + | ((bytes.bytes[pos++] & 0xFF) << 8) | (bytes.bytes[pos++] & 0xFF); + + values[ordinal] += value; + } } - - // aggreate association values for ordinals - int bytesUpto = bytes.offset + bytes.length; - int pos = bytes.offset; - while (pos < bytesUpto) { - int ordinal = ((bytes.bytes[pos++] & 0xFF) << 24) | ((bytes.bytes[pos++] & 0xFF) << 16) - | ((bytes.bytes[pos++] & 0xFF) << 8) | (bytes.bytes[pos++] & 0xFF); - - int value = ((bytes.bytes[pos++] & 0xFF) << 24) | ((bytes.bytes[pos++] & 0xFF) << 16) - | ((bytes.bytes[pos++] & 0xFF) << 8) | (bytes.bytes[pos++] & 0xFF); - - values[ordinal] += value; - } - ++doc; } } diff --git a/lucene/facet/src/test/org/apache/lucene/facet/associations/AssociationsFacetRequestTest.java b/lucene/facet/src/test/org/apache/lucene/facet/associations/AssociationsFacetRequestTest.java index 54babc6d4da..29b2764b978 100644 --- a/lucene/facet/src/test/org/apache/lucene/facet/associations/AssociationsFacetRequestTest.java +++ b/lucene/facet/src/test/org/apache/lucene/facet/associations/AssociationsFacetRequestTest.java @@ -67,14 +67,18 @@ public class AssociationsFacetRequestTest extends FacetTestCase { AssociationsFacetFields assocFacetFields = new AssociationsFacetFields(taxoWriter); // index documents, 50% have only 'b' and all have 'a' - for (int i = 0; i < 100; i++) { + for (int i = 0; i < 110; i++) { Document doc = new Document(); CategoryAssociationsContainer associations = new CategoryAssociationsContainer(); - associations.setAssociation(aint, new CategoryIntAssociation(2)); - associations.setAssociation(afloat, new CategoryFloatAssociation(0.5f)); - if (i % 2 == 0) { // 50 - associations.setAssociation(bint, new CategoryIntAssociation(3)); - associations.setAssociation(bfloat, new CategoryFloatAssociation(0.2f)); + // every 11th document is added empty, this used to cause the association + // aggregators to go into an infinite loop + if (i % 11 != 0) { + associations.setAssociation(aint, new CategoryIntAssociation(2)); + associations.setAssociation(afloat, new CategoryFloatAssociation(0.5f)); + if (i % 2 == 0) { // 50 + associations.setAssociation(bint, new CategoryIntAssociation(3)); + associations.setAssociation(bfloat, new CategoryFloatAssociation(0.2f)); + } } assocFacetFields.addFields(doc, associations); writer.addDocument(doc); @@ -185,6 +189,6 @@ public class AssociationsFacetRequestTest extends FacetTestCase { assertEquals("Wrong count for category 'b'!",10f, (float) res.get(3).getFacetResultNode().value, 0.00001); taxo.close(); - } + } }