Skip to content

Support > and != on the same field #3454

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 1 commit into from
Feb 17, 2022
Merged

Conversation

schmidt-sebastian
Copy link
Contributor

This adds the ability to use inequality and equality filters on the same field.

public @Nullable List<Value> getNotInValues(FieldIndex fieldIndex) {
List<Value> values = new ArrayList<>();

public @Nullable Collection<Value> getNotInValues(FieldIndex fieldIndex) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method now supports a == 1 && a != 2, which now correctly just returns a != 2 instead of the concatenation of both.

Value.newBuilder()
.setArrayValue(ArrayValue.newBuilder().addValues(MIN_VALUE))
.build();
filterValue = Values.MIN_VALUE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not In and Not Equal use the same bound, which makes the comparison logic at the end of this method work as we do not want to compare to an array but to an actual minimum.

@@ -740,8 +740,10 @@ private boolean isInFilter(Target target, FieldPath fieldPath) {
for (Filter filter : target.getFilters()) {
if ((filter instanceof FieldFilter) && ((FieldFilter) filter).getField().equals(fieldPath)) {
FieldFilter.Operator operator = ((FieldFilter) filter).getOperator();
return operator.equals(FieldFilter.Operator.IN)
|| operator.equals(FieldFilter.Operator.NOT_IN);
if (operator.equals(FieldFilter.Operator.IN)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A not-in filter does not have to appear on the first filter for a field. If there are multiple filters for the field path, not in can appear later.

@@ -41,16 +41,15 @@
public static final Value NULL_VALUE =
Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build();
public static final Value MIN_VALUE = NULL_VALUE;
private static final Value MAX_VALUE_TYPE = Value.newBuilder().setStringValue("__max__").build();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All changes in this file are to support correct ordering for MAX_VALUE.

@google-oss-bot
Copy link
Contributor

Size Report 1

Affected Products

  • firebase-firestore

    TypeBase (6e96da6)Merge (053a5dc)Diff
    aar1.24 MB1.24 MB+33 B (+0.0%)
    apk (release)3.37 MB3.37 MB+248 B (+0.0%)

Test Logs

Notes

  • Commit (053a5dc) is created by Prow via merging PR base commit (6e96da6) and head commit (65f653f).

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/zu7Zox2Dy7.html

@google-oss-bot
Copy link
Contributor

Coverage Report 1

Affected Products

  • firebase-firestore

    Overall coverage changed from 45.56% (6e96da6) to 45.55% (053a5dc) by -0.02%.

    FilenameBase (6e96da6)Merge (053a5dc)Diff
    SQLiteIndexManager.java98.68%98.68%+0.00%
    Target.java96.00%95.83%-0.17%
    Values.java95.58%95.22%-0.36%

Test Logs

Notes

  • Commit (053a5dc) is created by Prow via merging PR base commit (6e96da6) and head commit (65f653f).
  • Run gradle <product>:checkCoverage to produce HTML coverage reports locally. After gradle commands finished, report files can be found under <product-build-dir>/reports/jacoco/.

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/CGzdTk2o0e.html

Copy link
Contributor

@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

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

Thanks for the comments.

@wu-hui wu-hui assigned schmidt-sebastian and unassigned wu-hui Feb 17, 2022
@schmidt-sebastian schmidt-sebastian merged commit d54c7c0 into master Feb 17, 2022
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/innotin branch February 17, 2022 17:29
@firebase firebase locked and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants