Skip to content

Commit 60e8538

Browse files
wu-huiwu-hui
authored
Fix an issue where TargetIndexMatcher rejects valid queries (#7335)
1 parent e642fc7 commit 60e8538

File tree

3 files changed

+155
-11
lines changed

3 files changed

+155
-11
lines changed

‎packages/firestore/src/model/target_index_matcher.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -118,18 +118,19 @@ export class TargetIndexMatcher {
118118
}
119119

120120
const segments = fieldIndexGetDirectionalSegments(index);
121+
let equalitySegments = new Set<string>();
121122
let segmentIndex = 0;
122123
let orderBysIndex = 0;
123124

124125
// Process all equalities first. Equalities can appear out of order.
125126
for (; segmentIndex < segments.length; ++segmentIndex) {
126127
// We attempt to greedily match all segments to equality filters. If a
127128
// filter matches an index segment, we can mark the segment as used.
128-
// Since it is not possible to use the same field path in both an equality
129-
// and inequality/oderBy clause, we do not have to consider the possibility
130-
// that a matching equality segment should instead be used to map to an
131-
// inequality filter or orderBy clause.
132-
if (!this.hasMatchingEqualityFilter(segments[segmentIndex])) {
129+
if (this.hasMatchingEqualityFilter(segments[segmentIndex])) {
130+
equalitySegments = equalitySegments.add(
131+
segments[segmentIndex].fieldPath.canonicalString()
132+
);
133+
} else {
133134
// If we cannot find a matching filter, we need to verify whether the
134135
// remaining segments map to the target's inequality and its orderBy
135136
// clauses.
@@ -144,16 +145,22 @@ export class TargetIndexMatcher {
144145
return true;
145146
}
146147

147-
// If there is an inequality filter, the next segment must match both the
148-
// filter and the first orderBy clause.
149148
if (this.inequalityFilter !== undefined) {
150-
const segment = segments[segmentIndex];
149+
// If there is an inequality filter and the field was not in one of the
150+
// equality filters above, the next segment must match both the filter
151+
// and the first orderBy clause.
151152
if (
152-
!this.matchesFilter(this.inequalityFilter, segment) ||
153-
!this.matchesOrderBy(this.orderBys[orderBysIndex++], segment)
153+
!equalitySegments.has(this.inequalityFilter.field.canonicalString())
154154
) {
155-
return false;
155+
const segment = segments[segmentIndex];
156+
if (
157+
!this.matchesFilter(this.inequalityFilter, segment) ||
158+
!this.matchesOrderBy(this.orderBys[orderBysIndex++], segment)
159+
) {
160+
return false;
161+
}
156162
}
163+
157164
++segmentIndex;
158165
}
159166

‎packages/firestore/test/unit/local/index_manager.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -939,6 +939,69 @@ describe('IndexedDbIndexManager', async () => {
939939
await verifyResults(testingQuery, 'coll/val2');
940940
});
941941

942+
it('can have filters on the same field', async () => {
943+
await indexManager.addFieldIndex(
944+
fieldIndex('coll', { fields: [['a', IndexKind.ASCENDING]] })
945+
);
946+
await indexManager.addFieldIndex(
947+
fieldIndex('coll', {
948+
fields: [
949+
['a', IndexKind.ASCENDING],
950+
['b', IndexKind.ASCENDING]
951+
]
952+
})
953+
);
954+
await addDoc('coll/val1', { 'a': 1, 'b': 1 });
955+
await addDoc('coll/val2', { 'a': 2, 'b': 2 });
956+
await addDoc('coll/val3', { 'a': 3, 'b': 3 });
957+
await addDoc('coll/val4', { 'a': 4, 'b': 4 });
958+
959+
let testingQuery = queryWithAddedFilter(
960+
queryWithAddedFilter(query('coll'), filter('a', '>', 1)),
961+
filter('a', '==', 2)
962+
);
963+
await verifyResults(testingQuery, 'coll/val2');
964+
965+
testingQuery = queryWithAddedFilter(
966+
queryWithAddedFilter(query('coll'), filter('a', '<=', 1)),
967+
filter('a', '==', 2)
968+
);
969+
await verifyResults(testingQuery);
970+
971+
testingQuery = queryWithAddedOrderBy(
972+
queryWithAddedFilter(
973+
queryWithAddedFilter(query('coll'), filter('a', '>', 1)),
974+
filter('a', '==', 2)
975+
),
976+
orderBy('a')
977+
);
978+
await verifyResults(testingQuery, 'coll/val2');
979+
980+
testingQuery = queryWithAddedOrderBy(
981+
queryWithAddedOrderBy(
982+
queryWithAddedFilter(
983+
queryWithAddedFilter(query('coll'), filter('a', '>', 1)),
984+
filter('a', '==', 2)
985+
),
986+
orderBy('a')
987+
),
988+
orderBy('__name__', 'desc')
989+
);
990+
await verifyResults(testingQuery, 'coll/val2');
991+
992+
testingQuery = queryWithAddedOrderBy(
993+
queryWithAddedOrderBy(
994+
queryWithAddedFilter(
995+
queryWithAddedFilter(query('coll'), filter('a', '>', 1)),
996+
filter('a', '==', 3)
997+
),
998+
orderBy('a')
999+
),
1000+
orderBy('b', 'desc')
1001+
);
1002+
await verifyResults(testingQuery, 'coll/val3');
1003+
});
1004+
9421005
it('support advances queries', async () => {
9431006
// This test compares local query results with those received from the Java
9441007
// Server SDK.

‎packages/firestore/test/unit/model/target_index_matcher.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,80 @@ describe('Target Bounds', () => {
639639
validateServesTarget(q, 'a', IndexKind.ASCENDING);
640640
});
641641

642+
it('with equality and inequality on the same field', () => {
643+
let q = queryWithAddedFilter(
644+
queryWithAddedFilter(query('collId'), filter('a', '>=', 5)),
645+
filter('a', '==', 0)
646+
);
647+
validateServesTarget(q, 'a', IndexKind.ASCENDING);
648+
649+
q = queryWithAddedOrderBy(
650+
queryWithAddedFilter(
651+
queryWithAddedFilter(query('collId'), filter('a', '>=', 5)),
652+
filter('a', '==', 0)
653+
),
654+
orderBy('a')
655+
);
656+
validateServesTarget(q, 'a', IndexKind.ASCENDING);
657+
658+
q = queryWithAddedOrderBy(
659+
queryWithAddedOrderBy(
660+
queryWithAddedFilter(
661+
queryWithAddedFilter(query('collId'), filter('a', '>=', 5)),
662+
filter('a', '==', 0)
663+
),
664+
orderBy('a')
665+
),
666+
orderBy('__name__')
667+
);
668+
validateServesTarget(q, 'a', IndexKind.ASCENDING);
669+
670+
q = queryWithAddedOrderBy(
671+
queryWithAddedOrderBy(
672+
queryWithAddedFilter(
673+
queryWithAddedFilter(query('collId'), filter('a', '>=', 5)),
674+
filter('a', '==', 0)
675+
),
676+
orderBy('a')
677+
),
678+
orderBy('__name__', 'desc')
679+
);
680+
validateServesTarget(q, 'a', IndexKind.ASCENDING);
681+
682+
q = queryWithAddedOrderBy(
683+
queryWithAddedOrderBy(
684+
queryWithAddedOrderBy(
685+
queryWithAddedFilter(
686+
queryWithAddedFilter(query('collId'), filter('a', '>=', 5)),
687+
filter('a', '==', 0)
688+
),
689+
orderBy('a')
690+
),
691+
orderBy('b')
692+
),
693+
orderBy('__name__', 'desc')
694+
);
695+
validateServesTarget(
696+
q,
697+
'a',
698+
IndexKind.ASCENDING,
699+
'b',
700+
IndexKind.DESCENDING
701+
);
702+
703+
q = queryWithAddedOrderBy(
704+
queryWithAddedOrderBy(
705+
queryWithAddedFilter(
706+
queryWithAddedFilter(query('collId'), filter('a', '>=', 5)),
707+
filter('a', '==', 0)
708+
),
709+
orderBy('a')
710+
),
711+
orderBy('__name__', 'desc')
712+
);
713+
validateServesTarget(q, 'a', IndexKind.ASCENDING);
714+
});
715+
642716
function validateServesTarget(
643717
query: Query,
644718
field: string,

0 commit comments

Comments
 (0)