Skip to content
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

fix: Add tests for multiple inequality support #1878

Merged
merged 19 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from 12 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
85 changes: 47 additions & 38 deletions dev/src/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -710,9 +710,6 @@ abstract class FilterInternal {
/** Returns a list of all filters that are contained within this filter */
abstract getFilters(): FilterInternal[];

/** Returns the field of the first filter that's an inequality, or null if none. */
abstract getFirstInequalityField(): FieldPath | null;

/** Returns the proto representation of this filter */
abstract toProto(): Filter;

Expand All @@ -735,13 +732,6 @@ class CompositeFilterInternal extends FilterInternal {
return this.filters;
}

public getFirstInequalityField(): FieldPath | null {
return (
this.getFlattenedFilters().find(filter => filter.isInequalityFilter())
?.field ?? null
);
}

public isConjunction(): boolean {
return this.operator === 'AND';
}
Expand Down Expand Up @@ -807,14 +797,6 @@ class FieldFilterInternal extends FilterInternal {
return [this];
}

getFirstInequalityField(): FieldPath | null {
if (this.isInequalityFilter()) {
return this.field;
} else {
return null;
}
}

/**
* @param serializer The Firestore serializer
* @param field The path of the property value to compare.
Expand Down Expand Up @@ -843,6 +825,7 @@ class FieldFilterInternal extends FilterInternal {
case 'GREATER_THAN_OR_EQUAL':
case 'LESS_THAN':
case 'LESS_THAN_OR_EQUAL':
case 'NOT_EQUAL':
ehsannas marked this conversation as resolved.
Show resolved Hide resolved
return true;
default:
return false;
Expand Down Expand Up @@ -1821,6 +1804,25 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
);
}

/**
* Returns the sorted array of inequality filter fields used in this query.
ehsannas marked this conversation as resolved.
Show resolved Hide resolved
*
* @return An array of inequality filter fields sorted lexicographically by FieldPath.
*/
private getInequalityFilterFields(): FieldPath[] {
const inequalityFields: FieldPath[] = [];

for (const filter of this._queryOptions.filters) {
for (const subFilter of filter.getFlattenedFilters()) {
if (subFilter.isInequalityFilter()) {
inequalityFields.push(subFilter.field);
}
}
}

return inequalityFields.sort((a, b) => a.compareTo(b));
}

/**
* Computes the backend ordering semantics for DocumentSnapshot cursors.
*
Expand All @@ -1846,29 +1848,36 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
}
ehsannas marked this conversation as resolved.
Show resolved Hide resolved

const fieldOrders = this._queryOptions.fieldOrders.slice();

// If no explicit ordering is specified, use the first inequality to
// define an implicit order.
if (fieldOrders.length === 0) {
for (const filter of this._queryOptions.filters) {
const fieldReference = filter.getFirstInequalityField();
if (fieldReference !== null) {
fieldOrders.push(new FieldOrder(fieldReference));
break;
}
const fieldsNormalized = new Set([
...fieldOrders.map(item => item.field.toString()),
]);

/** The order of the implicit ordering always matches the last explicit order by. */
const lastDirection =
fieldOrders.length === 0
? directionOperators.ASC
: fieldOrders[fieldOrders.length - 1].direction;

/**
* Any inequality fields not explicitly ordered should be implicitly ordered in a
* lexicographical order. When there are multiple inequality filters on the same field, the
* field should be added only once.
* Note: getInequalityFilterFields function sorts the key field before
* other fields. However, we want the key field to be sorted last.
*/
const inequalityFields = this.getInequalityFilterFields();
for (const field of inequalityFields) {
if (
!fieldsNormalized.has(field.toString()) &&
!field.isEqual(FieldPath.documentId())
) {
fieldOrders.push(new FieldOrder(field, lastDirection));
fieldsNormalized.add(field.toString());
}
}

const hasDocumentId = !!fieldOrders.find(fieldOrder =>
FieldPath.documentId().isEqual(fieldOrder.field)
);
if (!hasDocumentId) {
// Add implicit sorting by name, using the last specified direction.
const lastDirection =
fieldOrders.length === 0
? directionOperators.ASC
: fieldOrders[fieldOrders.length - 1].direction;

// Add the document key field to the last if it is not explicitly ordered.
if (!fieldsNormalized.has(FieldPath.documentId().toString())) {
fieldOrders.push(new FieldOrder(FieldPath.documentId(), lastDirection));
}

Expand Down
Loading