Skip to content

fix(firestore): Correct the cursors when LimitToLast is used#9413

Merged
bhshkh merged 8 commits intogoogleapis:mainfrom
bhshkh:fix/issue-9175
Feb 15, 2024
Merged

fix(firestore): Correct the cursors when LimitToLast is used#9413
bhshkh merged 8 commits intogoogleapis:mainfrom
bhshkh:fix/issue-9175

Conversation

@bhshkh
Copy link
Copy Markdown
Contributor

@bhshkh bhshkh commented Feb 12, 2024

Context:
Consider the Firestore collection has documents with following ids {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}

  1. Before field

While sending a Firestore run query request 2 cursors can be specified(StartAt and EndAt). Cursors have a field called Before which specifies whether the result should include the field at the cursor.

When the user runs below query:

query.OrderBy("id", Asc).StartAt(1).EndAt(5)

The query sent to Firestore service should have:

StructuredQuery{
  StartAt: &Cursor{
       Values: {1},
       Before: true // Why true? described below
  },
  EndAt: &Cursor {
       Values: {5},
       Before: false // Why false? Described below
  },
  OrderBy: []{
       Field: "id",
       Direction: ASC
  }
}

Firestore service behavior:
When Before field of StartAt is true, the response includes the key in StartAt cursor
When Before field of EndAt is true, the response does not include the key in EndAt cursor
So, the above query would return: {1, 2, 3, 4, 5}

  1. LimitToLast
    While sending a Firestore run query request, a limit can be specified which limits the number of results retuned. Taking the previous example:
query.OrderBy("id", Asc).StartAt(1).EndAt(5).Limit(3)

The request sent will; be

StructuredQuery{
  StartAt: &Cursor{
       Values: {1},
       Before: true
  },
  EndAt: &Cursor {
       Values: {5},
       Before: false
  },
  Limit: 3,
  OrderBy: []{
       Field: "id",
       Direction: ASC
  }
}

would return {1, 2, 3}

If user wants only the last 3 results i.e. {3, 4, 5}, the query they would have to use would be:

query.OrderBy("id", Asc).StartAt(1).EndAt(5).LimitToLast(3)

Since the Firestore service does not provide any LimitToLast field out of the box, Go library queries the documents in reverse order and then reverses the result. So, in the above example, the request sent should be:

StructuredQuery{
  StartAt: &Cursor{
       Values: {5}, <= Notice this
       Before: true
  },
  EndAt: &Cursor {
       Values: {1}, <= Notice this
       Before: false
  },
  Limit: 3,
  OrderBy: []{
       Field: "id",
       Direction: DESC <= Notice this
  }
}

This query would return results: {5, 4, 3} which then the library will reverse and return to the user.

Issue:
Below queries return incorrect results

  1. StartAfter(5).EndBefore(1).LimitToLast(5).OrderBy("id", Desc)
    Expected behavior {4, 3, 2}
    Actual behavior {5, 4, 3, 2}

  2. StartAt(5).EndAt(1).LimitToLast(5).OrderBy("id", Desc)
    Expected behavior {5, 4, 3, 2, 1}
    Actual behavior {4, 3, 2}

Cause:

  1. For case 1, request sent should be :
StructuredQuery{
  StartAt: &Cursor{
       Values: {1}, 
       Before: false
  },
  EndAt: &Cursor {
       Values: {5}, 
       Before: true  <====== This is incorrectly being sent as false
  },
  Limit: 5,
  OrderBy: []{
       Field: "id",
       Direction: ASC 
  }
}

But the current request sent has Before: false in EndAt cursor. So, the result contains the value in the EndAt cursor i.e. 5 is available in the results.

  1. For case 2, request should be:
StructuredQuery{
  StartAt: &Cursor{
       Values: {1}, 
       Before: true   <======= This is incorrectly being sent as false
  },
  EndAt: &Cursor {
       Values: {5}, 
       Before: false  <======= This is incorrectly being sent as true
  },
  Limit: 5,
  OrderBy: []{
       Field: "id",
       Direction: ASC 
  }
}

But the current request sent has Before: false in StartAt cursor and Before: true in EndAt cursor. So, the result does not contain the values in the StartAt and EndAt cursors i.e. 1 and 5 are not available in the results.

Fix:
Set the startBefore and endBefore fields correctly

@bhshkh bhshkh requested review from a team February 12, 2024 21:23
@product-auto-label product-auto-label Bot added size: m Pull request size is medium. api: firestore Issues related to the Firestore API. labels Feb 12, 2024
@bhshkh bhshkh requested a review from gkevinzheng February 12, 2024 21:28
@bhshkh bhshkh linked an issue Feb 12, 2024 that may be closed by this pull request
Comment thread firestore/query.go
Comment thread firestore/integration_test.go
@bhshkh bhshkh enabled auto-merge (squash) February 15, 2024 20:35
@bhshkh bhshkh merged commit 2090651 into googleapis:main Feb 15, 2024
@bhshkh bhshkh deleted the fix/issue-9175 branch February 15, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: firestore Issues related to the Firestore API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

firestore: LimitToLast is not working.

3 participants