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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.getMinOffset(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
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
* minimum offset of them all. Asserts that the target can be served from index.
*/
IndexOffset getMinOffset(Target target);

/** Returns the minimum offset for the given collection group. */
IndexOffset getMinOffset(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
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 getMinOffset(Target target) {
return IndexOffset.NONE;
}

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

@Override
public void updateIndexEntries(ImmutableSortedMap<DocumentKey, Document> documents) {
// Field indices are not supported with memory persistence.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,21 @@ 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());
values(indexedDocuments), query, indexManager.getMinOffset(target));
}

/**
Expand All @@ -131,8 +130,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