Skip to content

Serve OR queries from the index. #3368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 6, 2022
Merged

Serve OR queries from the index. #3368

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6d9be9a
Serve or queries from the index.
ehsannas Jan 25, 2022
6e62711
Update test.
ehsannas Jan 25, 2022
4860613
Clean up.
ehsannas Jan 25, 2022
893a014
Move LogicUtils to 'util' directory.
ehsannas Jan 25, 2022
28ac1cb
Reuse existing code for finding least recent index offset.
ehsannas Jan 25, 2022
81b6e16
Add missing function to MemoryIndexManager
ehsannas Jan 25, 2022
c829fe1
Add license to new file.
ehsannas Jan 26, 2022
d65a6e0
Address comments.
ehsannas Feb 4, 2022
4b7858b
Address comments.
ehsannas Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 1 addition & 23 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexBackfiller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,10 @@
import androidx.annotation.VisibleForTesting;
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.FieldIndex;
import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
import com.google.firebase.firestore.util.AsyncQueue;
import com.google.firebase.firestore.util.Logger;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -132,8 +129,7 @@ private int writeIndexEntries() {
private int writeEntriesForCollectionGroup(
String collectionGroup, int documentsRemainingUnderCap) {
// Use the earliest offset of all field indexes to query the local cache.
Collection<FieldIndex> fieldIndexes = indexManager.getFieldIndexes(collectionGroup);
IndexOffset existingOffset = getExistingOffset(fieldIndexes);
IndexOffset existingOffset = indexManager.minOffset(collectionGroup);

LocalDocumentsResult nextBatch =
localDocumentsView.getNextDocuments(
Expand Down Expand Up @@ -161,24 +157,6 @@ private IndexOffset getNewOffset(IndexOffset existingOffset, LocalDocumentsResul
Math.max(lookupResult.getBatchId(), existingOffset.getLargestBatchId()));
}

/** Returns the lowest offset for the provided index group. */
private IndexOffset getExistingOffset(Collection<FieldIndex> fieldIndexes) {
hardAssert(!fieldIndexes.isEmpty(), "Updating collection without indexes");

Iterator<FieldIndex> it = fieldIndexes.iterator();
IndexOffset minOffset = it.next().getIndexState().getOffset();
int minBatchId = minOffset.getLargestBatchId();
while (it.hasNext()) {
IndexOffset newOffset = it.next().getIndexState().getOffset();
if (newOffset.compareTo(minOffset) < 0) {
minOffset = newOffset;
}
minBatchId = Math.max(newOffset.getLargestBatchId(), minBatchId);
}

return IndexOffset.create(minOffset.getReadTime(), minOffset.getDocumentKey(), minBatchId);
}

@VisibleForTesting
void setMaxDocumentsToProcess(int newMax) {
maxDocumentsToProcess = newMax;
Expand Down
15 changes: 14 additions & 1 deletion firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.FieldIndex;
import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
import com.google.firebase.firestore.model.ResourcePath;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -74,6 +75,18 @@ public interface IndexManager {
/** Returns all configured field indexes. */
Collection<FieldIndex> getFieldIndexes();

/** Returns whether we can serve the given target from an index. */
boolean canServeFromIndex(Target target);

/**
* Iterates over all field indexes that are used to serve the given target, and returns the least
* recent offset of them all. Asserts that the target can be served from index.
*/
IndexOffset minOffset(Target target);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Java methods should start with a verb: getMinOffset()

You should also align the two comments on their usage of "lowest" and "least recent" (which contrasts min).


/** Returns the lowest offset for the given collection group. */
IndexOffset minOffset(String collectionGroup);

/**
* Returns an index that can be used to serve the provided target. Returns {@code null} if no
* index is configured.
Expand All @@ -82,7 +95,7 @@ public interface IndexManager {
FieldIndex getFieldIndex(Target target);

/** Returns the documents that match the given target based on the provided index. */
Set<DocumentKey> getDocumentsMatchingTarget(FieldIndex fieldIndex, Target target);
Set<DocumentKey> getDocumentsMatchingTarget(Target target);

/** Returns the next collection group to update. Returns {@code null} if no group exists. */
@Nullable
Expand Down
20 changes: 18 additions & 2 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryIndexManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.FieldIndex;
import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
import com.google.firebase.firestore.model.ResourcePath;
import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -69,7 +70,7 @@ public FieldIndex getFieldIndex(Target target) {

@Override
@Nullable
public Set<DocumentKey> getDocumentsMatchingTarget(FieldIndex fieldIndex, Target target) {
public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
// Field indices are not supported with memory persistence.
return Collections.emptySet();
}
Expand All @@ -82,7 +83,7 @@ public String getNextCollectionGroupToUpdate() {
}

@Override
public void updateCollectionGroup(String collectionGroup, FieldIndex.IndexOffset offset) {
public void updateCollectionGroup(String collectionGroup, IndexOffset offset) {
// Field indices are not supported with memory persistence.
}

Expand All @@ -98,6 +99,21 @@ public Collection<FieldIndex> getFieldIndexes() {
return Collections.emptyList();
}

@Override
public boolean canServeFromIndex(Target target) {
return false;
}

@Override
public IndexOffset minOffset(Target target) {
return IndexOffset.NONE;
}

@Override
public IndexOffset minOffset(String collectionGroup) {
return IndexOffset.NONE;
}

@Override
public void updateIndexEntries(ImmutableSortedMap<DocumentKey, Document> documents) {
// Field indices are not supported with memory persistence.
Expand Down
15 changes: 6 additions & 9 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,20 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
*/
private @Nullable ImmutableSortedMap<DocumentKey, Document> performQueryUsingIndex(
Query query, Target target) {
// TODO(orquery): Update this condition when we are able to serve or queries from the index.
if (query.matchesAllDocuments() || query.containsCompositeFilters()) {
if (query.matchesAllDocuments()) {
// Don't use index queries that can be executed by scanning the collection.
return null;
}

FieldIndex fieldIndex = indexManager.getFieldIndex(query.toTarget());
if (fieldIndex == null) {
if (!indexManager.canServeFromIndex(target)) {
return null;
}

Set<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(fieldIndex, target);
Set<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(target);
ImmutableSortedMap<DocumentKey, Document> indexedDocuments =
localDocumentsView.getDocuments(keys);
return appendRemainingResults(
values(indexedDocuments), query, fieldIndex.getIndexState().getOffset());

return appendRemainingResults(values(indexedDocuments), query, indexManager.minOffset(target));
}

/**
Expand All @@ -131,8 +129,7 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
Query query,
ImmutableSortedSet<DocumentKey> remoteKeys,
SnapshotVersion lastLimboFreeSnapshotVersion) {
// TODO(orquery): Update this condition when we are able to serve or queries from the index.
if (query.matchesAllDocuments() || query.containsCompositeFilters()) {
if (query.matchesAllDocuments()) {
// Don't use index queries that can be executed by scanning the collection.
return null;
}
Expand Down
Loading