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

Unexpected behavior from first & where #148

Closed
matthewlilley opened this issue Jul 13, 2022 · 13 comments
Closed

Unexpected behavior from first & where #148

matthewlilley opened this issue Jul 13, 2022 · 13 comments
Assignees

Comments

@matthewlilley
Copy link

matthewlilley commented Jul 13, 2022

const { liquidityPositions }: { liquidityPositions: LiquidityPosition[] } = await sdk.LiquidityPositions({
first: 1000,
where: { user: MAKER_ADDRESS[chainId] },
})

You would think this would be "WHERE user = MAKER_ADDRESS" return first 1000 enitites where this condition is met.

However, The GraphQL implementation by The Graph does not work in this way, it is scanning the first 1000 rows, and returning entities where that condition is met. This is very confusing behavior.... And means that for the above usage, first would have to be the total number of LiquidityPosition's ever known in the system, regardless of user.

@matthewlilley
Copy link
Author

matthewlilley commented Jul 13, 2022

Unfortunately, this makes auto pagination pretty much useless in this context. I think there has been an implementation error in the creation of auto pagination, maybe because this behavior is odd. You cannot use first/skip as a limit/offset. It just doesn't work the same way, and isn't performant either.

Would like to hear your thoughts on this: @dotansimha

@ardatan
Copy link
Member

ardatan commented Jul 19, 2022

@dotansimha I'm not sure how we can solve this on the client side. Any ideas?

@matthewlilley
Copy link
Author

@ardatan this is a bit of a tricky one. The alternative is querying this way around but first > 1000 does nothing.

#149

@ardatan
Copy link
Member

ardatan commented Jul 22, 2022

@matthewlilley I think this is an unexpected behavior of graph-node and I don't think this is something needs to be fixed on Graph Node itself.

@matthewlilley
Copy link
Author

@ardatan Yes, it's unusual behavior of The Graph node. Sometimes we do need to get all records for a particular reason though, in our previous pager we used this method to speed it up.

https://thegraph.com/docs/en/querying/graphql-api/#example-using-and-2

@ardatan
Copy link
Member

ardatan commented Jul 23, 2022

@dotansimha @kamilkisiela Should we move this issue to graph-node?

@dotansimha
Copy link
Collaborator

@dotansimha @kamilkisiela Should we move this issue to graph-node?

I'm working on a reproduction and then will open an issue with all the details. Thanks!

@ardatan ardatan assigned dotansimha and unassigned ardatan Aug 23, 2022
@dotansimha dotansimha moved this to In Progress in GraphQL API Aug 25, 2022
@dotansimha
Copy link
Collaborator

Hi @matthewlilley , I tested ~10 subgraphs with different setups ,but this does not reproduce at all. Can you please share a Subgraph + a query that produces invalid data based on pagination + where filter?

@matthewlilley
Copy link
Author

@dotansimha I've asked Lufy to make a repro. We can reproduce this quite easily in a lot of different cases. Will post shortly.

@dotansimha
Copy link
Collaborator

@dotansimha I've asked Lufy to make a repro. We can reproduce this quite easily in a lot of different cases. Will post shortly.

Thanks! Please keep us updated :) Keeping open

@matthewlilley
Copy link
Author

@dotansimha when you use orderBy in combination with pagination, the results will always be unexpected since the order of operations are wrong with the graph node. There's a write up here:

https://hackmd.io/@UjLXyH-WRSOCu0gweFfdyQ/B1A_XPGIo

@azf20
Copy link
Contributor

azf20 commented Nov 18, 2022

Hey @matthewlilley I think this is expected behaviour, filtering on ID is applied before the orderBy - you need to filter by the field you are ordering by. This is a slight issue with current Graph Node pagination if the orderBy is not distinct. There is work in progress to support cursor-based pagination graphprotocol/graph-node#3214 (this is an older PR, there is more recent work here)

@matthewlilley
Copy link
Author

Yeah, this seems like a limitation of the graph node. Using reserveUSD as the cursor ID isn't a great solution because there's no gaurentee that these are unique.

@dotansimha dotansimha moved this from In Progress to Done in GraphQL API Dec 6, 2022
@saihaj saihaj closed this as completed Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

5 participants