Skip to content

Commit 4a62633

Browse files
authored
fix: Fields used in whereIn should be equality filters (#216)
* fix: Fields used in whereIn should be equality filters * fix: proper naming.
1 parent 81c20c5 commit 4a62633

File tree

2 files changed

+42
-6
lines changed
  • google-cloud-firestore/src

2 files changed

+42
-6
lines changed

google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ Value encodeValue() {
104104
return encodedValue;
105105
}
106106

107-
abstract boolean isEqualsFilter();
107+
abstract boolean isInequalityFilter();
108108

109109
abstract Filter toProto();
110110
}
@@ -117,8 +117,8 @@ private static class UnaryFilter extends FieldFilter {
117117
}
118118

119119
@Override
120-
boolean isEqualsFilter() {
121-
return true;
120+
boolean isInequalityFilter() {
121+
return false;
122122
}
123123

124124
Filter toProto() {
@@ -148,8 +148,11 @@ private static class ComparisonFilter extends FieldFilter {
148148
}
149149

150150
@Override
151-
boolean isEqualsFilter() {
152-
return operator.equals(EQUAL);
151+
boolean isInequalityFilter() {
152+
return operator.equals(GREATER_THAN)
153+
|| operator.equals(GREATER_THAN_OR_EQUAL)
154+
|| operator.equals(LESS_THAN)
155+
|| operator.equals(LESS_THAN_OR_EQUAL);
153156
}
154157

155158
Filter toProto() {
@@ -306,7 +309,7 @@ private ImmutableList<FieldOrder> createImplicitOrderBy() {
306309
if (implicitOrders.isEmpty()) {
307310
// If no explicit ordering is specified, use the first inequality to define an implicit order.
308311
for (FieldFilter fieldFilter : options.getFieldFilters()) {
309-
if (!fieldFilter.isEqualsFilter()) {
312+
if (fieldFilter.isInequalityFilter()) {
310313
implicitOrders.add(new FieldOrder(fieldFilter.fieldPath, Direction.ASCENDING));
311314
break;
312315
}

google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,39 @@ public void inQueriesWithReferenceArray() throws Exception {
303303
assertEquals(expectedRequest, runQuery.getValue());
304304
}
305305

306+
@Test
307+
public void inQueriesFieldsNotUsedInOrderBy() throws Exception {
308+
doAnswer(queryResponse())
309+
.when(firestoreMock)
310+
.streamRequest(
311+
runQuery.capture(),
312+
streamObserverCapture.capture(),
313+
Matchers.<ServerStreamingCallable>any());
314+
315+
// Field "foo" used in `whereIn` should not appear in implicit orderBys in the resulting query.
316+
query
317+
.whereIn(FieldPath.of("foo"), Arrays.<Object>asList("value1", "value2"))
318+
.startAt(SINGLE_FIELD_SNAPSHOT)
319+
.get()
320+
.get();
321+
322+
Value value =
323+
Value.newBuilder()
324+
.setArrayValue(
325+
ArrayValue.newBuilder()
326+
.addValues(Value.newBuilder().setStringValue("value1").build())
327+
.addValues(Value.newBuilder().setStringValue("value2").build())
328+
.build())
329+
.build();
330+
RunQueryRequest expectedRequest =
331+
query(
332+
filter(Operator.IN, "foo", value),
333+
order("__name__", Direction.ASCENDING),
334+
startAt(reference(DOCUMENT_NAME), true));
335+
336+
assertEquals(expectedRequest, runQuery.getValue());
337+
}
338+
306339
@Test
307340
public void validatesInQueries() {
308341
try {

0 commit comments

Comments
 (0)