Skip to content

Commit 2d2e008

Browse files
authored
Fix equality and inequality on the same field (#5034)
1 parent e189eed commit 2d2e008

File tree

3 files changed

+103
-10
lines changed

3 files changed

+103
-10
lines changed

firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
import com.google.firebase.firestore.core.OrderBy;
2323
import com.google.firebase.firestore.core.Target;
2424
import java.util.ArrayList;
25+
import java.util.HashSet;
2526
import java.util.Iterator;
2627
import java.util.List;
28+
import java.util.Set;
2729

2830
/**
2931
* A light query planner for Firestore.
@@ -144,14 +146,14 @@ public boolean servedByIndex(FieldIndex index) {
144146
List<FieldIndex.Segment> segments = index.getDirectionalSegments();
145147
int segmentIndex = 0;
146148

149+
Set<String> equalitySegments = new HashSet<>();
147150
// Process all equalities first. Equalities can appear out of order.
148151
for (; segmentIndex < segments.size(); ++segmentIndex) {
149152
// We attempt to greedily match all segments to equality filters. If a filter matches an
150-
// index segment, we can mark the segment as used. Since it is not possible to use the same
151-
// field path in both an equality and inequality/oderBy clause, we do not have to consider the
152-
// possibility that a matching equality segment should instead be used to map to an inequality
153-
// filter or orderBy clause.
154-
if (!hasMatchingEqualityFilter(segments.get(segmentIndex))) {
153+
// index segment, we can mark the segment as used.
154+
if (hasMatchingEqualityFilter(segments.get(segmentIndex))) {
155+
equalitySegments.add(segments.get(segmentIndex).getFieldPath().canonicalString());
156+
} else {
155157
// If we cannot find a matching filter, we need to verify whether the remaining segments map
156158
// to the target's inequality and its orderBy clauses.
157159
break;
@@ -165,13 +167,17 @@ public boolean servedByIndex(FieldIndex index) {
165167
return true;
166168
}
167169

168-
// If there is an inequality filter, the next segment must match both the filter and the first
169-
// orderBy clause.
170170
if (inequalityFilter != null) {
171-
FieldIndex.Segment segment = segments.get(segmentIndex);
172-
if (!matchesFilter(inequalityFilter, segment) || !matchesOrderBy(orderBys.next(), segment)) {
173-
return false;
171+
// If there is an inequality filter and the field was not in one of the equality filters
172+
// above, the next segment must match both the filter and the first orderBy clause.
173+
if (!equalitySegments.contains(inequalityFilter.getField().canonicalString())) {
174+
FieldIndex.Segment segment = segments.get(segmentIndex);
175+
if (!matchesFilter(inequalityFilter, segment)
176+
|| !matchesOrderBy(orderBys.next(), segment)) {
177+
return false;
178+
}
174179
}
180+
175181
++segmentIndex;
176182
}
177183

firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,38 @@ public void testCursorsDoNoExpandResultSet() {
624624
verifyResults(query, "coll/val2");
625625
}
626626

627+
@Test
628+
public void testFiltersOnTheSameField() {
629+
indexManager.addFieldIndex(fieldIndex("coll", "a", Kind.ASCENDING));
630+
631+
addDoc("coll/val1", map("a", 1, "b", 1));
632+
addDoc("coll/val2", map("a", 2, "b", 2));
633+
addDoc("coll/val3", map("a", 3, "b", 3));
634+
addDoc("coll/val4", map("a", 4, "b", 4));
635+
636+
Query query = query("coll").filter(filter("a", ">", 1)).filter(filter("a", "==", 2));
637+
verifyResults(query, "coll/val2");
638+
639+
query = query("coll").filter(filter("a", "<=", 1)).filter(filter("a", "==", 2));
640+
verifyResults(query);
641+
642+
query =
643+
query("coll")
644+
.filter(filter("a", ">", 1))
645+
.filter(filter("a", "==", 2))
646+
.orderBy(orderBy("a"))
647+
.orderBy(orderBy(DocumentKey.KEY_FIELD_NAME));
648+
verifyResults(query, "coll/val2");
649+
650+
query =
651+
query("coll")
652+
.filter(filter("a", ">", 1))
653+
.filter(filter("a", "==", 2))
654+
.orderBy(orderBy("a"))
655+
.orderBy(orderBy(DocumentKey.KEY_FIELD_NAME, "desc"));
656+
verifyResults(query, "coll/val2");
657+
}
658+
627659
@Test
628660
public void testAdvancedQueries() {
629661
// This test compares local query results with those received from the Java Server SDK.

firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,61 @@ public void withInAndOrderBySameField() {
565565
validateServesTarget(q, "a", FieldIndex.Segment.Kind.ASCENDING);
566566
}
567567

568+
@Test
569+
public void withEqualityAndInequalityOnTheSameField() {
570+
validateServesTarget(
571+
query("collId").filter(filter("a", ">=", 5)).filter(filter("a", "==", 0)),
572+
"a",
573+
FieldIndex.Segment.Kind.ASCENDING);
574+
575+
validateServesTarget(
576+
query("collId")
577+
.filter(filter("a", ">=", 5))
578+
.filter(filter("a", "==", 0))
579+
.orderBy(orderBy("a")),
580+
"a",
581+
FieldIndex.Segment.Kind.ASCENDING);
582+
583+
validateServesTarget(
584+
query("collId")
585+
.filter(filter("a", ">=", 5))
586+
.filter(filter("a", "==", 0))
587+
.orderBy(orderBy("a"))
588+
.orderBy(orderBy(DocumentKey.KEY_FIELD_NAME)),
589+
"a",
590+
FieldIndex.Segment.Kind.ASCENDING);
591+
592+
validateServesTarget(
593+
query("collId")
594+
.filter(filter("a", ">=", 5))
595+
.filter(filter("a", "==", 0))
596+
.orderBy(orderBy("a"))
597+
.orderBy(orderBy(DocumentKey.KEY_FIELD_NAME, "desc")),
598+
"a",
599+
FieldIndex.Segment.Kind.ASCENDING);
600+
601+
validateServesTarget(
602+
query("collId")
603+
.filter(filter("a", ">=", 5))
604+
.filter(filter("a", "==", 0))
605+
.filter(filter("b", "==", 0))
606+
.orderBy(orderBy("a", "desc"))
607+
.orderBy(orderBy(DocumentKey.KEY_FIELD_NAME, "desc")),
608+
"a",
609+
FieldIndex.Segment.Kind.ASCENDING,
610+
"b",
611+
FieldIndex.Segment.Kind.ASCENDING);
612+
613+
validateServesTarget(
614+
query("collId")
615+
.filter(filter("a", ">=", 5))
616+
.filter(filter("a", "==", 0))
617+
.orderBy(orderBy("a", "desc"))
618+
.orderBy(orderBy(DocumentKey.KEY_FIELD_NAME, "desc")),
619+
"a",
620+
FieldIndex.Segment.Kind.DESCENDING);
621+
}
622+
568623
private void validateServesTarget(
569624
Query query, String field, FieldIndex.Segment.Kind kind, Object... fieldsAndKind) {
570625
FieldIndex expectedIndex = fieldIndex("collId", field, kind, fieldsAndKind);

0 commit comments

Comments
 (0)