Skip to content
This repository has been archived by the owner on Jul 1, 2024. It is now read-only.

feat: topOffers resolver #52

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: topOffers resolver #52

wants to merge 1 commit into from

Conversation

zhengow
Copy link
Contributor

@zhengow zhengow commented Aug 12, 2022

Thank you for your contribution to the KodaDot Indexer.
👇 _ Let's make a quick check before the contribution.

PR type

  • Bugfix
  • Feature
  • Refactoring

What's new?

Before submitting Pull Request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Optional

  • I've run the indexer and it hasn't failed
  • I've changed schema and performed a migration
  • I found edge cases

Screenshot

  • My contribution is a resolver so I have pic from playground

image

Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

would it not be missing LIMIT in query?

@zhengow
Copy link
Contributor Author

zhengow commented Aug 12, 2022

would it not be missing LIMIT in query?

Yes, it should have.
But some questions:

  1. Why offerStatsResolver doesn't have LIMIT?
  2. How can I also return totalCount while return the top offers nft lists? Because if adding LIMIT in query, it should have pagination component which should know total amount.

@roiLeo
Copy link
Contributor

roiLeo commented Aug 20, 2022

1. Why `offerStatsResolver` doesn't have `LIMIT`?

It looks like offer query don't need LIMIT as it returns only numbers of rows (COUNT)
your topOffer query is returning data and it can be exponential (and potentially very resource-intensive)

2. How can I also return `totalCount` while return the top offers nft lists? Because if adding `LIMIT` in query, it should have pagination component which should know total amount.

Can't help you on that sorry :/
maybe try to get some inspiration frome events

Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

Please use limits :/

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

Successfully merging this pull request may close these issues.

top offers resolver
3 participants