Skip to content

Commit bc90e10

Browse files
schmidt-sebastianschmidt-sebastian
committed
Feedback
1 parent f81c44b commit bc90e10

File tree

5 files changed

+92
-13
lines changed

5 files changed

+92
-13
lines changed

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,17 @@ public int hashCode() {
132132

133133
@Override
134134
public String toString() {
135-
return "Bound{before=" + before + ", position=" + position + '}';
135+
StringBuilder builder = new StringBuilder();
136+
builder.append("Bound(before=");
137+
builder.append(before);
138+
builder.append(", position=");
139+
for (int i = 0; i < position.size(); i++) {
140+
if (i > 0) {
141+
builder.append(" and ");
142+
}
143+
builder.append(Values.canonicalId(position.get(i)));
144+
}
145+
builder.append(")");
146+
return builder.toString();
136147
}
137148
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public String getCanonicalId() {
137137

138138
@Override
139139
public String toString() {
140-
return field.canonicalString() + " " + operator + " " + value;
140+
return field.canonicalString() + " " + operator + " " + Values.canonicalId(value);
141141
}
142142

143143
@Override

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,11 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
131131
FieldFilter fieldFilter = (FieldFilter) filter;
132132
switch (fieldFilter.getOperator()) {
133133
case LESS_THAN:
134-
case NOT_EQUAL:
135134
case LESS_THAN_OR_EQUAL:
135+
// TODO(indexing): Implement type clamping. Only field values with the same type
136+
// should match the query.
137+
break;
138+
case NOT_EQUAL:
136139
// These filters cannot be used as a lower bound. Skip.
137140
break;
138141
case NOT_IN:
@@ -199,12 +202,15 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
199202
if (filter.getField().equals(segment.getFieldPath())) {
200203
FieldFilter fieldFilter = (FieldFilter) filter;
201204
switch (fieldFilter.getOperator()) {
202-
case GREATER_THAN:
203205
case NOT_IN:
204206
case NOT_EQUAL:
205-
case GREATER_THAN_OR_EQUAL:
206207
// These filters cannot be used as an upper bound. Skip.
207208
break;
209+
case GREATER_THAN_OR_EQUAL:
210+
case GREATER_THAN:
211+
// TODO(indexing): Implement type clamping. Only field values with the same type
212+
// should match the query.
213+
break;
208214
case EQUAL:
209215
case IN:
210216
case ARRAY_CONTAINS_ANY:
@@ -364,7 +370,7 @@ public String toString() {
364370
if (i > 0) {
365371
builder.append(" and ");
366372
}
367-
builder.append(filters.get(i).toString());
373+
builder.append(filters.get(i));
368374
}
369375
}
370376

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.google.firebase.firestore.model.FieldPath;
3131
import com.google.firebase.firestore.model.ResourcePath;
3232
import com.google.firebase.firestore.model.TargetIndexMatcher;
33+
import com.google.firebase.firestore.util.Logger;
3334
import com.google.firestore.admin.v1.Index;
3435
import com.google.firestore.v1.Value;
3536
import com.google.protobuf.InvalidProtocolBufferException;
@@ -41,6 +42,8 @@
4142

4243
/** A persisted implementation of IndexManager. */
4344
final class SQLiteIndexManager implements IndexManager {
45+
private static final String TAG = SQLiteIndexManager.class.getSimpleName();
46+
4447
/**
4548
* An in-memory copy of the index entries we've already written since the SDK launched. Used to
4649
* avoid re-writing the same entry repeatedly.
@@ -107,7 +110,8 @@ public void addFieldIndex(FieldIndex index) {
107110

108111
@Override
109112
public void addIndexEntries(Document document) {
110-
String collectionGroup = document.getKey().getPath().popLast().canonicalString();
113+
ResourcePath documentPath = document.getKey().getPath();
114+
String collectionGroup = documentPath.getSegment(documentPath.length() - 2);
111115
db.query(
112116
"SELECT index_id, index_proto FROM index_configuration WHERE collection_group = ? AND active = 1")
113117
.binding(collectionGroup)
@@ -122,6 +126,14 @@ public void addIndexEntries(Document document) {
122126
List<Value> values = extractFieldValue(document, fieldIndex);
123127
if (values == null) return;
124128

129+
if (Logger.isDebugEnabled()) {
130+
Logger.warn(
131+
TAG,
132+
"Adding index values for document '%s' to index '%s'",
133+
documentPath,
134+
fieldIndex);
135+
}
136+
125137
List<byte[]> encodeValues = encodeDocumentValues(fieldIndex, values);
126138
for (byte[] encoded : encodeValues) {
127139
// TODO(indexing): Handle different values for different users
@@ -132,7 +144,7 @@ public void addIndexEntries(Document document) {
132144
+ "document_name) VALUES(?, ?, ?)",
133145
indexId,
134146
encoded,
135-
document.getKey().getPath().canonicalString());
147+
documentPath.canonicalString());
136148
}
137149
} catch (InvalidProtocolBufferException e) {
138150
throw fail("Invalid index: " + e);
@@ -166,6 +178,16 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
166178
Bound lowerBound = target.getLowerBound(fieldIndex);
167179
@Nullable Bound upperBound = target.getUpperBound(fieldIndex);
168180

181+
if (Logger.isDebugEnabled()) {
182+
Logger.warn(
183+
TAG,
184+
"Using index '%s' to execute '%s' (Lower bound: %s, Upper bound: %s)",
185+
fieldIndex,
186+
target,
187+
lowerBound,
188+
upperBound);
189+
}
190+
169191
Set<DocumentKey> result = new HashSet<>();
170192

171193
if (upperBound != null) {
@@ -202,6 +224,8 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
202224
row -> result.add(DocumentKey.fromPath(ResourcePath.fromString(row.getString(0)))));
203225
}
204226
}
227+
228+
Logger.debug(TAG, "Index scan returned %s documents", result.size());
205229
return result;
206230
}
207231

@@ -304,7 +328,8 @@ private List<byte[]> getEncodedBytes(List<IndexByteEncoder> encoders) {
304328
/**
305329
* Creates a separate encoder for each element of an array.
306330
*
307-
* <p>The method appends each value to all existing encoders (e.g. filter("a", "==", "a1").filter("b", "in", ["b1", "b2"]) becomes ["a1,b1", "a1,b2"]). A list of new encoders is
331+
* <p>The method appends each value to all existing encoders (e.g. filter("a", "==",
332+
* "a1").filter("b", "in", ["b1", "b2"]) becomes ["a1,b1", "a1,b2"]). A list of new encoders is
308333
* returned.
309334
*/
310335
private List<IndexByteEncoder> expandIndexValues(List<IndexByteEncoder> encoders, Value value) {

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

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
import static com.google.firebase.firestore.testutil.TestUtil.key;
2323
import static com.google.firebase.firestore.testutil.TestUtil.map;
2424
import static com.google.firebase.firestore.testutil.TestUtil.orderBy;
25+
import static com.google.firebase.firestore.testutil.TestUtil.path;
2526
import static com.google.firebase.firestore.testutil.TestUtil.query;
27+
import static org.junit.Assert.assertNull;
2628

2729
import com.google.firebase.firestore.core.Query;
2830
import com.google.firebase.firestore.model.DocumentKey;
@@ -205,14 +207,49 @@ public void testArrayContainsDoesNotMatchNonArray() {
205207
setUpArrayValueFilter();
206208
setUpSingleValueFilter();
207209
addDoc("coll/nonmatching", map("values", 1));
208-
Query query =
209-
query("coll").filter(filter("values", "array-contains-any", Arrays.asList(1)));
210+
Query query = query("coll").filter(filter("values", "array-contains-any", Arrays.asList(1)));
210211
verifyResults(query, "coll/doc1");
211212
}
212213

214+
@Test
215+
public void testNoMatchingFilter() {
216+
setUpSingleValueFilter();
217+
Query query = query("coll").filter(filter("unknown", "==", true));
218+
assertNull(indexManager.getDocumentsMatchingTarget(query.toTarget()));
219+
}
220+
221+
@Test
222+
public void testNoMatchingDocs() {
223+
setUpSingleValueFilter();
224+
Query query = query("coll").filter(filter("count", "==", -1));
225+
verifyResults(query);
226+
}
227+
228+
@Test
229+
public void testEqualityFilterWithNonMatchingType() {
230+
indexManager.addFieldIndex(
231+
new FieldIndex("coll").withAddedField(field("value"), FieldIndex.Segment.Kind.ORDERED));
232+
addDoc("coll/boolean", map("value", true));
233+
addDoc("coll/string", map("value", "true"));
234+
addDoc("coll/number", map("value", 1));
235+
Query query = query("coll").filter(filter("value", "==", true));
236+
verifyResults(query, "coll/boolean");
237+
}
238+
239+
@Test
240+
public void testCollectionGroup() {
241+
indexManager.addFieldIndex(
242+
new FieldIndex("coll1").withAddedField(field("value"), FieldIndex.Segment.Kind.ORDERED));
243+
addDoc("coll1/doc1", map("value", true));
244+
addDoc("coll2/doc2/coll1/doc1", map("value", true));
245+
addDoc("coll2/doc2", map("value", true));
246+
Query query = new Query(path(""), "coll1").filter(filter("value", "==", true));
247+
verifyResults(query, "coll1/doc1", "coll2/doc2/coll1/doc1");
248+
}
249+
213250
private void addDoc(String key, Map<String, Object> data) {
214-
MutableDocument count1Doc = doc(key, 1, data);
215-
indexManager.addIndexEntries(count1Doc);
251+
MutableDocument doc = doc(key, 1, data);
252+
indexManager.addIndexEntries(doc);
216253
}
217254

218255
private void verifyResults(Query query, String... documents) {

0 commit comments

Comments
 (0)