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

before and after in the API skips over objects with identical createdTime #2304

Open
Axel-Jacobsen opened this issue Dec 23, 2023 · 4 comments

Comments

@Axel-Jacobsen
Copy link
Contributor

Howdy!

I found a bug where using before or after in the API url will miss objects. I'll paste in the urls that I used for easy reproducibility, I've just been poking around with my browser. I haven't tested on all endpoints, but just going by the docs, I'd guess that this affects any GET that returns a sequence ordered by created time, so GET /v0/markets, GET /v0/users/, GET /v0/groups, and GET /v0/managrams. Here's an example w/ the GET /v0/bets endpoint:

https://api.manifold.markets/v0/bets?&userId=pZOxBAWFRrUJPK6c1a5RI89Fw6x1&before=

As of the time of creating this issue, the 0th bet is C4nKqKXEnPwR6ORnALsa with creation time 1702224677794. The next bet is 1fmJ0FRUEsUc40eYvwMo with creation time 1702152570207. However, it turns out that the next 8 bets also have the same creation time of 1702152570207.

So, if you ask all bets before the 0th, you get bet id 1fmJ0FRUEsUc40eYvwMo as expected:

https://api.manifold.markets/v0/bets?&userId=pZOxBAWFRrUJPK6c1a5RI89Fw6x1&before=C4nKqKXEnPwR6ORnALsa

But, if you ask for all bets after 1fmJ0FRUEsUc40eYvwMo, it skips the next 8 bets, and the first bet you get is IrAVl6uo04tkBax71DUy, creation time 1702152557761.

https://api.manifold.markets/v0/bets?&userId=pZOxBAWFRrUJPK6c1a5RI89Fw6x1&before=1fmJ0FRUEsUc40eYvwMo

I guess that the endpoints are implemented as something like SELECT * FROM BETS WHERE createdTime < beforeId.createdTime (excuse my pseudo sql), which will skip over bets where createdTime == beforeId.createdTime. And, if you do SELECT * FROM BETS WHERE createdTime <= beforeId.createdTime, that will also include before which would be annoying for the API user.

Two solutions that I can think of:

  1. change createdTime to a f64 - ie down to the millisecond. It probably would work, but it would be a breaking change and doesn't get to the core of the issue (but it would be easy!)
  2. change the query to SELECT * FROM BETS WHERE createdTime <= beforeId.createdTime && id != beforeId.id

Also, I wouldn't mind just doing this if someone can point me where to look and what solution you prefer!

Cheers, and Merry Christmas!

@ProgramCrafter
Copy link
Contributor

change the query to SELECT * FROM BETS WHERE createdTime <= beforeId.createdTime && id != beforeId.id

Won't work if there are three bets A@1s, B@1s, C@1s and you've already got A and B; that solution would return A and C.

@Axel-Jacobsen
Copy link
Contributor Author

change the query to SELECT * FROM BETS WHERE createdTime <= beforeId.createdTime && id != beforeId.id

Won't work if there are three bets A@1s, B@1s, C@1s and you've already got A and B; that solution would return A and C.

Ah lol you're right - I guess you just need f64 timestamps eh

Axel-Jacobsen added a commit to Axel-Jacobsen/mmmbacktest that referenced this issue Jan 7, 2024
@sipec
Copy link
Member

sipec commented Feb 1, 2024

yeahh lol we really should do a real pagination method. for most APIs we can now, since we use supabase. the current behavior with before and after should stay the same though since it is semantically correct

I think this would be a pretty good task for an open source contribution ;)

@benmanns
Copy link
Contributor

You can do something like:

SELECT *
FROM bets
WHERE
  (
    createdTime = beforeId.createdTime
    AND id < beforeId.id
  ) OR (
    createdTime < beforeId.createdTime
  )
ORDER BY
  createdTime DESC,
  id DESC

basically using the tuple (createdTime, id). With Postgres directly I think you could actually go even simpler:

SELECT *
FROM bets
WHERE
  (createdTime, id) < (beforeId.createdTime, beforeId.id)
ORDER BY
  createdTime DESC,
  id DESC

though that’s not supported via the public Supabase API.

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

4 participants