Browse Source

Partition-based include-exclude does not implement equals/hashcode/serialization correctly. (#22051)

Adrien Grand 8 years ago
parent
commit
1bdf4a2c5b

+ 11 - 3
core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java

@@ -690,6 +690,10 @@ public class IncludeExclude implements Writeable, ToXContent {
             }
             builder.endArray();
         }
+        if (isPartitionBased()) {
+            builder.field(PARTITION_FIELD.getPreferredName(), incZeroBasedPartition);
+            builder.field(NUM_PARTITIONS_FIELD.getPreferredName(), incNumPartitions);
+        }
         if (exclude != null) {
             builder.field(EXCLUDE_FIELD.getPreferredName(), exclude.getOriginalString());
         }
@@ -705,8 +709,10 @@ public class IncludeExclude implements Writeable, ToXContent {
 
     @Override
     public int hashCode() {
-        return Objects.hash(include == null ? null : include.getOriginalString(), exclude == null ? null : exclude.getOriginalString(),
-                includeValues, excludeValues);
+        return Objects.hash(
+                include == null ? null : include.getOriginalString(),
+                exclude == null ? null : exclude.getOriginalString(),
+                includeValues, excludeValues, incZeroBasedPartition, incNumPartitions);
     }
 
     @Override
@@ -720,7 +726,9 @@ public class IncludeExclude implements Writeable, ToXContent {
         return Objects.equals(include == null ? null : include.getOriginalString(), other.include == null ? null : other.include.getOriginalString())
                 && Objects.equals(exclude == null ? null : exclude.getOriginalString(), other.exclude == null ? null : other.exclude.getOriginalString())
                 && Objects.equals(includeValues, other.includeValues)
-                && Objects.equals(excludeValues, other.excludeValues);
+                && Objects.equals(excludeValues, other.excludeValues)
+                && Objects.equals(incZeroBasedPartition, other.incZeroBasedPartition)
+                && Objects.equals(incNumPartitions, other.incNumPartitions);
     }
 
 }

+ 6 - 1
core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsTests.java

@@ -117,7 +117,7 @@ public class TermsTests extends BaseAggregationTestCase<TermsAggregationBuilder>
         }
         if (randomBoolean()) {
             IncludeExclude incExc = null;
-            switch (randomInt(5)) {
+            switch (randomInt(6)) {
             case 0:
                 incExc = new IncludeExclude(new RegExp("foobar"), null);
                 break;
@@ -158,6 +158,11 @@ public class TermsTests extends BaseAggregationTestCase<TermsAggregationBuilder>
                 }
                 incExc = new IncludeExclude(includeValues3, excludeValues3);
                 break;
+            case 6:
+                final int numPartitions = randomIntBetween(1, 100);
+                final int partition = randomIntBetween(0, numPartitions - 1);
+                incExc = new IncludeExclude(partition, numPartitions);
+                break;
             default:
                 fail();
             }

+ 9 - 0
core/src/test/java/org/elasticsearch/search/aggregations/support/IncludeExcludeTests.java

@@ -125,4 +125,13 @@ public class IncludeExcludeTests extends ESTestCase {
         assertFalse(acceptedOrds.get(0));
     }
 
+    public void testEquals() {
+        assertEquals(new IncludeExclude(3, 20), new IncludeExclude(3, 20));
+        assertEquals(new IncludeExclude(3, 20).hashCode(), new IncludeExclude(3, 20).hashCode());
+        assertFalse(new IncludeExclude(3, 20).equals(new IncludeExclude(4, 20)));
+        assertTrue(new IncludeExclude(3, 20).hashCode() != new IncludeExclude(4, 20).hashCode());
+        assertFalse(new IncludeExclude(3, 20).equals(new IncludeExclude(3, 21)));
+        assertTrue(new IncludeExclude(3, 20).hashCode() != new IncludeExclude(3, 21).hashCode());
+    }
+
 }