Skip to content

Commit 0679f5e

Browse files
schmidt-sebastianschmidt-sebastian
committed
Merge
2 parents b7be296 + adbac78 commit 0679f5e

File tree

7 files changed

+111
-80
lines changed

7 files changed

+111
-80
lines changed

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

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

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

17-
import static com.google.firebase.firestore.model.Values.getFirstValue;
17+
import static com.google.firebase.firestore.model.Values.getLowestValue;
1818
import static com.google.firebase.firestore.model.Values.getNextValue;
1919
import static com.google.firebase.firestore.model.Values.max;
2020
import static com.google.firebase.firestore.model.Values.min;
@@ -126,28 +126,29 @@ public boolean hasLimit() {
126126
*/
127127
public Bound getLowerBound(FieldIndex fieldIndex) {
128128
List<Value> values = new ArrayList<>();
129-
boolean before = true;
129+
boolean inclusive = true;
130130

131131
// Go through all filters to find a value for the current field segment
132132
for (FieldIndex.Segment segment : fieldIndex) {
133-
Value lowestValue = Values.NULL_VALUE;
133+
Value segmentValue = Values.NULL_VALUE;
134+
boolean segmentInclusive = true;
134135

135136
for (Filter filter : filters) {
136137
if (filter.getField().equals(segment.getFieldPath())) {
137138
FieldFilter fieldFilter = (FieldFilter) filter;
139+
Value filterValue = null;
140+
boolean filterInclusive = true;
138141

139-
Value newValue = null;
140-
boolean newBefore = true;
141142
switch (fieldFilter.getOperator()) {
142143
case LESS_THAN:
143144
case LESS_THAN_OR_EQUAL:
144-
newValue = getFirstValue(fieldFilter.getValue().getValueTypeCase());
145+
filterValue = getLowestValue(fieldFilter.getValue().getValueTypeCase());
145146
break;
146147
case NOT_EQUAL:
147-
newValue = Values.NULL_VALUE;
148+
filterValue = Values.NULL_VALUE;
148149
break;
149150
case NOT_IN:
150-
newValue =
151+
filterValue =
151152
Value.newBuilder()
152153
.setArrayValue(ArrayValue.newBuilder().addValues(Values.NULL_VALUE))
153154
.build();
@@ -157,17 +158,17 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
157158
case ARRAY_CONTAINS_ANY:
158159
case ARRAY_CONTAINS:
159160
case GREATER_THAN_OR_EQUAL:
160-
newValue = fieldFilter.getValue();
161+
filterValue = fieldFilter.getValue();
161162
break;
162163
case GREATER_THAN:
163-
newValue = fieldFilter.getValue();
164-
newBefore = false;
164+
filterValue = fieldFilter.getValue();
165+
filterInclusive = false;
165166
break;
166167
}
167168

168-
if (max(lowestValue, newValue) == newValue) {
169-
lowestValue = newValue;
170-
before = newBefore;
169+
if (max(segmentValue, filterValue) == filterValue) {
170+
segmentValue = filterValue;
171+
segmentInclusive = filterInclusive;
171172
}
172173
}
173174
}
@@ -179,21 +180,20 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
179180
OrderBy orderBy = this.orderBys.get(i);
180181
if (orderBy.getField().equals(segment.getFieldPath())) {
181182
Value cursorValue = startAt.getPosition().get(i);
182-
if (max(lowestValue, cursorValue) == cursorValue) {
183-
lowestValue = cursorValue;
184-
// `before` is shared by all cursor values. If any cursor value is used, we set before
185-
// to the cursor's value.
186-
before = startAt.isBefore();
183+
if (max(segmentValue, cursorValue) == cursorValue) {
184+
segmentValue = cursorValue;
185+
segmentInclusive = startAt.isBefore();
187186
}
188187
break;
189188
}
190189
}
191190
}
192191

193-
values.add(lowestValue);
192+
values.add(segmentValue);
193+
inclusive &= segmentInclusive;
194194
}
195195

196-
return new Bound(values, before);
196+
return new Bound(values, inclusive);
197197
}
198198

199199
/**
@@ -206,43 +206,45 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
206206
*/
207207
public @Nullable Bound getUpperBound(FieldIndex fieldIndex) {
208208
List<Value> values = new ArrayList<>();
209-
boolean before = false;
209+
boolean inclusive = true;
210210

211211
for (FieldIndex.Segment segment : fieldIndex) {
212-
@Nullable Value largestValue = null;
212+
@Nullable Value segmentValue = null;
213+
boolean segmentInclusive = true;
213214

214215
// Go through all filters to find a value for the current field segment
215216
for (Filter filter : filters) {
216217
if (filter.getField().equals(segment.getFieldPath())) {
217218
FieldFilter fieldFilter = (FieldFilter) filter;
219+
Value filterValue = null;
220+
boolean filterInclusive = true;
218221

219-
Value newValue = null;
220-
boolean newBefore = true;
221222
switch (fieldFilter.getOperator()) {
222223
case NOT_IN:
223224
case NOT_EQUAL:
224225
// These filters cannot be used as an upper bound. Skip.
225226
break;
226227
case GREATER_THAN_OR_EQUAL:
227228
case GREATER_THAN:
228-
newValue = getNextValue(fieldFilter.getValue().getValueTypeCase());
229+
filterValue = getNextValue(fieldFilter.getValue().getValueTypeCase());
230+
filterInclusive = false;
229231
break;
230232
case EQUAL:
231233
case IN:
232234
case ARRAY_CONTAINS_ANY:
233235
case ARRAY_CONTAINS:
234236
case LESS_THAN_OR_EQUAL:
235-
newValue = fieldFilter.getValue();
237+
filterValue = fieldFilter.getValue();
236238
break;
237239
case LESS_THAN:
238-
newValue = fieldFilter.getValue();
239-
newBefore = false;
240+
filterValue = fieldFilter.getValue();
241+
filterInclusive = false;
240242
break;
241243
}
242244

243-
if (min(largestValue, newValue) == newValue) {
244-
largestValue = newValue;
245-
before = newBefore;
245+
if (min(segmentValue, filterValue) == filterValue) {
246+
segmentValue = filterValue;
247+
segmentInclusive = filterInclusive;
246248
}
247249
}
248250
}
@@ -254,28 +256,29 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
254256
OrderBy orderBy = this.orderBys.get(i);
255257
if (orderBy.getField().equals(segment.getFieldPath())) {
256258
Value cursorValue = endAt.getPosition().get(i);
257-
if (min(largestValue, cursorValue) == cursorValue) {
258-
largestValue = cursorValue;
259-
before = endAt.isBefore();
259+
if (min(segmentValue, cursorValue) == cursorValue) {
260+
segmentValue = cursorValue;
261+
segmentInclusive = !endAt.isBefore();
260262
}
261263
break;
262264
}
263265
}
264266
}
265267

266-
if (largestValue == null) {
268+
if (segmentValue == null) {
267269
// No upper bound exists
268270
return null;
269271
}
270272

271-
values.add(largestValue);
273+
values.add(segmentValue);
274+
inclusive &= segmentInclusive;
272275
}
273276

274277
if (values.isEmpty()) {
275278
return null;
276279
}
277280

278-
return new Bound(values, before);
281+
return new Bound(values, !inclusive);
279282
}
280283

281284
public List<OrderBy> getOrderBy() {

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ public void addFieldIndex(FieldIndex index) {
110110

111111
@Override
112112
public void addIndexEntries(Document document) {
113-
ResourcePath documentPath = document.getKey().getPath();
114-
String collectionGroup = documentPath.getSegment(documentPath.length() - 2);
113+
DocumentKey documentKey = document.getKey();
114+
String collectionGroup = documentKey.getCollectionGroup();
115115
db.query(
116116
"SELECT index_id, index_proto FROM index_configuration WHERE collection_group = ? AND active = 1")
117117
.binding(collectionGroup)
@@ -127,10 +127,10 @@ public void addIndexEntries(Document document) {
127127
if (values == null) return;
128128

129129
if (Logger.isDebugEnabled()) {
130-
Logger.warn(
130+
Logger.debug(
131131
TAG,
132132
"Adding index values for document '%s' to index '%s'",
133-
documentPath,
133+
documentKey,
134134
fieldIndex);
135135
}
136136

@@ -144,7 +144,7 @@ public void addIndexEntries(Document document) {
144144
+ "document_name) VALUES(?, ?, ?)",
145145
indexId,
146146
encoded,
147-
documentPath.canonicalString());
147+
documentKey.toString());
148148
}
149149
} catch (InvalidProtocolBufferException e) {
150150
throw fail("Invalid index: " + e);
@@ -176,6 +176,8 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
176176
if (fieldIndex == null) return null;
177177

178178
Bound lowerBound = target.getLowerBound(fieldIndex);
179+
String lowerBoundOp = lowerBound.isBefore() ? ">=" : ">"; // `startAt()` versus `startAfter()`
180+
179181
@Nullable Bound upperBound = target.getUpperBound(fieldIndex);
180182

181183
if (Logger.isDebugEnabled()) {
@@ -200,13 +202,16 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
200202
lowerBoundValues.size() == upperBoundValues.size(),
201203
"Expected upper and lower bound size to match");
202204

205+
String upperBoundOp = upperBound.isBefore() ? "<" : "<="; // `endBefore()` versus `endAt()`
206+
203207
// TODO(indexing): To avoid reading the same documents multiple times, we should ideally only
204208
// send one query that combines all clauses.
209+
// TODO(indexing): Add limit handling
205210
for (int i = 0; i < lowerBoundValues.size(); ++i) {
206211
db.query(
207212
String.format(
208213
"SELECT document_name from index_entries WHERE index_id = ? AND index_value %s ? AND index_value %s ?",
209-
lowerBound.isBefore() ? ">=" : ">", upperBound.isBefore() ? "<=" : "<"))
214+
lowerBoundOp, upperBoundOp))
210215
.binding(fieldIndex.getIndexId(), lowerBoundValues.get(i), upperBoundValues.get(i))
211216
.forEach(
212217
row -> result.add(DocumentKey.fromPath(ResourcePath.fromString(row.getString(0)))));
@@ -218,7 +223,7 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
218223
db.query(
219224
String.format(
220225
"SELECT document_name from index_entries WHERE index_id = ? AND index_value %s ?",
221-
lowerBound.isBefore() ? ">=" : ">"))
226+
lowerBoundOp))
222227
.binding(fieldIndex.getIndexId(), lowerBoundValue)
223228
.forEach(
224229
row -> result.add(DocumentKey.fromPath(ResourcePath.fromString(row.getString(0)))));
@@ -259,15 +264,14 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
259264
throw fail("Failed to decode index: " + e);
260265
}
261266
});
262-
;
263267

264268
if (activeIndices.isEmpty()) {
265269
return null;
266270
}
267271

268272
// Return the index with the most number of segments
269-
Collections.sort(activeIndices, (l, r) -> Integer.compare(r.segmentCount(), l.segmentCount()));
270-
return activeIndices.get(0);
273+
return Collections.max(
274+
activeIndices, (l, r) -> Integer.compare(l.segmentCount(), r.segmentCount()));
271275
}
272276

273277
/**

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,11 @@ public ResourcePath getPath() {
108108
return path;
109109
}
110110

111+
/** Returns the collection group (i.e. the name of the parent collection) for this key. */
112+
public String getCollectionGroup() {
113+
return path.getSegment(path.length() - 2);
114+
}
115+
111116
/** Returns true if the document is in the specified collectionId. */
112117
public boolean hasCollectionId(String collectionId) {
113118
return path.length() >= 2 && path.segments.get(path.length() - 2).equals(collectionId);

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ public static Value refValue(DatabaseId databaseId, DocumentKey key) {
459459
}
460460

461461
/** Returns the lowest value for the given value type (inclusive). */
462-
public static Value getFirstValue(Value.ValueTypeCase valueTypeCase) {
462+
public static Value getLowestValue(Value.ValueTypeCase valueTypeCase) {
463463
switch (valueTypeCase) {
464464
case NULL_VALUE:
465465
return Values.NULL_VALUE;
@@ -497,24 +497,24 @@ public static Value getFirstValue(Value.ValueTypeCase valueTypeCase) {
497497
public static @Nullable Value getNextValue(Value.ValueTypeCase valueTypeCase) {
498498
switch (valueTypeCase) {
499499
case NULL_VALUE:
500-
return getFirstValue(Value.ValueTypeCase.BOOLEAN_VALUE);
500+
return getLowestValue(Value.ValueTypeCase.BOOLEAN_VALUE);
501501
case BOOLEAN_VALUE:
502-
return getFirstValue(Value.ValueTypeCase.INTEGER_VALUE);
502+
return getLowestValue(Value.ValueTypeCase.INTEGER_VALUE);
503503
case INTEGER_VALUE:
504504
case DOUBLE_VALUE:
505-
return getFirstValue(Value.ValueTypeCase.TIMESTAMP_VALUE);
505+
return getLowestValue(Value.ValueTypeCase.TIMESTAMP_VALUE);
506506
case TIMESTAMP_VALUE:
507-
return getFirstValue(Value.ValueTypeCase.STRING_VALUE);
507+
return getLowestValue(Value.ValueTypeCase.STRING_VALUE);
508508
case STRING_VALUE:
509-
return getFirstValue(Value.ValueTypeCase.BYTES_VALUE);
509+
return getLowestValue(Value.ValueTypeCase.BYTES_VALUE);
510510
case BYTES_VALUE:
511-
return getFirstValue(Value.ValueTypeCase.REFERENCE_VALUE);
511+
return getLowestValue(Value.ValueTypeCase.REFERENCE_VALUE);
512512
case REFERENCE_VALUE:
513-
return getFirstValue(Value.ValueTypeCase.GEO_POINT_VALUE);
513+
return getLowestValue(Value.ValueTypeCase.GEO_POINT_VALUE);
514514
case GEO_POINT_VALUE:
515-
return getFirstValue(Value.ValueTypeCase.ARRAY_VALUE);
515+
return getLowestValue(Value.ValueTypeCase.ARRAY_VALUE);
516516
case ARRAY_VALUE:
517-
return getFirstValue(Value.ValueTypeCase.MAP_VALUE);
517+
return getLowestValue(Value.ValueTypeCase.MAP_VALUE);
518518
case MAP_VALUE:
519519
// There is no type that sorts higher than a map.
520520
return null;

‎firebase-firestore/src/test/java/com/google/firebase/firestore/core/TargetTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public void equalsQueryBound() {
6060
verifyBound(lowerBound, true, "bar");
6161

6262
Bound upperBound = target.getUpperBound(index);
63-
verifyBound(upperBound, true, "bar");
63+
verifyBound(upperBound, false, "bar");
6464
}
6565

6666
@Test
@@ -73,7 +73,7 @@ public void lowerThanQueryBound() {
7373
verifyBound(lowerBound, true, "");
7474

7575
Bound upperBound = target.getUpperBound(index);
76-
verifyBound(upperBound, false, "bar");
76+
verifyBound(upperBound, true, "bar");
7777
}
7878

7979
@Test
@@ -86,7 +86,7 @@ public void lowerThanOrEqualsQueryBound() {
8686
verifyBound(lowerBound, true, "");
8787

8888
Bound upperBound = target.getUpperBound(index);
89-
verifyBound(upperBound, true, "bar");
89+
verifyBound(upperBound, false, "bar");
9090
}
9191

9292
@Test
@@ -125,7 +125,7 @@ public void containsQueryBound() {
125125
verifyBound(lowerBound, true, "bar");
126126

127127
Bound upperBound = target.getUpperBound(index);
128-
verifyBound(upperBound, true, "bar");
128+
verifyBound(upperBound, false, "bar");
129129
}
130130

131131
@Test
@@ -311,7 +311,7 @@ public void endBeforeDoesNotChangeBoundIfNotApplicable() {
311311
verifyBound(lowerBound, true, "", "b1");
312312

313313
Bound upperBound = target.getUpperBound(index);
314-
verifyBound(upperBound, true, "a1", "b1");
314+
verifyBound(upperBound, false, "a1", "b1");
315315
}
316316

317317
@Test
@@ -325,7 +325,7 @@ public void partialIndexMatchQueryBound() {
325325
verifyBound(lowerBound, true, "a");
326326

327327
Bound upperBound = target.getUpperBound(index);
328-
verifyBound(upperBound, true, "a");
328+
verifyBound(upperBound, false, "a");
329329
}
330330

331331
private void verifyBound(Bound bound, boolean before, Object... values) {

0 commit comments

Comments
 (0)