Skip to content

Commit c30a15a

Browse files
schmidt-sebastianschmidt-sebastian
authored
Tighten the query bounds (#2979)
1 parent da62838 commit c30a15a

File tree

7 files changed

+277
-65
lines changed

7 files changed

+277
-65
lines changed

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

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

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

17+
import static com.google.firebase.firestore.model.Values.max;
18+
import static com.google.firebase.firestore.model.Values.min;
19+
1720
import androidx.annotation.Nullable;
1821
import com.google.firebase.firestore.core.OrderBy.Direction;
1922
import com.google.firebase.firestore.model.DocumentKey;
@@ -121,25 +124,29 @@ public boolean hasLimit() {
121124
*/
122125
public Bound getLowerBound(FieldIndex fieldIndex) {
123126
List<Value> values = new ArrayList<>();
124-
boolean before = true;
127+
boolean inclusive = true;
125128

126129
// Go through all filters to find a value for the current field segment
127130
for (FieldIndex.Segment segment : fieldIndex) {
128-
Value lowestValue = Values.NULL_VALUE;
131+
Value segmentValue = Values.NULL_VALUE;
132+
boolean segmentInclusive = true;
133+
129134
for (Filter filter : filters) {
130135
if (filter.getField().equals(segment.getFieldPath())) {
131136
FieldFilter fieldFilter = (FieldFilter) filter;
137+
Value filterValue = null;
138+
boolean filterInclusive = true;
139+
132140
switch (fieldFilter.getOperator()) {
133141
case LESS_THAN:
134142
case LESS_THAN_OR_EQUAL:
135-
// TODO(indexing): Implement type clamping. Only field values with the same type
136-
// should match the query.
143+
filterValue = Values.getLowerBound(fieldFilter.getValue().getValueTypeCase());
137144
break;
138145
case NOT_EQUAL:
139-
// These filters cannot be used as a lower bound. Skip.
146+
filterValue = Values.NULL_VALUE;
140147
break;
141148
case NOT_IN:
142-
lowestValue =
149+
filterValue =
143150
Value.newBuilder()
144151
.setArrayValue(ArrayValue.newBuilder().addValues(Values.NULL_VALUE))
145152
.build();
@@ -149,13 +156,18 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
149156
case ARRAY_CONTAINS_ANY:
150157
case ARRAY_CONTAINS:
151158
case GREATER_THAN_OR_EQUAL:
152-
lowestValue = fieldFilter.getValue();
159+
filterValue = fieldFilter.getValue();
153160
break;
154161
case GREATER_THAN:
155-
lowestValue = fieldFilter.getValue();
156-
before = false;
162+
filterValue = fieldFilter.getValue();
163+
filterInclusive = false;
157164
break;
158165
}
166+
167+
if (max(segmentValue, filterValue) == filterValue) {
168+
segmentValue = filterValue;
169+
segmentInclusive = filterInclusive;
170+
}
159171
}
160172
}
161173

@@ -166,20 +178,20 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
166178
OrderBy orderBy = this.orderBys.get(i);
167179
if (orderBy.getField().equals(segment.getFieldPath())) {
168180
Value cursorValue = startAt.getPosition().get(i);
169-
if (Values.compare(lowestValue, cursorValue) <= 0) {
170-
lowestValue = cursorValue;
171-
// `before` is shared by all cursor values. If any cursor value is used, we set before
172-
// to the cursor's value.
173-
before = startAt.isBefore();
181+
if (max(segmentValue, cursorValue) == cursorValue) {
182+
segmentValue = cursorValue;
183+
segmentInclusive = startAt.isBefore();
174184
}
175185
break;
176186
}
177187
}
178188
}
179-
values.add(lowestValue);
189+
190+
values.add(segmentValue);
191+
inclusive &= segmentInclusive;
180192
}
181193

182-
return new Bound(values, before);
194+
return new Bound(values, inclusive);
183195
}
184196

185197
/**
@@ -192,38 +204,46 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
192204
*/
193205
public @Nullable Bound getUpperBound(FieldIndex fieldIndex) {
194206
List<Value> values = new ArrayList<>();
195-
boolean before = false;
207+
boolean inclusive = true;
196208

197209
for (FieldIndex.Segment segment : fieldIndex) {
198-
@Nullable Value largestValue = null;
210+
@Nullable Value segmentValue = null;
211+
boolean segmentInclusive = true;
199212

200213
// Go through all filters to find a value for the current field segment
201214
for (Filter filter : filters) {
202215
if (filter.getField().equals(segment.getFieldPath())) {
203216
FieldFilter fieldFilter = (FieldFilter) filter;
217+
Value filterValue = null;
218+
boolean filterInclusive = true;
219+
204220
switch (fieldFilter.getOperator()) {
205221
case NOT_IN:
206222
case NOT_EQUAL:
207223
// These filters cannot be used as an upper bound. Skip.
208224
break;
209225
case GREATER_THAN_OR_EQUAL:
210226
case GREATER_THAN:
211-
// TODO(indexing): Implement type clamping. Only field values with the same type
212-
// should match the query.
227+
filterValue = Values.getUpperBound(fieldFilter.getValue().getValueTypeCase());
228+
filterInclusive = false;
213229
break;
214230
case EQUAL:
215231
case IN:
216232
case ARRAY_CONTAINS_ANY:
217233
case ARRAY_CONTAINS:
218234
case LESS_THAN_OR_EQUAL:
219-
largestValue = fieldFilter.getValue();
220-
before = false;
235+
filterValue = fieldFilter.getValue();
221236
break;
222237
case LESS_THAN:
223-
largestValue = fieldFilter.getValue();
224-
before = true;
238+
filterValue = fieldFilter.getValue();
239+
filterInclusive = false;
225240
break;
226241
}
242+
243+
if (min(segmentValue, filterValue) == filterValue) {
244+
segmentValue = filterValue;
245+
segmentInclusive = filterInclusive;
246+
}
227247
}
228248
}
229249

@@ -234,28 +254,29 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
234254
OrderBy orderBy = this.orderBys.get(i);
235255
if (orderBy.getField().equals(segment.getFieldPath())) {
236256
Value cursorValue = endAt.getPosition().get(i);
237-
if (largestValue == null || Values.compare(largestValue, cursorValue) > 0) {
238-
largestValue = cursorValue;
239-
before = endAt.isBefore();
257+
if (min(segmentValue, cursorValue) == cursorValue) {
258+
segmentValue = cursorValue;
259+
segmentInclusive = !endAt.isBefore();
240260
}
241261
break;
242262
}
243263
}
244264
}
245265

246-
if (largestValue == null) {
266+
if (segmentValue == null) {
247267
// No upper bound exists
248268
return null;
249269
}
250270

251-
values.add(largestValue);
271+
values.add(segmentValue);
272+
inclusive &= segmentInclusive;
252273
}
253274

254275
if (values.isEmpty()) {
255276
return null;
256277
}
257278

258-
return new Bound(values, before);
279+
return new Bound(values, !inclusive);
259280
}
260281

261282
public List<OrderBy> getOrderBy() {

‎firebase-firestore/src/main/java/com/google/firebase/firestore/index/FirestoreIndexValueWriter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public class FirestoreIndexValueWriter {
3434
public static final int INDEX_TYPE_BOOLEAN = 10;
3535
public static final int INDEX_TYPE_NAN = 13;
3636
public static final int INDEX_TYPE_NUMBER = 15;
37-
public static final int INDEX_TYPE_TIMESTAMP = 10;
37+
public static final int INDEX_TYPE_TIMESTAMP = 20;
3838
public static final int INDEX_TYPE_STRING = 25;
3939
public static final int INDEX_TYPE_BLOB = 30;
4040
public static final int INDEX_TYPE_REFERENCE = 37;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
206206

207207
// TODO(indexing): To avoid reading the same documents multiple times, we should ideally only
208208
// send one query that combines all clauses.
209+
// TODO(indexing): Add limit handling
209210
for (int i = 0; i < lowerBoundValues.size(); ++i) {
210211
db.query(
211212
String.format(

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
/** Represents a particular database in Firestore */
2222
public final class DatabaseId implements Comparable<DatabaseId> {
2323
public static final String DEFAULT_DATABASE_ID = "(default)";
24+
public static final DatabaseId EMPTY = DatabaseId.forDatabase("", "");
2425

2526
public static DatabaseId forProject(String projectId) {
2627
return forDatabase(projectId, DEFAULT_DATABASE_ID);

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.firestore.v1.ArrayValueOrBuilder;
2626
import com.google.firestore.v1.MapValue;
2727
import com.google.firestore.v1.Value;
28+
import com.google.protobuf.ByteString;
2829
import com.google.protobuf.NullValue;
2930
import com.google.protobuf.Timestamp;
3031
import com.google.type.LatLng;
@@ -211,6 +212,30 @@ public static int compare(Value left, Value right) {
211212
}
212213
}
213214

215+
public static @Nullable Value max(@Nullable Value left, @Nullable Value right) {
216+
if (left == null && right == null) {
217+
return null;
218+
} else if (left == null) {
219+
return right;
220+
} else if (right == null) {
221+
return left;
222+
} else {
223+
return compare(left, right) > 0 ? left : right;
224+
}
225+
}
226+
227+
public static @Nullable Value min(@Nullable Value left, @Nullable Value right) {
228+
if (left == null && right == null) {
229+
return null;
230+
} else if (left == null) {
231+
return right;
232+
} else if (right == null) {
233+
return left;
234+
} else {
235+
return compare(left, right) < 0 ? left : right;
236+
}
237+
}
238+
214239
private static int compareNumbers(Value left, Value right) {
215240
if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
216241
double leftDouble = left.getDoubleValue();
@@ -432,4 +457,69 @@ public static Value refValue(DatabaseId databaseId, DocumentKey key) {
432457
.build();
433458
return value;
434459
}
460+
461+
/** Returns the lowest value for the given value type (inclusive). */
462+
public static Value getLowerBound(Value.ValueTypeCase valueTypeCase) {
463+
switch (valueTypeCase) {
464+
case NULL_VALUE:
465+
return Values.NULL_VALUE;
466+
case BOOLEAN_VALUE:
467+
return Value.newBuilder().setBooleanValue(false).build();
468+
case INTEGER_VALUE:
469+
case DOUBLE_VALUE:
470+
return Value.newBuilder().setDoubleValue(Double.NaN).build();
471+
case TIMESTAMP_VALUE:
472+
return Value.newBuilder()
473+
.setTimestampValue(Timestamp.newBuilder().setSeconds(Long.MIN_VALUE))
474+
.build();
475+
case STRING_VALUE:
476+
return Value.newBuilder().setStringValue("").build();
477+
case BYTES_VALUE:
478+
return Value.newBuilder().setBytesValue(ByteString.EMPTY).build();
479+
case REFERENCE_VALUE:
480+
return refValue(DatabaseId.EMPTY, DocumentKey.empty());
481+
case GEO_POINT_VALUE:
482+
return Value.newBuilder()
483+
.setGeoPointValue(LatLng.newBuilder().setLatitude(-90.0).setLongitude(-180.0))
484+
.build();
485+
case ARRAY_VALUE:
486+
return Value.newBuilder().setArrayValue(ArrayValue.getDefaultInstance()).build();
487+
case MAP_VALUE:
488+
return Value.newBuilder().setMapValue(MapValue.getDefaultInstance()).build();
489+
default:
490+
throw new IllegalArgumentException("Unknown value type: " + valueTypeCase);
491+
}
492+
}
493+
494+
/**
495+
* Returns the largest value for the given value type (exclusive). Returns {@code null} for maps.
496+
*/
497+
public static @Nullable Value getUpperBound(Value.ValueTypeCase valueTypeCase) {
498+
switch (valueTypeCase) {
499+
case NULL_VALUE:
500+
return getLowerBound(Value.ValueTypeCase.BOOLEAN_VALUE);
501+
case BOOLEAN_VALUE:
502+
return getLowerBound(Value.ValueTypeCase.INTEGER_VALUE);
503+
case INTEGER_VALUE:
504+
case DOUBLE_VALUE:
505+
return getLowerBound(Value.ValueTypeCase.TIMESTAMP_VALUE);
506+
case TIMESTAMP_VALUE:
507+
return getLowerBound(Value.ValueTypeCase.STRING_VALUE);
508+
case STRING_VALUE:
509+
return getLowerBound(Value.ValueTypeCase.BYTES_VALUE);
510+
case BYTES_VALUE:
511+
return getLowerBound(Value.ValueTypeCase.REFERENCE_VALUE);
512+
case REFERENCE_VALUE:
513+
return getLowerBound(Value.ValueTypeCase.GEO_POINT_VALUE);
514+
case GEO_POINT_VALUE:
515+
return getLowerBound(Value.ValueTypeCase.ARRAY_VALUE);
516+
case ARRAY_VALUE:
517+
return getLowerBound(Value.ValueTypeCase.MAP_VALUE);
518+
case MAP_VALUE:
519+
// There is no type that sorts higher than a map.
520+
return null;
521+
default:
522+
throw new IllegalArgumentException("Unknown value type: " + valueTypeCase);
523+
}
524+
}
435525
}

0 commit comments

Comments
 (0)