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

Cursor tiebreaker #27

Open
AJDowds opened this issue Jan 22, 2024 · 5 comments
Open

Cursor tiebreaker #27

AJDowds opened this issue Jan 22, 2024 · 5 comments

Comments

@AJDowds
Copy link

AJDowds commented Jan 22, 2024

I've noticed that id isn't working as a tiebreaker for duplicate updatedAt values using the following code:

I know that my ids are all unique and I also know that I do have duplicate updatedAt values in my db

let qb: SelectQueryBuilder<Database, "user" | "userRelationship", any> = db
      .selectFrom("user")
      .select([
        "user.id",
        "user.username",
        "user.createdAt",
        "user.updatedAt",
      ])
      .where("user.id", "!=", context.user.id)

    qb = qb.orderBy("user.updatedAt", "desc")
    qb = qb.orderBy("user.id", "desc")

    const page = await executeWithCursorPagination({
      qb,
      opts: {
        after: after,
        cursorPerRow: true,
        perPage: first,
        fields: [
          { key: "updatedAt", expression: "user.updatedAt", direction: "desc" },
          { key: "id", expression: "user.id", direction: "desc" },
        ],
        parseCursor: (cursor) => {
          const parsed = {
            updatedAt: new Date(cursor.updatedAt),
            id: parseInt(cursor.id, 10),
          }

          console.log("!!parsed", parsed)

          return parsed
        },
      },
    })

    const result = {
      edges: page.rows.map((row) => {
        return {
          cursor: row.$cursor,
          node: { ...row },
        }
      }),
      pageInfo: {
        startCursor: page.startCursor,
        endCursor: page.endCursor,
        hasNextPage: page.hasNextPage,
        hasPrevPage: page.hasPrevPage,
      },
    }

    return result
@EDjur
Copy link

EDjur commented Oct 8, 2024

Hey! I'm looking into potentially using this library. But no ID tiebreakers is a deal-breaker unfortunately. Do you know if this is still the case @Fedaz8 ?

Thanks!

@AJDowds
Copy link
Author

AJDowds commented Oct 8, 2024

Hey! I'm looking into potentially using this library. But no ID tiebreakers is a deal-breaker unfortunately. Do you know if this is still the case @Fedaz8 ?

Thanks!

I've not looked at the project I was using this in for some time now unfortunately. I can't recall exactly what happened with this.

@EDjur
Copy link

EDjur commented Oct 8, 2024

Gotcha - thanks for the swift reply 😊

@charlie-hadden do you know perhaps then? And if not implemented, maybe if you have a pointer to where to look?

If the other parts of this lib works for my use-cases I could look at making a PR potentially.

Thanks!

@charlie-hadden
Copy link
Owner

Sorry I somehow missed this issue when it was first posted up. ID tie breakers should work correctly already. I won't get time to properly confirm until the weekend, but from looking at the sample provided in this issue, I believe the explicit manual sortBy calls may be the issue. With the cursor pagination function the ordering should be left up to this library to handle so that it can reverse the order internally when needed - it gets applied automatically based on the fields option provided.

I'll try and find time at the weekend to make sure there isn't a real issue here, but it you give it a test before then and find that it is an issue, do please let me know.

Thanks

@EDjur
Copy link

EDjur commented Oct 14, 2024

Nice, ok thanks ! Sounds like perhaps it isn't an issue after all then 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants