Skip to content

Commit 97a21b1

Browse files
expaniexpani
committed
Removed equals and hashCode from filters and changed back to using List
Signed-off-by: expani <[email protected]>
1 parent 197e885 commit 97a21b1

File tree

8 files changed

+25
-120
lines changed

8 files changed

+25
-120
lines changed

‎server/src/main/java/org/opensearch/search/startree/StarTreeTraversalUtil.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public static FixedBitSet getStarTreeResult(StarTreeValues starTreeValues, StarT
8282

8383
StarTreeValuesIterator valuesIterator = starTreeValues.getDimensionValuesIterator(remainingPredicateColumn);
8484
// Get the query value directly
85-
Set<DimensionFilter> dimensionFilters = starTreeFilter.getFiltersForDimension(remainingPredicateColumn);
85+
List<DimensionFilter> dimensionFilters = starTreeFilter.getFiltersForDimension(remainingPredicateColumn);
8686

8787
// Clear the temporary bit set before reuse
8888
tempBitSet.clear(0, starTreeResult.maxMatchedDoc + 1);
@@ -167,7 +167,7 @@ private static StarTreeResult traverseStarTree(StarTreeValues starTreeValues, St
167167
}
168168

169169
if (remainingPredicateColumns.contains(childDimension)) {
170-
Set<DimensionFilter> dimensionFilters = starTreeFilter.getFiltersForDimension(childDimension);
170+
List<DimensionFilter> dimensionFilters = starTreeFilter.getFiltersForDimension(childDimension);
171171
final boolean[] tempFoundLeafNodes = new boolean[1];
172172
for (DimensionFilter dimensionFilter : dimensionFilters) {
173173
dimensionFilter.matchStarTreeNodes(starTreeNode, starTreeValues, node -> {

‎server/src/main/java/org/opensearch/search/startree/filter/ExactMatchDimFilter.java

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,8 @@
1818
import org.opensearch.search.startree.filter.provider.DimensionFilterMapper;
1919

2020
import java.io.IOException;
21-
import java.util.HashSet;
2221
import java.util.List;
23-
import java.util.Objects;
2422
import java.util.Optional;
25-
import java.util.Set;
2623
import java.util.TreeSet;
2724

2825
/**
@@ -33,14 +30,14 @@ public class ExactMatchDimFilter implements DimensionFilter {
3330

3431
private final String dimensionName;
3532

36-
private final Set<Object> rawValues;
33+
private final List<Object> rawValues;
3734

3835
// Order is essential for successive binary search
3936
private TreeSet<Long> convertedOrdinals;
4037

4138
public ExactMatchDimFilter(String dimensionName, List<Object> valuesToMatch) {
4239
this.dimensionName = dimensionName;
43-
this.rawValues = new HashSet<>(valuesToMatch);
40+
this.rawValues = valuesToMatch;
4441
}
4542

4643
@Override
@@ -80,18 +77,6 @@ public void matchStarTreeNodes(StarTreeNode parentNode, StarTreeValues starTreeV
8077
}
8178
}
8279

83-
@Override
84-
public boolean equals(Object o) {
85-
if (!(o instanceof ExactMatchDimFilter)) return false;
86-
ExactMatchDimFilter that = (ExactMatchDimFilter) o;
87-
return Objects.equals(dimensionName, that.dimensionName) && Objects.equals(rawValues, that.rawValues);
88-
}
89-
90-
@Override
91-
public int hashCode() {
92-
return Objects.hash(dimensionName, rawValues);
93-
}
94-
9580
@Override
9681
public boolean matchDimValue(long ordinal, StarTreeValues starTreeValues) {
9782
return convertedOrdinals.contains(ordinal);

‎server/src/main/java/org/opensearch/search/startree/filter/RangeMatchDimFilter.java

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.opensearch.search.startree.filter.provider.DimensionFilterMapper;
1717

1818
import java.io.IOException;
19-
import java.util.Objects;
2019
import java.util.Optional;
2120

2221
/**
@@ -87,19 +86,4 @@ public boolean matchDimValue(long ordinal, StarTreeValues starTreeValues) {
8786
return lowOrdinal <= ordinal && ordinal <= highOrdinal;
8887
}
8988

90-
@Override
91-
public boolean equals(Object o) {
92-
if (!(o instanceof RangeMatchDimFilter)) return false;
93-
RangeMatchDimFilter that = (RangeMatchDimFilter) o;
94-
return includeLow == that.includeLow
95-
&& includeHigh == that.includeHigh
96-
&& Objects.equals(dimensionName, that.dimensionName)
97-
&& Objects.equals(low, that.low)
98-
&& Objects.equals(high, that.high);
99-
}
100-
101-
@Override
102-
public int hashCode() {
103-
return Objects.hash(dimensionName, low, high, includeLow, includeHigh);
104-
}
10589
}

‎server/src/main/java/org/opensearch/search/startree/filter/StarTreeFilter.java

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010

1111
import org.opensearch.common.annotation.ExperimentalApi;
1212

13+
import java.util.List;
1314
import java.util.Map;
14-
import java.util.Objects;
1515
import java.util.Set;
1616

1717
/**
@@ -20,31 +20,19 @@
2020
@ExperimentalApi
2121
public class StarTreeFilter {
2222

23-
private final Map<String, Set<DimensionFilter>> dimensionFilterMap;
23+
private final Map<String, List<DimensionFilter>> dimensionFilterMap;
2424

25-
public StarTreeFilter(Map<String, Set<DimensionFilter>> dimensionFilterMap) {
25+
public StarTreeFilter(Map<String, List<DimensionFilter>> dimensionFilterMap) {
2626
this.dimensionFilterMap = dimensionFilterMap;
2727
}
2828

29-
public Set<DimensionFilter> getFiltersForDimension(String dimension) {
29+
public List<DimensionFilter> getFiltersForDimension(String dimension) {
3030
return dimensionFilterMap.get(dimension);
3131
}
3232

3333
public Set<String> getDimensions() {
3434
return dimensionFilterMap.keySet();
3535
}
36-
37-
@Override
38-
public boolean equals(Object o) {
39-
if (!(o instanceof StarTreeFilter)) return false;
40-
StarTreeFilter that = (StarTreeFilter) o;
41-
return Objects.equals(dimensionFilterMap, that.dimensionFilterMap);
42-
}
43-
44-
@Override
45-
public int hashCode() {
46-
return Objects.hashCode(dimensionFilterMap);
47-
}
4836
// TODO : Implement Merging of 2 Star Tree Filters
4937
// This would also involve merging 2 different types of dimension filters.
5038
// It also brings in the challenge of sorting input values in user query for efficient merging.

‎server/src/main/java/org/opensearch/search/startree/filter/provider/StarTreeFilterProvider.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import java.util.Collections;
2828
import java.util.List;
2929
import java.util.Map;
30-
import java.util.Set;
3130

3231
/**
3332
* Converts a {@link QueryBuilder} into a {@link StarTreeFilter} by generating the appropriate @{@link org.opensearch.search.startree.filter.DimensionFilter}
@@ -95,7 +94,10 @@ public StarTreeFilter getFilter(SearchContext context, QueryBuilder rawFilter, C
9594
return new StarTreeFilter(Collections.emptyMap());
9695
} else {
9796
return new StarTreeFilter(
98-
Map.of(field, Set.of(dimensionFilterMapper.getExactMatchFilter(mappedFieldType, List.of(termQueryBuilder.value()))))
97+
Map.of(
98+
field,
99+
List.of(dimensionFilterMapper.getExactMatchFilter(mappedFieldType, List.of(termQueryBuilder.value())))
100+
)
99101
);
100102
}
101103
}
@@ -122,7 +124,7 @@ public StarTreeFilter getFilter(SearchContext context, QueryBuilder rawFilter, C
122124
return new StarTreeFilter(Collections.emptyMap());
123125
} else {
124126
return new StarTreeFilter(
125-
Map.of(field, Set.of(dimensionFilterMapper.getExactMatchFilter(mappedFieldType, termsQueryBuilder.values())))
127+
Map.of(field, List.of(dimensionFilterMapper.getExactMatchFilter(mappedFieldType, termsQueryBuilder.values())))
126128
);
127129
}
128130
}
@@ -152,7 +154,7 @@ public StarTreeFilter getFilter(SearchContext context, QueryBuilder rawFilter, C
152154
return new StarTreeFilter(
153155
Map.of(
154156
field,
155-
Set.of(
157+
List.of(
156158
dimensionFilterMapper.getRangeMatchFilter(
157159
mappedFieldType,
158160
rangeQueryBuilder.from(),

‎server/src/test/java/org/opensearch/search/aggregations/startree/DimensionFilterAndMapperTests.java

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,11 @@
1414
import org.opensearch.index.compositeindex.datacube.startree.utils.iterator.SortedSetStarTreeValuesIterator;
1515
import org.opensearch.index.mapper.KeywordFieldMapper;
1616
import org.opensearch.search.startree.filter.DimensionFilter.MatchType;
17-
import org.opensearch.search.startree.filter.ExactMatchDimFilter;
18-
import org.opensearch.search.startree.filter.RangeMatchDimFilter;
19-
import org.opensearch.search.startree.filter.StarTreeFilter;
2017
import org.opensearch.search.startree.filter.provider.DimensionFilterMapper;
2118
import org.opensearch.test.OpenSearchTestCase;
2219

2320
import java.io.IOException;
24-
import java.util.List;
25-
import java.util.Map;
2621
import java.util.Optional;
27-
import java.util.Set;
2822

2923
import static org.mockito.Mockito.mock;
3024
import static org.mockito.Mockito.when;
@@ -115,51 +109,4 @@ public void testKeywordOrdinalMapping() throws IOException {
115109

116110
}
117111

118-
public void testFilterComparison() {
119-
120-
// Range Filter Same
121-
RangeMatchDimFilter rangeFilter1 = new RangeMatchDimFilter("field", 12, 14, true, false);
122-
RangeMatchDimFilter rangeFilter2 = new RangeMatchDimFilter("field", 12, 14, true, false);
123-
assertEquals(rangeFilter1, rangeFilter2);
124-
125-
// Range Filter Different
126-
rangeFilter2 = new RangeMatchDimFilter("field", 12, 14, true, true);
127-
assertNotEquals(rangeFilter1, rangeFilter2);
128-
rangeFilter2 = new RangeMatchDimFilter("field", 12, 15, true, false);
129-
assertNotEquals(rangeFilter1, rangeFilter2);
130-
rangeFilter2 = new RangeMatchDimFilter("fields", 12, 14, true, false);
131-
assertNotEquals(rangeFilter1, rangeFilter2);
132-
rangeFilter2 = new RangeMatchDimFilter("field", 12, 14, false, false);
133-
assertNotEquals(rangeFilter1, rangeFilter2);
134-
rangeFilter2 = new RangeMatchDimFilter("field", 11, 14, true, false);
135-
assertNotEquals(rangeFilter1, rangeFilter2);
136-
137-
// ExactMatch Filter Same
138-
ExactMatchDimFilter exactFilter1 = new ExactMatchDimFilter("field", List.of("hello", "world"));
139-
ExactMatchDimFilter exactFilter2 = new ExactMatchDimFilter("field", List.of("world", "hello"));
140-
assertEquals(exactFilter1, exactFilter2);
141-
142-
// ExactMatch Filter Different
143-
exactFilter2 = new ExactMatchDimFilter("field2", List.of("hello", "world"));
144-
assertNotEquals(exactFilter1, exactFilter2);
145-
exactFilter2 = new ExactMatchDimFilter("field", List.of("hello", "worlds"));
146-
assertNotEquals(exactFilter1, exactFilter2);
147-
148-
// Same StarTreeFilter
149-
StarTreeFilter starTreeFilter1 = new StarTreeFilter(Map.of("field", Set.of(rangeFilter1)));
150-
StarTreeFilter starTreeFilter2 = new StarTreeFilter(Map.of("field", Set.of(rangeFilter1)));
151-
assertEquals(starTreeFilter1, starTreeFilter2);
152-
rangeFilter2 = new RangeMatchDimFilter("field", 12, 14, true, false);
153-
starTreeFilter2 = new StarTreeFilter(Map.of("field", Set.of(rangeFilter2)));
154-
assertEquals(starTreeFilter1, starTreeFilter2);
155-
156-
// Different StarTreeFilter
157-
starTreeFilter2 = new StarTreeFilter(Map.of("field1", Set.of(rangeFilter2)));
158-
assertNotEquals(starTreeFilter1, starTreeFilter2);
159-
rangeFilter2 = new RangeMatchDimFilter("field", 12, 15, true, false);
160-
starTreeFilter2 = new StarTreeFilter(Map.of("field", Set.of(rangeFilter2)));
161-
assertNotEquals(starTreeFilter1, starTreeFilter2);
162-
163-
}
164-
165112
}

‎server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ private void testStarTreeDocValuesInternal(Codec codec, List<DimensionFieldData>
395395

396396
// Keyword Range query with missing Low Ordinal
397397
RangeQueryBuilder rangeQueryBuilder = new RangeQueryBuilder("keyword_field");
398-
rangeQueryBuilder.from("non_existing_value_here").includeLower(random().nextBoolean());
398+
rangeQueryBuilder.from(Long.MAX_VALUE).includeLower(random().nextBoolean());
399399
testCase(
400400
indexSearcher,
401401
rangeQueryBuilder.toQuery(queryShardContext),

‎server/src/test/java/org/opensearch/search/aggregations/startree/StarTreeQueryTests.java

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import java.util.LinkedHashMap;
5353
import java.util.List;
5454
import java.util.Map;
55-
import java.util.Set;
5655

5756
import org.mockito.Mockito;
5857

@@ -172,7 +171,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
172171
// single filter - matches docs
173172
starTreeDocCount = getDocCountFromStarTree(
174173
starTreeDocValuesReader,
175-
new StarTreeFilter(Map.of(SNDV, Set.of(new ExactMatchDimFilter(SNDV, List.of(0L))))),
174+
new StarTreeFilter(Map.of(SNDV, List.of(new ExactMatchDimFilter(SNDV, List.of(0L))))),
176175
context,
177176
searchContext
178177
);
@@ -183,7 +182,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
183182
// single filter on 3rd field in ordered dimension - matches docs
184183
starTreeDocCount = getDocCountFromStarTree(
185184
starTreeDocValuesReader,
186-
new StarTreeFilter(Map.of(DV, Set.of(new ExactMatchDimFilter(DV, List.of(0L))))),
185+
new StarTreeFilter(Map.of(DV, List.of(new ExactMatchDimFilter(DV, List.of(0L))))),
187186
context,
188187
searchContext
189188
);
@@ -194,7 +193,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
194193
// single filter - does not match docs
195194
starTreeDocCount = getDocCountFromStarTree(
196195
starTreeDocValuesReader,
197-
new StarTreeFilter(Map.of(SNDV, Set.of(new ExactMatchDimFilter(SNDV, List.of(101L))))),
196+
new StarTreeFilter(Map.of(SNDV, List.of(new ExactMatchDimFilter(SNDV, List.of(101L))))),
198197
context,
199198
searchContext
200199
);
@@ -205,7 +204,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
205204
// single filter on 3rd field in ordered dimension - does not match docs
206205
starTreeDocCount = getDocCountFromStarTree(
207206
starTreeDocValuesReader,
208-
new StarTreeFilter(Map.of(SNDV, Set.of(new ExactMatchDimFilter(SNDV, List.of(-101L))))),
207+
new StarTreeFilter(Map.of(SNDV, List.of(new ExactMatchDimFilter(SNDV, List.of(-101L))))),
209208
context,
210209
searchContext
211210
);
@@ -217,7 +216,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
217216
starTreeDocCount = getDocCountFromStarTree(
218217
starTreeDocValuesReader,
219218
new StarTreeFilter(
220-
Map.of(SNDV, Set.of(new ExactMatchDimFilter(SNDV, List.of(0L))), DV, Set.of(new ExactMatchDimFilter(DV, List.of(0L))))
219+
Map.of(SNDV, List.of(new ExactMatchDimFilter(SNDV, List.of(0L))), DV, List.of(new ExactMatchDimFilter(DV, List.of(0L))))
221220
),
222221
context,
223222
searchContext
@@ -230,7 +229,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
230229
starTreeDocCount = getDocCountFromStarTree(
231230
starTreeDocValuesReader,
232231
new StarTreeFilter(
233-
Map.of(SNDV, Set.of(new ExactMatchDimFilter(SNDV, List.of(0L))), DV, Set.of(new ExactMatchDimFilter(DV, List.of(-11L))))
232+
Map.of(SNDV, List.of(new ExactMatchDimFilter(SNDV, List.of(0L))), DV, List.of(new ExactMatchDimFilter(DV, List.of(-11L))))
234233
),
235234
context,
236235
searchContext
@@ -243,7 +242,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
243242
starTreeDocCount = getDocCountFromStarTree(
244243
starTreeDocValuesReader,
245244
new StarTreeFilter(
246-
Map.of(SNDV, Set.of(new ExactMatchDimFilter(SNDV, List.of(0L))), DV, Set.of(new ExactMatchDimFilter(DV, List.of(-100L))))
245+
Map.of(SNDV, List.of(new ExactMatchDimFilter(SNDV, List.of(0L))), DV, List.of(new ExactMatchDimFilter(DV, List.of(-100L))))
247246
),
248247
context,
249248
searchContext
@@ -257,7 +256,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
257256
IllegalStateException.class,
258257
() -> getDocCountFromStarTree(
259258
starTreeDocValuesReader,
260-
new StarTreeFilter(Map.of(FIELD_NAME, Set.of(new ExactMatchDimFilter(FIELD_NAME, List.of(0L))))),
259+
new StarTreeFilter(Map.of(FIELD_NAME, List.of(new ExactMatchDimFilter(FIELD_NAME, List.of(0L))))),
261260
context,
262261
searchContext
263262
)
@@ -267,7 +266,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
267266
// Documents are not indexed
268267
starTreeDocCount = getDocCountFromStarTree(
269268
starTreeDocValuesReader,
270-
new StarTreeFilter(Map.of(SDV, Set.of(new ExactMatchDimFilter(SDV, List.of(4L))))),
269+
new StarTreeFilter(Map.of(SDV, List.of(new ExactMatchDimFilter(SDV, List.of(4L))))),
271270
context,
272271
searchContext
273272
);
@@ -278,7 +277,7 @@ private void testStarTreeFilter(int maxLeafDoc, boolean skipStarNodeCreationForS
278277
// Documents are indexed
279278
starTreeDocCount = getDocCountFromStarTree(
280279
starTreeDocValuesReader,
281-
new StarTreeFilter(Map.of(SDV, Set.of(new ExactMatchDimFilter(SDV, List.of(4L))))),
280+
new StarTreeFilter(Map.of(SDV, List.of(new ExactMatchDimFilter(SDV, List.of(4L))))),
282281
context,
283282
searchContext
284283
);

0 commit comments

Comments
 (0)