Skip to content

Commit 372dd33

Browse files
schmidt-sebastianjeremyjiang-dev
schmidt-sebastian
authored and
jeremyjiang-dev
committed
Support > and != on the same field (#3454)
1 parent b4be657 commit 372dd33

File tree

4 files changed

+62
-33
lines changed

4 files changed

+62
-33
lines changed

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/Target.java‎

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414

1515
package com.google.firebase.firestore.core;
1616

17-
import static com.google.firebase.firestore.model.Values.MAX_VALUE;
18-
import static com.google.firebase.firestore.model.Values.MIN_VALUE;
1917
import static com.google.firebase.firestore.model.Values.max;
2018
import static com.google.firebase.firestore.model.Values.min;
2119

@@ -25,10 +23,11 @@
2523
import com.google.firebase.firestore.model.FieldPath;
2624
import com.google.firebase.firestore.model.ResourcePath;
2725
import com.google.firebase.firestore.model.Values;
28-
import com.google.firestore.v1.ArrayValue;
2926
import com.google.firestore.v1.Value;
3027
import java.util.ArrayList;
28+
import java.util.Collection;
3129
import java.util.Collections;
30+
import java.util.LinkedHashMap;
3231
import java.util.List;
3332

3433
/**
@@ -153,23 +152,21 @@ private List<FieldFilter> getFieldFiltersForPath(FieldPath path) {
153152
* Returns the list of values that are used in != or NOT_IN filters. Returns {@code null} if there
154153
* are no such filters.
155154
*/
156-
public @Nullable List<Value> getNotInValues(FieldIndex fieldIndex) {
157-
List<Value> values = new ArrayList<>();
158-
155+
public @Nullable Collection<Value> getNotInValues(FieldIndex fieldIndex) {
156+
LinkedHashMap<FieldPath, Value> values = new LinkedHashMap<>();
159157
for (FieldIndex.Segment segment : fieldIndex.getDirectionalSegments()) {
160158
for (FieldFilter fieldFilter : getFieldFiltersForPath(segment.getFieldPath())) {
161159
switch (fieldFilter.getOperator()) {
162160
case EQUAL:
163161
case IN:
164162
// Encode equality prefix, which is encoded in the index value before the inequality
165163
// (e.g. `a == 'a' && b != 'b'` is encoded to `value != 'ab'`).
166-
values.add(fieldFilter.getValue());
164+
values.put(segment.getFieldPath(), fieldFilter.getValue());
167165
break;
168166
case NOT_IN:
169167
case NOT_EQUAL:
170-
// NotIn/NotEqual is always a suffix
171-
values.add(fieldFilter.getValue());
172-
return values;
168+
values.put(segment.getFieldPath(), fieldFilter.getValue());
169+
return values.values(); // NotIn/NotEqual is always a suffix
173170
}
174171
}
175172
}
@@ -211,13 +208,8 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
211208
filterInclusive = false;
212209
break;
213210
case NOT_EQUAL:
214-
filterValue = Values.MIN_VALUE;
215-
break;
216211
case NOT_IN:
217-
filterValue =
218-
Value.newBuilder()
219-
.setArrayValue(ArrayValue.newBuilder().addValues(MIN_VALUE))
220-
.build();
212+
filterValue = Values.MIN_VALUE;
221213
break;
222214
default:
223215
// Remaining filters cannot be used as lower bounds.
@@ -291,13 +283,8 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
291283
filterInclusive = false;
292284
break;
293285
case NOT_EQUAL:
294-
filterValue = Values.MAX_VALUE;
295-
break;
296286
case NOT_IN:
297-
filterValue =
298-
Value.newBuilder()
299-
.setArrayValue(ArrayValue.newBuilder().addValues(MAX_VALUE))
300-
.build();
287+
filterValue = Values.MAX_VALUE;
301288
break;
302289
default:
303290
// Remaining filters cannot be used as upper bounds.

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteIndexManager.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
448448
}
449449

450450
@Nullable List<Value> arrayValues = subTarget.getArrayValues(fieldIndex);
451-
@Nullable List<Value> notInValues = subTarget.getNotInValues(fieldIndex);
451+
@Nullable Collection<Value> notInValues = subTarget.getNotInValues(fieldIndex);
452452
@Nullable Bound lowerBound = subTarget.getLowerBound(fieldIndex);
453453
@Nullable Bound upperBound = subTarget.getUpperBound(fieldIndex);
454454

@@ -564,7 +564,7 @@ private Object[] generateQueryAndBindings(
564564
Object[] bindArgs =
565565
fillBounds(statementCount, indexId, arrayValues, lowerBounds, upperBounds, notIn);
566566

567-
List<Object> result = new ArrayList<Object>();
567+
List<Object> result = new ArrayList<>();
568568
result.add(sql.toString());
569569
result.addAll(Arrays.asList(bindArgs));
570570
return result.toArray();
@@ -673,7 +673,7 @@ private byte[] encodeSingleElement(Value value) {
673673
* queries, a list of possible values is returned.
674674
*/
675675
private @Nullable Object[] encodeValues(
676-
FieldIndex fieldIndex, Target target, @Nullable List<Value> values) {
676+
FieldIndex fieldIndex, Target target, @Nullable Collection<Value> values) {
677677
if (values == null) return null;
678678

679679
List<IndexByteEncoder> encoders = new ArrayList<>();
@@ -740,8 +740,10 @@ private boolean isInFilter(Target target, FieldPath fieldPath) {
740740
for (Filter filter : target.getFilters()) {
741741
if ((filter instanceof FieldFilter) && ((FieldFilter) filter).getField().equals(fieldPath)) {
742742
FieldFilter.Operator operator = ((FieldFilter) filter).getOperator();
743-
return operator.equals(FieldFilter.Operator.IN)
744-
|| operator.equals(FieldFilter.Operator.NOT_IN);
743+
if (operator.equals(FieldFilter.Operator.IN)
744+
|| operator.equals(FieldFilter.Operator.NOT_IN)) {
745+
return true;
746+
}
745747
}
746748
}
747749
return false;

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/Values.java‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,15 @@ public class Values {
4141
public static final Value NULL_VALUE =
4242
Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build();
4343
public static final Value MIN_VALUE = NULL_VALUE;
44+
private static final Value MAX_VALUE_TYPE = Value.newBuilder().setStringValue("__max__").build();
4445
public static final Value MAX_VALUE =
4546
Value.newBuilder()
46-
.setMapValue(
47-
MapValue.newBuilder()
48-
.putFields("__type__", Value.newBuilder().setStringValue("__max__").build()))
47+
.setMapValue(MapValue.newBuilder().putFields("__type__", MAX_VALUE_TYPE))
4948
.build();
5049

5150
/**
5251
* The order of types in Firestore. This order is based on the backend's ordering, but modified to
53-
* support server timestamps.
52+
* support server timestamps and {@link #MAX_VALUE}.
5453
*/
5554
public static final int TYPE_ORDER_NULL = 0;
5655

@@ -65,6 +64,8 @@ public class Values {
6564
public static final int TYPE_ORDER_ARRAY = 9;
6665
public static final int TYPE_ORDER_MAP = 10;
6766

67+
public static final int TYPE_ORDER_MAX_VALUE = Integer.MAX_VALUE;
68+
6869
/** Returns the backend's type order of the given Value type. */
6970
public static int typeOrder(Value value) {
7071
switch (value.getValueTypeCase()) {
@@ -91,8 +92,11 @@ public static int typeOrder(Value value) {
9192
case MAP_VALUE:
9293
if (isServerTimestamp(value)) {
9394
return TYPE_ORDER_SERVER_TIMESTAMP;
95+
} else if (isMaxValue(value)) {
96+
return TYPE_ORDER_MAX_VALUE;
97+
} else {
98+
return TYPE_ORDER_MAP;
9499
}
95-
return TYPE_ORDER_MAP;
96100
default:
97101
throw fail("Invalid value type: " + value.getValueTypeCase());
98102
}
@@ -122,6 +126,8 @@ public static boolean equals(Value left, Value right) {
122126
return objectEquals(left, right);
123127
case TYPE_ORDER_SERVER_TIMESTAMP:
124128
return getLocalWriteTime(left).equals(getLocalWriteTime(right));
129+
case TYPE_ORDER_MAX_VALUE:
130+
return true;
125131
default:
126132
return left.equals(right);
127133
}
@@ -195,6 +201,7 @@ public static int compare(Value left, Value right) {
195201

196202
switch (leftType) {
197203
case TYPE_ORDER_NULL:
204+
case TYPE_ORDER_MAX_VALUE:
198205
return 0;
199206
case TYPE_ORDER_BOOLEAN:
200207
return Util.compareBooleans(left.getBooleanValue(), right.getBooleanValue());
@@ -531,6 +538,6 @@ public static Value getUpperBound(Value.ValueTypeCase valueTypeCase) {
531538

532539
/** Returns true if the Value represents the canonical {@link #MAX_VALUE} . */
533540
public static boolean isMaxValue(Value value) {
534-
return equals(value, MAX_VALUE);
541+
return MAX_VALUE_TYPE.equals(value.getMapValue().getFieldsMap().get("__type__"));
535542
}
536543
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,19 @@ public void testEqualsWithNotEqualsFilter() {
137137
verifyResults(query, "coll/val2");
138138
}
139139

140+
@Test
141+
public void testEqualsWithNotEqualsFilterSameField() {
142+
setUpSingleValueFilter();
143+
Query query = query("coll").filter(filter("count", ">", 1)).filter(filter("count", "!=", 2));
144+
verifyResults(query, "coll/val3");
145+
146+
query = query("coll").filter(filter("count", "==", 1)).filter(filter("count", "!=", 2));
147+
verifyResults(query, "coll/val1");
148+
149+
query = query("coll").filter(filter("count", "==", 1)).filter(filter("count", "!=", 1));
150+
verifyResults(query);
151+
}
152+
140153
@Test
141154
public void testLessThanFilter() {
142155
setUpSingleValueFilter();
@@ -227,6 +240,26 @@ public void testNotInFilter() {
227240
verifyResults(query, "coll/val3");
228241
}
229242

243+
@Test
244+
public void testNotInWithGreaterThanFilter() {
245+
setUpSingleValueFilter();
246+
Query query =
247+
query("coll")
248+
.filter(filter("count", ">", 1))
249+
.filter(filter("count", "not-in", Collections.singletonList(2)));
250+
verifyResults(query, "coll/val3");
251+
}
252+
253+
@Test
254+
public void testOutOfBoundsNotInWithGreaterThanFilter() {
255+
setUpSingleValueFilter();
256+
Query query =
257+
query("coll")
258+
.filter(filter("count", ">", 2))
259+
.filter(filter("count", "not-in", Collections.singletonList(1)));
260+
verifyResults(query, "coll/val3");
261+
}
262+
230263
@Test
231264
public void testArrayContainsFilter() {
232265
setUpArrayValueFilter();

0 commit comments

Comments
 (0)