Skip to content

Commit d27d5b6

Browse files
schmidt-sebastianschmidt-sebastian
committed
Make LowerBound nullable
1 parent a301274 commit d27d5b6

File tree

3 files changed

+83
-65
lines changed

3 files changed

+83
-65
lines changed

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

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.google.firebase.firestore.model.FieldIndex;
2323
import com.google.firebase.firestore.model.ResourcePath;
2424
import com.google.firebase.firestore.model.Values;
25-
import com.google.firestore.v1.ArrayValue;
2625
import com.google.firestore.v1.Value;
2726
import java.util.ArrayList;
2827
import java.util.Collections;
@@ -152,7 +151,7 @@ public List<Value> getArrayValues(FieldIndex fieldIndex) {
152151
* <p>Unlike {@link #getUpperBound}, lower bounds always exist as the SDK can use {@code null} as
153152
* a starting point for missing boundary values.
154153
*/
155-
public Bound getLowerBound(FieldIndex fieldIndex) {
154+
public @Nullable Bound getLowerBound(FieldIndex fieldIndex) {
156155
List<Value> values = new ArrayList<>();
157156
boolean inclusive = true;
158157

@@ -161,7 +160,8 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
161160
if (!segment.getKind().equals(FieldIndex.Segment.Kind.ORDERED)) {
162161
continue;
163162
}
164-
Value segmentValue = Values.NULL_VALUE;
163+
164+
Value segmentValue = null;
165165
boolean segmentInclusive = true;
166166

167167
for (Filter filter : filters) {
@@ -176,18 +176,12 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
176176
filterValue = Values.getLowerBound(fieldFilter.getValue().getValueTypeCase());
177177
break;
178178
case NOT_EQUAL:
179-
filterValue = Values.NULL_VALUE;
180-
break;
181179
case NOT_IN:
182-
filterValue =
183-
Value.newBuilder()
184-
.setArrayValue(ArrayValue.newBuilder().addValues(Values.NULL_VALUE))
185-
.build();
180+
case ARRAY_CONTAINS_ANY:
181+
case ARRAY_CONTAINS:
186182
break;
187183
case EQUAL:
188184
case IN:
189-
case ARRAY_CONTAINS_ANY:
190-
case ARRAY_CONTAINS:
191185
case GREATER_THAN_OR_EQUAL:
192186
filterValue = fieldFilter.getValue();
193187
break;
@@ -220,10 +214,25 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
220214
}
221215
}
222216

217+
if (segmentValue == null) {
218+
for (OrderBy orderBy : orderBys) {
219+
if (orderBy.getField().equals(segment.getFieldPath())) {
220+
segmentValue = Values.NULL_VALUE;
221+
}
222+
}
223+
224+
if (segmentValue == null) {
225+
// No lower bound exists
226+
return null;
227+
}
228+
}
229+
223230
values.add(segmentValue);
224231
inclusive &= segmentInclusive;
225232
}
226233

234+
if (values.isEmpty()) return null;
235+
227236
return new Bound(values, inclusive);
228237
}
229238

@@ -240,6 +249,10 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
240249
boolean inclusive = true;
241250

242251
for (FieldIndex.Segment segment : fieldIndex) {
252+
if (!segment.getKind().equals(FieldIndex.Segment.Kind.ORDERED)) {
253+
continue;
254+
}
255+
243256
@Nullable Value segmentValue = null;
244257
boolean segmentInclusive = true;
245258

@@ -253,6 +266,8 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
253266
switch (fieldFilter.getOperator()) {
254267
case NOT_IN:
255268
case NOT_EQUAL:
269+
case ARRAY_CONTAINS_ANY:
270+
case ARRAY_CONTAINS:
256271
// These filters cannot be used as an upper bound. Skip.
257272
break;
258273
case GREATER_THAN_OR_EQUAL:
@@ -262,8 +277,6 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
262277
break;
263278
case EQUAL:
264279
case IN:
265-
case ARRAY_CONTAINS_ANY:
266-
case ARRAY_CONTAINS:
267280
case LESS_THAN_OR_EQUAL:
268281
filterValue = fieldFilter.getValue();
269282
break;

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

Lines changed: 56 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,13 @@ public void addIndexEntries(Document document) {
164164
}
165165

166166
private void addSingleEntry(
167-
DocumentKey documentKey, int indexId, @Nullable Value arrayIndex, Object directionalIndex) {
167+
DocumentKey documentKey, int indexId, @Nullable Value arrayIndex, Object directionalIndex) {
168168
// TODO(indexing): Handle different values for different users
169169
db.execute(
170-
"INSERT INTO index_entries (index_id, array_value, directional_value, document_name) " +
171-
"VALUES(?, ?, ?, ?)",
170+
"INSERT INTO index_entries (index_id, array_value, directional_value, document_name) "
171+
+ "VALUES(?, ?, ?, ?)",
172172
indexId,
173-
arrayIndex != null ? encodeDocumentValues(Collections.singletonList(arrayIndex)) : null,
173+
arrayIndex != null ? encodeDocumentValues(Collections.singletonList(arrayIndex)) : null,
174174
directionalIndex,
175175
documentKey.toString());
176176
}
@@ -186,7 +186,7 @@ private List<Value> extractValue(
186186
for (FieldIndex.Segment segment : fieldIndex) {
187187
if (segment.getKind().equals(kind)) {
188188
Value field = document.getField(segment.getFieldPath());
189-
if (field == null ) {
189+
if (field == null) {
190190
return null;
191191
}
192192
if (segment.getKind().equals(FieldIndex.Segment.Kind.ARRAYS)) {
@@ -209,25 +209,23 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
209209
if (fieldIndex == null) return null;
210210

211211
@Nullable List<Value> arrayValues = target.getArrayValues(fieldIndex);
212-
Bound lowerBound = target.getLowerBound(fieldIndex);
212+
@Nullable Bound lowerBound = target.getLowerBound(fieldIndex);
213213
@Nullable Bound upperBound = target.getUpperBound(fieldIndex);
214214

215215
if (Logger.isDebugEnabled()) {
216216
Logger.debug(
217217
TAG,
218-
"Using index '%s' to execute '%s' (Lower bound: %s, Upper bound: %s)",
218+
"Using index '%s' to execute '%s' (Arrays: %s, Lower bound: %s, Upper bound: %s)",
219219
fieldIndex,
220220
target,
221+
arrayValues,
221222
lowerBound,
222223
upperBound);
223224
}
224225

225-
Object[] lowerBoundValues = encodeTargetValues(fieldIndex, target, lowerBound.getPosition());
226-
String lowerBoundOp = lowerBound.isInclusive() ? ">=" : ">";
227-
Object[] upperBoundValues =
228-
upperBound != null
229-
? encodeTargetValues(fieldIndex, target, upperBound.getPosition())
230-
: null;
226+
Object[] lowerBoundValues = encodeBound(fieldIndex, target, lowerBound);
227+
String lowerBoundOp = lowerBound != null ? (lowerBound.isInclusive() ? ">=" : ">") : null;
228+
Object[] upperBoundValues = encodeBound(fieldIndex, target, upperBound);
231229
String upperBoundOp = upperBound != null ? (upperBound.isInclusive() ? "<=" : "<") : null;
232230

233231
SQLitePersistence.Query query =
@@ -253,40 +251,33 @@ private SQLitePersistence.Query generateQuery(
253251
Target target,
254252
int indexId,
255253
@Nullable List<Value> arrayValues,
256-
Object[] lowerBounds,
257-
String lowerBoundOp,
254+
@Nullable Object[] lowerBounds,
255+
@Nullable String lowerBoundOp,
258256
@Nullable Object[] upperBounds,
259257
@Nullable String upperBoundOp) {
260-
String orderBy;
261-
String limit;
262-
263-
if (target.getLimit() != -1) {
264-
String direction = target.getFirstOrderBy().getDirection().canonicalString();
265-
orderBy = "ORDER BY directional_value " + direction + ", document_name " + direction;
266-
limit = "LIMIT " + target.getLimit();
267-
} else {
268-
orderBy = "";
269-
limit = "";
270-
}
271-
272258
StringBuilder statement = new StringBuilder();
273259
statement.append(
274260
"SELECT document_name, directional_value FROM index_entries WHERE index_id = ? ");
275-
276-
if (arrayValues != null) {
277-
statement.append("AND array_value = ? ");
278-
}
279-
statement.append("AND directional_value ").append(lowerBoundOp).append(" ? ");
261+
if (arrayValues != null) {
262+
statement.append("AND array_value = ? ");
263+
}
264+
if (lowerBounds != null) {
265+
statement.append("AND directional_value ").append(lowerBoundOp).append(" ? ");
266+
}
280267
if (upperBounds != null) {
281268
statement.append("AND directional_value ").append(upperBoundOp).append(" ? ");
282269
}
283270

284271
int statementCount =
285-
(arrayValues == null ? 1 : arrayValues.size()) * lowerBounds.length
272+
(arrayValues == null ? 1 : arrayValues.size())
273+
* (lowerBounds == null ? 1 : lowerBounds.length)
286274
* (upperBounds == null ? 1 : upperBounds.length);
287-
String sql =
288-
String.format(
289-
"%s %s %s", repeatSequence(statement, statementCount, " UNION "), orderBy, limit);
275+
String sql = repeatSequence(statement, statementCount, " UNION ");
276+
if (target.getLimit() != -1) {
277+
String direction = target.getFirstOrderBy().getDirection().canonicalString();
278+
sql += "ORDER BY directional_value " + direction + ", document_name " + direction;
279+
sql += "LIMIT " + target.getLimit();
280+
}
290281

291282
List<Object> bindArgs = new ArrayList<>();
292283
if (arrayValues == null) {
@@ -308,23 +299,28 @@ private SQLitePersistence.Query generateQuery(
308299
private List<Object> computeBounds(
309300
int indexId,
310301
@Nullable Object arrayValue,
311-
Object[] lowerBounds,
302+
@Nullable Object[] lowerBounds,
312303
@Nullable Object[] upperBounds) {
304+
if (lowerBounds == null) {
305+
lowerBounds = new Object[] {null};
306+
}
307+
if (upperBounds == null) {
308+
upperBounds = new Object[] {null};
309+
}
310+
313311
List<Object> bindArgs = new ArrayList<>();
314312
for (Object lower : lowerBounds) {
315-
if (upperBounds != null) {
316-
for (Object upper : upperBounds) {
317-
bindArgs.add(indexId);
318-
if (arrayValue != null)
313+
for (Object upper : upperBounds) {
314+
bindArgs.add(indexId);
315+
if (arrayValue != null) {
319316
bindArgs.add(arrayValue);
317+
}
318+
if (lower != null) {
320319
bindArgs.add(lower);
320+
}
321+
if (upper != null) {
321322
bindArgs.add(upper);
322323
}
323-
} else {
324-
bindArgs.add(indexId);
325-
if (arrayValue != null)
326-
bindArgs.add(arrayValue);
327-
bindArgs.add(lower);
328324
}
329325
}
330326
return bindArgs;
@@ -390,14 +386,23 @@ private Object encodeDocumentValues(List<Value> values) {
390386
* Encodes the given field values according to the specification in {@code target}. For IN and
391387
* ArrayContainsAny queries, a list of possible values is returned.
392388
*/
393-
private Object[] encodeTargetValues(FieldIndex fieldIndex, Target target, List<Value> values) {
389+
private @Nullable Object[] encodeBound(
390+
FieldIndex fieldIndex, Target target, @Nullable Bound bound) {
391+
if (bound == null) {
392+
return null;
393+
}
394394
List<IndexByteEncoder> encoders = new ArrayList<>();
395395
encoders.add(new IndexByteEncoder());
396-
for (int i = 0; i < fieldIndex.segmentCount(); ++i) {
397-
FieldIndex.Segment segment = fieldIndex.getSegment(i);
398-
Value value = values.get(i);
396+
397+
int i = 0;
398+
for (FieldIndex.Segment segment : fieldIndex) { // get ordered segments, get array segments
399+
if (!segment.getKind().equals(FieldIndex.Segment.Kind.ORDERED)) {
400+
continue;
401+
}
402+
Value value = bound.getPosition().get(i);
399403
for (IndexByteEncoder encoder : encoders) {
400-
if (isMultiValueFilter(target, segment.getFieldPath())) {
404+
if (isMultiValueFilter(target, segment.getFieldPath())
405+
&& value.getValueTypeCase() == Value.ValueTypeCase.ARRAY_VALUE) {
401406
encoders = expandIndexValues(encoders, value);
402407
} else {
403408
FirestoreIndexValueWriter.INSTANCE.writeIndexValue(value, encoder);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public void addAndRemoveIndexEntry() {
7474
IndexEntry entry = backfiller.getIndexEntry(1);
7575
assertNotNull(entry);
7676
assertEquals("FOO", new String(entry.getArrayValue()));
77-
assertEquals("BAR", new String(entry.getDirectionalValue()));
77+
assertEquals("BAR", new String(entry.getDirectionalValue()));
7878
assertEquals("coll/sample-documentId", entry.getDocumentName());
7979
assertEquals("sample-uid", entry.getUid());
8080

0 commit comments

Comments
 (0)