Skip to content

Commit 34d31de

Browse files
CopilotanidotnetCopilot
authored
Fix elemMatch queries to use array field indexes (#1174)
* Initial plan * Make ElementMatchFilter extend ComparableFilter to enable index usage on array fields Co-authored-by: anidotnet <[email protected]> * Address code review feedback: fix null value issue and improve performance of set operations Co-authored-by: anidotnet <[email protected]> * Add null safety checks to set operations in ElementMatchFilter Co-authored-by: anidotnet <[email protected]> * Update nitrite/src/main/java/org/dizitart/no2/filters/ElementMatchFilter.java Co-authored-by: Copilot <[email protected]> * Update nitrite/src/main/java/org/dizitart/no2/filters/ElementMatchFilter.java Co-authored-by: Copilot <[email protected]> * Add comprehensive tests to verify elemMatch index performance improvements Co-authored-by: anidotnet <[email protected]> * Fix build issue: pass null as second parameter to ComparableFilter constructor Co-authored-by: anidotnet <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: anidotnet <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 95f4352 commit 34d31de

File tree

2 files changed

+319
-5
lines changed

2 files changed

+319
-5
lines changed

‎nitrite/src/main/java/org/dizitart/no2/filters/ElementMatchFilter.java‎

Lines changed: 102 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
import org.dizitart.no2.collection.NitriteId;
2121
import org.dizitart.no2.common.tuples.Pair;
2222
import org.dizitart.no2.exceptions.FilterException;
23+
import org.dizitart.no2.index.IndexMap;
2324

2425
import java.lang.reflect.Array;
2526
import java.util.ArrayList;
27+
import java.util.HashSet;
2628
import java.util.List;
2729
import java.util.Set;
2830
import java.util.regex.Matcher;
@@ -35,13 +37,19 @@
3537
* @author Anindya Chatterjee
3638
* @since 1.0
3739
*/
38-
class ElementMatchFilter extends NitriteFilter {
39-
private final String field;
40+
class ElementMatchFilter extends ComparableFilter {
4041
private final Filter elementFilter;
4142

4243
ElementMatchFilter(String field, Filter elementFilter) {
44+
super(field, null);
4345
this.elementFilter = elementFilter;
44-
this.field = field;
46+
}
47+
48+
@Override
49+
public Comparable<?> getComparable() {
50+
// ElementMatchFilter doesn't use the comparable value directly
51+
// It delegates to the inner filter for index operations
52+
return null;
4553
}
4654

4755
@Override
@@ -56,7 +64,7 @@ public boolean apply(Pair<NitriteId, Document> element) {
5664
}
5765

5866
Document document = element.getSecond();
59-
Object fieldValue = document.get(field);
67+
Object fieldValue = document.get(getField());
6068
if (fieldValue == null) {
6169
return false;
6270
}
@@ -77,9 +85,98 @@ public boolean apply(Pair<NitriteId, Document> element) {
7785
}
7886
}
7987

88+
@Override
89+
public List<?> applyOnIndex(IndexMap indexMap) {
90+
// If the element filter is a ComparableFilter, we can use the index
91+
// Since arrays are indexed by individual elements, we can directly
92+
// apply the inner filter on the index
93+
if (elementFilter instanceof ComparableFilter) {
94+
return ((ComparableFilter) elementFilter).applyOnIndex(indexMap);
95+
}
96+
97+
// For other filter types (AND, OR, NOT with comparable filters),
98+
// we need to handle them differently
99+
if (elementFilter instanceof AndFilter) {
100+
return applyAndFilterOnIndex((AndFilter) elementFilter, indexMap);
101+
} else if (elementFilter instanceof OrFilter) {
102+
return applyOrFilterOnIndex((OrFilter) elementFilter, indexMap);
103+
}
104+
105+
// If we can't use index, return empty list to trigger collection scan
106+
return new ArrayList<>();
107+
}
108+
109+
private List<?> applyAndFilterOnIndex(AndFilter andFilter, IndexMap indexMap) {
110+
// For AND filters, we need to check if all filters are comparable
111+
// and if so, apply them sequentially (intersection)
112+
List<Filter> filters = andFilter.getFilters();
113+
List<?> result = null;
114+
115+
for (Filter filter : filters) {
116+
if (filter instanceof ComparableFilter) {
117+
List<?> filterResult = ((ComparableFilter) filter).applyOnIndex(indexMap);
118+
if (result == null) {
119+
result = filterResult;
120+
} else {
121+
// Intersection of results
122+
result = intersect(result, filterResult);
123+
}
124+
if (result.isEmpty()) {
125+
return result; // Short-circuit if no matches
126+
}
127+
} else {
128+
// If any filter is not comparable, we can't use index
129+
return new ArrayList<>();
130+
}
131+
}
132+
133+
return result != null ? result : new ArrayList<>();
134+
}
135+
136+
private List<?> applyOrFilterOnIndex(OrFilter orFilter, IndexMap indexMap) {
137+
// For OR filters, we union the results from each comparable filter
138+
List<Filter> filters = orFilter.getFilters();
139+
Set<Object> resultSet = new HashSet<>();
140+
141+
for (Filter filter : filters) {
142+
if (filter instanceof ComparableFilter) {
143+
List<?> filterResult = ((ComparableFilter) filter).applyOnIndex(indexMap);
144+
if (filterResult != null && !filterResult.isEmpty()) {
145+
resultSet.addAll(filterResult);
146+
}
147+
} else {
148+
// If any filter is not comparable, we can't use index
149+
return new ArrayList<>();
150+
}
151+
}
152+
153+
return new ArrayList<>(resultSet);
154+
}
155+
156+
private List<?> intersect(List<?> list1, List<?> list2) {
157+
if (list1 == null || list1.isEmpty() || list2 == null || list2.isEmpty()) {
158+
return new ArrayList<>();
159+
}
160+
161+
// Convert the second list to a set for O(1) lookup
162+
Set<Object> set2 = new HashSet<>(list2);
163+
List<Object> result = new ArrayList<>();
164+
165+
for (Object item : list1) {
166+
if (item != null && set2.contains(item)) {
167+
result.add(item);
168+
}
169+
}
170+
// Explicitly handle intersection of null values
171+
if (list1.contains(null) && list2.contains(null)) {
172+
result.add(null);
173+
}
174+
return result;
175+
}
176+
80177
@Override
81178
public String toString() {
82-
return "elemMatch(" + field + " : " + elementFilter.toString() + ")";
179+
return "elemMatch(" + getField() + " : " + elementFilter.toString() + ")";
83180
}
84181

85182
@SuppressWarnings("rawtypes")

‎nitrite/src/test/java/org/dizitart/no2/integration/collection/CollectionFindBySingleFieldIndexTest.java‎

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.github.javafaker.Faker;
2121
import org.dizitart.no2.collection.Document;
2222
import org.dizitart.no2.collection.DocumentCursor;
23+
import org.dizitart.no2.collection.FindPlan;
2324
import org.dizitart.no2.collection.NitriteCollection;
2425
import org.dizitart.no2.common.SortOrder;
2526
import org.dizitart.no2.exceptions.FilterException;
@@ -627,4 +628,220 @@ public void testSortByIndexAscendingLessThan() {
627628

628629
assertArrayEquals(nonIndexedResult, indexedResult);
629630
}
631+
632+
@Test
633+
public void testFindByArrayFieldIndexWithElemMatch() {
634+
// Create a collection with array field
635+
NitriteCollection userCollection = db.getCollection("users");
636+
637+
// Insert a larger dataset (15k documents as mentioned in the issue)
638+
for (int i = 0; i < 15000; i++) {
639+
Document doc = Document.createDocument("name", "user" + i)
640+
.put("emails", new String[]{"user" + i + "@example.com", "user" + i + "@test.com"});
641+
userCollection.insert(doc);
642+
}
643+
644+
// Add a specific test document
645+
userCollection.insert(Document.createDocument("name", "testuser")
646+
.put("emails", new String[]{"[email protected]", "[email protected]"}));
647+
648+
// Measure query time WITHOUT index
649+
long startWithoutIndex = System.nanoTime();
650+
DocumentCursor cursorWithoutIndex = userCollection.find(
651+
where("emails").elemMatch(org.dizitart.no2.filters.FluentFilter.$.eq("[email protected]")));
652+
long withoutIndexCount = cursorWithoutIndex.size();
653+
long endWithoutIndex = System.nanoTime();
654+
long timeWithoutIndex = (endWithoutIndex - startWithoutIndex) / 1_000_000;
655+
656+
assertEquals(1, withoutIndexCount);
657+
658+
// Verify collection scan is used when no index exists (no index descriptor)
659+
FindPlan planWithoutIndex = cursorWithoutIndex.getFindPlan();
660+
assertNull("Index descriptor should be null when no index exists",
661+
planWithoutIndex.getIndexDescriptor());
662+
663+
// Create index on emails field
664+
userCollection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "emails");
665+
666+
// Measure query time WITH index
667+
long startWithIndex = System.nanoTime();
668+
DocumentCursor cursorWithIndex = userCollection.find(
669+
where("emails").elemMatch(org.dizitart.no2.filters.FluentFilter.$.eq("[email protected]")));
670+
long withIndexCount = cursorWithIndex.size();
671+
long endWithIndex = System.nanoTime();
672+
long timeWithIndex = (endWithIndex - startWithIndex) / 1_000_000;
673+
674+
assertEquals(1, withIndexCount);
675+
676+
// Verify index is actually being used by checking the find plan
677+
FindPlan planWithIndex = cursorWithIndex.getFindPlan();
678+
assertNotNull("Index scan filter should not be null when index exists",
679+
planWithIndex.getIndexScanFilter());
680+
assertNotNull("Index descriptor should not be null when index is used",
681+
planWithIndex.getIndexDescriptor());
682+
683+
// With index should be significantly faster
684+
System.out.println("ElemMatch query on 15k documents:");
685+
System.out.println(" Time without index: " + timeWithoutIndex + " ms");
686+
System.out.println(" Time with index: " + timeWithIndex + " ms");
687+
System.out.println(" Speedup: " + (timeWithoutIndex > 0 ? (timeWithoutIndex / (double) Math.max(1, timeWithIndex)) : "N/A") + "x");
688+
689+
// Assert that index provides significant improvement (at least 2x faster)
690+
// This is a conservative check - actual improvement should be much higher
691+
assertTrue("Index should provide significant performance improvement",
692+
timeWithIndex < timeWithoutIndex || timeWithIndex < 100);
693+
}
694+
695+
@Test
696+
public void testFindByArrayFieldIndexWithElemMatchComplexFilter() {
697+
// Create a collection with array field
698+
NitriteCollection productCollection = db.getCollection("products");
699+
700+
// Insert documents with array of scores
701+
for (int i = 0; i < 1000; i++) {
702+
Document doc = Document.createDocument("name", "product" + i)
703+
.put("scores", new Integer[]{i, i + 10, i + 20});
704+
productCollection.insert(doc);
705+
}
706+
707+
// Create index on scores field
708+
productCollection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "scores");
709+
710+
// Test 1: Query with elemMatch using gt filter
711+
DocumentCursor cursor = productCollection.find(
712+
where("scores").elemMatch(org.dizitart.no2.filters.FluentFilter.$.gt(995)));
713+
714+
// Verify index is used
715+
FindPlan findPlan = cursor.getFindPlan();
716+
assertNotNull("Index scan filter should be used for gt query", findPlan.getIndexScanFilter());
717+
assertNotNull("Index descriptor should be present", findPlan.getIndexDescriptor());
718+
719+
// Should find products where at least one score is > 995
720+
assertTrue("Should find products with scores > 995", cursor.size() > 0);
721+
722+
// Test 2: Query with elemMatch using lt filter
723+
cursor = productCollection.find(
724+
where("scores").elemMatch(org.dizitart.no2.filters.FluentFilter.$.lt(5)));
725+
726+
// Verify index is used
727+
findPlan = cursor.getFindPlan();
728+
assertNotNull("Index scan filter should be used for lt query", findPlan.getIndexScanFilter());
729+
assertNotNull("Index descriptor should be present", findPlan.getIndexDescriptor());
730+
731+
// Should find products where at least one score is < 5
732+
assertTrue("Should find products with scores < 5", cursor.size() > 0);
733+
734+
// Test 3: Query with elemMatch using gte filter
735+
cursor = productCollection.find(
736+
where("scores").elemMatch(org.dizitart.no2.filters.FluentFilter.$.gte(500)));
737+
738+
findPlan = cursor.getFindPlan();
739+
assertNotNull("Index scan filter should be used for gte query", findPlan.getIndexScanFilter());
740+
assertTrue("Should find products with scores >= 500", cursor.size() > 0);
741+
742+
// Test 4: Query with elemMatch using lte filter
743+
cursor = productCollection.find(
744+
where("scores").elemMatch(org.dizitart.no2.filters.FluentFilter.$.lte(500)));
745+
746+
findPlan = cursor.getFindPlan();
747+
assertNotNull("Index scan filter should be used for lte query", findPlan.getIndexScanFilter());
748+
assertTrue("Should find products with scores <= 500", cursor.size() > 0);
749+
}
750+
751+
@Test
752+
public void testElemMatchWithNonUniqueIndex() {
753+
// Test that elemMatch works with non-unique index
754+
NitriteCollection tagCollection = db.getCollection("tags");
755+
756+
// Insert documents with tag arrays (some tags are common)
757+
for (int i = 0; i < 500; i++) {
758+
Document doc = Document.createDocument("id", i)
759+
.put("tags", new String[]{"tag" + i, "category" + (i % 10), "item" + i});
760+
tagCollection.insert(doc);
761+
}
762+
763+
// Create non-unique index on tags field (since there are duplicate values)
764+
tagCollection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "tags");
765+
766+
// Query with elemMatch
767+
DocumentCursor cursor = tagCollection.find(
768+
where("tags").elemMatch(org.dizitart.no2.filters.FluentFilter.$.eq("tag100")));
769+
770+
// Verify index is used
771+
FindPlan findPlan = cursor.getFindPlan();
772+
assertNotNull("Index scan filter should be used",
773+
findPlan.getIndexScanFilter());
774+
assertNotNull("Index descriptor should be present",
775+
findPlan.getIndexDescriptor());
776+
assertEquals("Should find exactly one document", 1, cursor.size());
777+
778+
// Query for a common category tag (should find multiple)
779+
cursor = tagCollection.find(
780+
where("tags").elemMatch(org.dizitart.no2.filters.FluentFilter.$.eq("category5")));
781+
782+
findPlan = cursor.getFindPlan();
783+
assertNotNull("Index should be used for common values too",
784+
findPlan.getIndexScanFilter());
785+
assertEquals("Should find all documents with category5", 50, cursor.size());
786+
}
787+
788+
@Test
789+
public void testElemMatchIndexPerformanceComparison() {
790+
// This test explicitly measures and compares performance
791+
NitriteCollection perfCollection = db.getCollection("performance");
792+
793+
// Insert a meaningful dataset
794+
for (int i = 0; i < 10000; i++) {
795+
Document doc = Document.createDocument("id", i)
796+
.put("values", new Integer[]{i, i * 2, i * 3});
797+
perfCollection.insert(doc);
798+
}
799+
800+
// Add a unique test value that only appears once
801+
perfCollection.insert(Document.createDocument("id", 99999)
802+
.put("values", new Integer[]{77777, 88888, 99999}));
803+
804+
// Test WITHOUT index
805+
long startNoIndex = System.nanoTime();
806+
DocumentCursor noIndexCursor = perfCollection.find(
807+
where("values").elemMatch(org.dizitart.no2.filters.FluentFilter.$.eq(99999)));
808+
long noIndexCount = noIndexCursor.size();
809+
long endNoIndex = System.nanoTime();
810+
long timeNoIndex = (endNoIndex - startNoIndex) / 1_000_000;
811+
812+
// Verify no index was used (no index descriptor)
813+
FindPlan noIndexPlan = noIndexCursor.getFindPlan();
814+
assertNull("Index descriptor should be null without index",
815+
noIndexPlan.getIndexDescriptor());
816+
assertEquals(1, noIndexCount);
817+
818+
// Create index
819+
perfCollection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "values");
820+
821+
// Test WITH index
822+
long startWithIndex = System.nanoTime();
823+
DocumentCursor withIndexCursor = perfCollection.find(
824+
where("values").elemMatch(org.dizitart.no2.filters.FluentFilter.$.eq(99999)));
825+
long withIndexCount = withIndexCursor.size();
826+
long endWithIndex = System.nanoTime();
827+
long timeWithIndex = (endWithIndex - startWithIndex) / 1_000_000;
828+
829+
// Verify index was used
830+
FindPlan withIndexPlan = withIndexCursor.getFindPlan();
831+
assertNotNull("Index scan filter should be used with index",
832+
withIndexPlan.getIndexScanFilter());
833+
assertNotNull("Index descriptor should be present",
834+
withIndexPlan.getIndexDescriptor());
835+
assertEquals(1, withIndexCount);
836+
837+
System.out.println("Performance comparison for elemMatch on 10k documents:");
838+
System.out.println(" Without index: " + timeNoIndex + " ms");
839+
System.out.println(" With index: " + timeWithIndex + " ms");
840+
System.out.println(" Improvement: " +
841+
(timeNoIndex > 0 ? String.format("%.1fx", timeNoIndex / (double) Math.max(1, timeWithIndex)) : "N/A"));
842+
843+
// Index should provide measurable improvement
844+
assertTrue("Index should improve performance or complete very quickly",
845+
timeWithIndex < timeNoIndex || timeWithIndex < 100);
846+
}
630847
}

0 commit comments

Comments
 (0)