Skip to content

Add ordering to Index implementation #2995

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 4 commits into from
Sep 29, 2021
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 @@ -24,18 +24,25 @@
public class OrderBy {
/** The direction of the ordering */
public enum Direction {
ASCENDING(1),
DESCENDING(-1);
ASCENDING(1, "asc"),
DESCENDING(-1, "desc");

private final int comparisonModifier;
private final String shorthand;

Direction(int comparisonModifier) {
Direction(int comparisonModifier, String canonicalString) {
this.comparisonModifier = comparisonModifier;
this.shorthand = canonicalString;
}

int getComparisonModifier() {
return comparisonModifier;
}

/** Returns "asc" for ascending or "desc" for descending. */
public String canonicalString() {
return shorthand;
}
}

public static OrderBy getInstance(Direction direction, FieldPath path) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@
import static com.google.firebase.firestore.model.Values.min;

import androidx.annotation.Nullable;
import com.google.firebase.firestore.core.OrderBy.Direction;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.FieldIndex;
import com.google.firebase.firestore.model.ResourcePath;
import com.google.firebase.firestore.model.Values;
import com.google.firestore.v1.ArrayValue;
import com.google.firestore.v1.Value;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/**
Expand Down Expand Up @@ -115,6 +114,27 @@ public boolean hasLimit() {
return endAt;
}

/** Returns the list of values that are used in ARRAY_CONTAINS or ARRAY_CONTAINS_ANY filters. */
public List<Value> getArrayValues(FieldIndex fieldIndex) {
for (FieldIndex.Segment segment : fieldIndex.getArraySegments()) {
for (Filter filter : filters) {
if (filter.getField().equals(segment.getFieldPath())) {
FieldFilter fieldFilter = (FieldFilter) filter;
switch (fieldFilter.getOperator()) {
case ARRAY_CONTAINS_ANY:
return fieldFilter.getValue().getArrayValue().getValuesList();
case ARRAY_CONTAINS:
return Collections.singletonList(fieldFilter.getValue());
default:
// Remaining filters cannot be used as array filters.
}
}
}
}

return Collections.emptyList();
}

/**
* Returns a lower bound of field values that can be used as a starting point to scan the index
* defined by {@code fieldIndex}.
Expand All @@ -127,7 +147,7 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
boolean inclusive = true;

// Go through all filters to find a value for the current field segment
for (FieldIndex.Segment segment : fieldIndex) {
for (FieldIndex.Segment segment : fieldIndex.getDirectionalSegments()) {
Value segmentValue = Values.NULL_VALUE;
boolean segmentInclusive = true;

Expand All @@ -142,26 +162,17 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
case LESS_THAN_OR_EQUAL:
filterValue = Values.getLowerBound(fieldFilter.getValue().getValueTypeCase());
break;
case NOT_EQUAL:
Copy link
Contributor

Choose a reason for hiding this comment

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

NOT_EQUAL is also deleted here because we do not support it yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can user the default value for this and other cases (see line 131 in the old code, 151 in the new code).

filterValue = Values.NULL_VALUE;
break;
case NOT_IN:
filterValue =
Value.newBuilder()
.setArrayValue(ArrayValue.newBuilder().addValues(Values.NULL_VALUE))
.build();
break;
case EQUAL:
case IN:
case ARRAY_CONTAINS_ANY:
case ARRAY_CONTAINS:
case GREATER_THAN_OR_EQUAL:
filterValue = fieldFilter.getValue();
break;
case GREATER_THAN:
filterValue = fieldFilter.getValue();
filterInclusive = false;
break;
default:
// Remaining filters cannot be used as lower bounds.
}

if (max(segmentValue, filterValue) == filterValue) {
Expand Down Expand Up @@ -206,7 +217,7 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
List<Value> values = new ArrayList<>();
boolean inclusive = true;

for (FieldIndex.Segment segment : fieldIndex) {
for (FieldIndex.Segment segment : fieldIndex.getDirectionalSegments()) {
@Nullable Value segmentValue = null;
boolean segmentInclusive = true;

Expand All @@ -218,26 +229,22 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
boolean filterInclusive = true;

switch (fieldFilter.getOperator()) {
case NOT_IN:
case NOT_EQUAL:
// These filters cannot be used as an upper bound. Skip.
break;
case GREATER_THAN_OR_EQUAL:
case GREATER_THAN:
filterValue = Values.getUpperBound(fieldFilter.getValue().getValueTypeCase());
filterInclusive = false;
break;
case EQUAL:
case IN:
case ARRAY_CONTAINS_ANY:
case ARRAY_CONTAINS:
case LESS_THAN_OR_EQUAL:
filterValue = fieldFilter.getValue();
break;
case LESS_THAN:
filterValue = fieldFilter.getValue();
filterInclusive = false;
break;
default:
// Remaining filters cannot be used as upper bounds.
}

if (min(segmentValue, filterValue) == filterValue) {
Expand Down Expand Up @@ -283,6 +290,11 @@ public List<OrderBy> getOrderBy() {
return this.orderBys;
}

/** Returns the first order by (which always exists). */
public OrderBy getFirstOrderBy() {
return this.orderBys.get(0);
}

/** Returns a canonical string representing this target. */
public String getCanonicalId() {
if (memoizedCannonicalId != null) {
Expand All @@ -307,7 +319,7 @@ public String getCanonicalId() {
builder.append("|ob:");
for (OrderBy orderBy : getOrderBy()) {
builder.append(orderBy.getField().canonicalString());
builder.append(orderBy.getDirection().equals(Direction.ASCENDING) ? "asc" : "desc");
builder.append(orderBy.getDirection().canonicalString());
}

// Add limit.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@
// TODO(indexing)
public class IndexEntry {
private final int indexId;
private final byte[] indexValue;
private final byte[] arrayValue;
private final byte[] directionalValue;
private final String uid;
private final String documentName;

public IndexEntry(int indexId, byte[] indexValue, String uid, String documentName) {
public IndexEntry(
int indexId, byte[] arrayValue, byte[] directionalValue, String uid, String documentName) {
this.indexId = indexId;
this.indexValue = indexValue;
this.arrayValue = arrayValue;
this.directionalValue = directionalValue;
this.uid = uid;
this.documentName = documentName;
}
Expand All @@ -36,8 +39,12 @@ public int getIndexId() {
return indexId;
}

public byte[] getIndexValue() {
return indexValue;
public byte[] getArrayValue() {
return arrayValue;
}

public byte[] getDirectionalValue() {
return directionalValue;
}

public String getUid() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,13 @@ void addIndexEntry(IndexEntry entry) {
persistence.execute(
"INSERT OR IGNORE INTO index_entries ("
+ "index_id, "
+ "index_value, "
+ "array_value, "
+ "directional_value, "
+ "uid, "
+ "document_name) VALUES(?, ?, ?, ?)",
+ "document_name) VALUES(?, ?, ?, ?, ?)",
entry.getIndexId(),
entry.getIndexValue(),
entry.getArrayValue(),
entry.getDirectionalValue(),
entry.getUid(),
entry.getDocumentName());
}
Expand All @@ -141,12 +143,18 @@ void removeIndexEntry(int indexId, String uid, String documentName) {
@VisibleForTesting
IndexEntry getIndexEntry(int indexId) {
return persistence
.query("SELECT index_value, uid, document_name FROM index_entries WHERE index_id = ?")
.query(
"SELECT array_value, directional_value, uid, document_name FROM index_entries WHERE index_id = ?")
.binding(indexId)
.firstValue(
row ->
row == null
? null
: new IndexEntry(indexId, row.getBlob(0), row.getString(1), row.getString(2)));
: new IndexEntry(
indexId,
row.getBlob(0),
row.getBlob(1),
row.getString(2),
row.getString(3)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ public Index encodeFieldIndex(FieldIndex fieldIndex) {
// queries against each collection separately.
index.setQueryScope(Index.QueryScope.COLLECTION_GROUP);

for (FieldIndex.Segment segment : fieldIndex) {
for (int i = 0; i < fieldIndex.segmentCount(); ++i) {
FieldIndex.Segment segment = fieldIndex.getSegment(i);
Index.IndexField.Builder indexField = Index.IndexField.newBuilder();
indexField.setFieldPath(segment.getFieldPath().canonicalString());
if (segment.getKind() == FieldIndex.Segment.Kind.CONTAINS) {
Expand Down
Loading