Skip to content

Revert "Make != and NOT_IN publicly available (#3772)" #3781

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

Closed
wants to merge 1 commit into from
Closed

Revert "Make != and NOT_IN publicly available (#3772)" #3781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 0 additions & 7 deletions .changeset/shiny-forks-pull.md
View file
Open in desktop

This file was deleted.

7 changes: 2 additions & 5 deletions packages/firebase/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9073,20 +9073,17 @@ declare namespace firebase.firestore {

/**
* Filter conditions in a `Query.where()` clause are specified using the
* strings '<', '<=', '==', '!=', '>=', '>', 'array-contains', 'in',
* 'array-contains-any', and 'not-in'.
* strings '<', '<=', '==', '>=', '>', 'array-contains', 'in', and 'array-contains-any'.
*/
export type WhereFilterOp =
| '<'
| '<='
| '=='
| '!='
| '>='
| '>'
| 'array-contains'
| 'in'
| 'array-contains-any'
| 'not-in';
| 'array-contains-any';

/**
* A `Query` refers to a Query which you can read or listen to. You can also
Expand Down
4 changes: 1 addition & 3 deletions packages/firestore-types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,11 @@ export type WhereFilterOp =
| '<'
| '<='
| '=='
| '!='
| '>='
| '>'
| 'array-contains'
| 'in'
| 'array-contains-any'
| 'not-in';
| 'array-contains-any';

export class Query<T = DocumentData> {
protected constructor();
Expand Down
4 changes: 1 addition & 3 deletions packages/firestore/exp-types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,13 +277,11 @@ export type WhereFilterOp =
| '<'
| '<='
| '=='
| '!='
| '>='
| '>'
| 'array-contains'
| 'in'
| 'array-contains-any'
| 'not-in';
| 'array-contains-any';

export class Query<T = DocumentData> {
protected constructor();
Expand Down
4 changes: 1 addition & 3 deletions packages/firestore/lite-types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,11 @@ export type WhereFilterOp =
| '<'
| '<='
| '=='
| '!='
| '>='
| '>'
| 'array-contains'
| 'in'
| 'array-contains-any'
| 'not-in';
| 'array-contains-any';

export class Query<T = DocumentData> {
protected constructor();
Expand Down
32 changes: 18 additions & 14 deletions packages/firestore/src/api/database.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1882,20 +1882,24 @@ export class Query<T = DocumentData> implements PublicQuery<T> {
validateExactNumberOfArgs('Query.where', arguments, 3);
validateDefined('Query.where', 3, value);

// Enumerated from the WhereFilterOp type in index.d.ts.
const whereFilterOpEnums = [
Operator.LESS_THAN,
Operator.LESS_THAN_OR_EQUAL,
Operator.EQUAL,
Operator.NOT_EQUAL,
Operator.GREATER_THAN_OR_EQUAL,
Operator.GREATER_THAN,
Operator.ARRAY_CONTAINS,
Operator.IN,
Operator.ARRAY_CONTAINS_ANY,
Operator.NOT_IN
];
const op = validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr);
// TODO(ne-queries): Add 'not-in' and '!=' to validation.
let op: Operator;
if ((opStr as unknown) === 'not-in' || (opStr as unknown) === '!=') {
op = opStr as Operator;
} else {
// Enumerated from the WhereFilterOp type in index.d.ts.
const whereFilterOpEnums = [
Operator.LESS_THAN,
Operator.LESS_THAN_OR_EQUAL,
Operator.EQUAL,
Operator.GREATER_THAN_OR_EQUAL,
Operator.GREATER_THAN,
Operator.ARRAY_CONTAINS,
Operator.IN,
Operator.ARRAY_CONTAINS_ANY
];
op = validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr);
}

const fieldPath = fieldPathFromArgument('Query.where', field);
const filter = newQueryFilter(
Expand Down
9 changes: 5 additions & 4 deletions packages/firestore/src/core/query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,17 +605,19 @@ export class FieldFilter extends Filter {
}
} else if (isNullValue(value)) {
if (op !== Operator.EQUAL && op !== Operator.NOT_EQUAL) {
// TODO(ne-queries): Update error message to include != comparison.
throw new FirestoreError(
Code.INVALID_ARGUMENT,
"Invalid query. Null only supports '==' and '!=' comparisons."
'Invalid query. Null supports only equality comparisons.'
);
}
return new FieldFilter(field, op, value);
} else if (isNanValue(value)) {
if (op !== Operator.EQUAL && op !== Operator.NOT_EQUAL) {
// TODO(ne-queries): Update error message to include != comparison.
throw new FirestoreError(
Code.INVALID_ARGUMENT,
"Invalid query. NaN only supports '==' and '!=' comparisons."
'Invalid query. NaN supports only equality comparisons.'
);
}
return new FieldFilter(field, op, value);
Expand Down Expand Up @@ -709,8 +711,7 @@ export class FieldFilter extends Filter {
Operator.LESS_THAN_OR_EQUAL,
Operator.GREATER_THAN,
Operator.GREATER_THAN_OR_EQUAL,
Operator.NOT_EQUAL,
Operator.NOT_IN
Operator.NOT_EQUAL
].indexOf(this.op) >= 0
);
}
Expand Down
30 changes: 26 additions & 4 deletions packages/firestore/test/integration/api/database.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import { EventsAccumulator } from '../util/events_accumulator';
import * as firebaseExport from '../util/firebase_export';
import {
apiDescribe,
notEqualOp,
notInOp,
withTestCollection,
withTestDb,
withTestDbs,
Expand Down Expand Up @@ -642,6 +644,14 @@ apiDescribe('Database', (persistence: boolean) => {
});
});

it('inequality and NOT_IN on different fields works', () => {
return withTestCollection(persistence, {}, async coll => {
expect(() =>
coll.where('x', '>=', 32).where('y', notInOp, [1, 2])
).not.to.throw();
});
});

it('inequality and array-contains-any on different fields works', () => {
return withTestCollection(persistence, {}, async coll => {
expect(() =>
Expand All @@ -659,8 +669,12 @@ apiDescribe('Database', (persistence: boolean) => {

it('!= same as orderBy works.', () => {
return withTestCollection(persistence, {}, async coll => {
expect(() => coll.where('x', '!=', 32).orderBy('x')).not.to.throw();
expect(() => coll.orderBy('x').where('x', '!=', 32)).not.to.throw();
expect(() =>
coll.where('x', notEqualOp, 32).orderBy('x')
).not.to.throw();
expect(() =>
coll.orderBy('x').where('x', notEqualOp, 32)
).not.to.throw();
});
});

Expand All @@ -678,10 +692,10 @@ apiDescribe('Database', (persistence: boolean) => {
it('!= same as first orderBy works.', () => {
return withTestCollection(persistence, {}, async coll => {
expect(() =>
coll.where('x', '!=', 32).orderBy('x').orderBy('y')
coll.where('x', notEqualOp, 32).orderBy('x').orderBy('y')
).not.to.throw();
expect(() =>
coll.orderBy('x').where('x', '!=', 32).orderBy('y')
coll.orderBy('x').where('x', notEqualOp, 32).orderBy('y')
).not.to.throw();
});
});
Expand All @@ -706,6 +720,14 @@ apiDescribe('Database', (persistence: boolean) => {
});
});

it('NOT_IN different than orderBy works', () => {
return withTestCollection(persistence, {}, async coll => {
expect(() =>
coll.orderBy('x').where('y', notInOp, [1, 2])
).not.to.throw();
});
});

it('array-contains-any different than orderBy works', () => {
return withTestCollection(persistence, {}, async coll => {
expect(() =>
Expand Down
Loading