search

kodadot / nft-gallery

Generative Art Marketplace
https://koda.art
MIT License
634 stars 361 forks source link

Optimize query on TokenEntity #7462

Closed vikiival closed 11 months ago

vikiival commented 1 year ago

What happened?

One of the reasons why #7419 on AHP was caused is this huge and scary query. It does so many joins that squid database decided to give up.

Screenshot 2023-10-02 at 12 59 10

query tokenListWithSearch($first: Int!, $offset: Int, $denyList: [String!], $search: [NFTEntityWhereInput!], $orderBy: [TokenEntityOrderByInput!]) {
  tokenEntities: tokenEntities(
    orderBy: $orderBy
    limit: $first
    offset: $offset
    where: {nfts_some: {burned_eq: false, issuer_not_in: $denyList, metadata_not_eq: "", AND: $search}}
  ) {
    id
    blockNumber
    hash
    image
    media
    name
    updatedAt
    createdAt
    count
    nfts(
      where: {burned_eq: false, issuer_not_in: $denyList, metadata_not_eq: "", AND: $search}
    ) {
      ...nft
      ...nftDetails
      royalty
      recipient
      collection {
        id
        name
        floor
        __typename
      }
      meta {
        ...baseMeta
        __typename
      }
      __typename
    }
    __typename
  }
  tokenEntitiesConnection(
    where: {nfts_some: {burned_eq: false, issuer_not_in: $denyList, metadata_not_eq: "", AND: $search}}
    orderBy: blockNumber_DESC
  ) {
    totalCount
    __typename
  }
}

fragment nft on NFTEntity {
  id
  metadata
  issuer
  currentOwner
  blockNumber
  burned
  __typename
}

fragment nftDetails on NFTEntity {
  name
  sn
  price
  __typename
}

fragment baseMeta on MetadataEntity {
  id
  image
  animationUrl
  name
  description
  __typename
}

{
    "search": [
        {}
    ],
    "first": 40,
    "offset": 1,
    "denyList": [
        "128qRiVjxU3TuT37tg7AX99zwqfPtj2t4nDKUv9Dvi5wzxuF",
        "155HWw3J9jyYphMm5is4vp9Bzj7ZRRd6HEzCPdWd8cq97KfT"
    ],
    "orderBy": [
        "blockNumber_DESC"
    ]
}

Please reproduce in steps

Expected Behavior

The task:

Delete unnecessary information from the query so I does not make too many complicated joins.

Blocked by Questions on @daiagi

  1. Do we really need to query all available NFTs?
  2. Does all nfts need all metadata?

What browsers are you seeing the problem on?

No response

At which address did you encounter bug?

No response

Are you logged in?

None

Which wallet you are using?

No response

At which chain did you encounter bug?

No response

Screenshots

photo_2023-10-02 12 59 21

Relevant log output

No response

Payment link for reward

https://kodadot.xyz/ksm/transfer/?target=%3CMy_Kusama_Address_check_https://github.com/kodadot/nft-gallery/blob/main/CONTRIBUTING.md#creating-your-ksm-address%3E

Code of Conduct

daiagi commented 1 year ago

@vikiival if you agree, I'll undo the change on the FE that uses TokenEntity i.e. revert back to using plain old NFTEntity and reimplement TokenEntity again

more data should be on tokenEntity, to avoid needing to query lots of nft data on the FE

vikiival commented 1 year ago

more data should be on tokenEntity, to avoid needing to query lots of nft data on the FE

Yes. Which data do we need?

daiagi commented 1 year ago

we need

id name image media count (unburnt) blockNumber (most recent NFT) - for sort updatedAt - for sort price (of cheapest nft) - for sort and "buy now" filter sn - for sort owners - for "own" filter collection: { id name floor price }

nftsForListing: { id owner } nftForBuying { id price

}

Gaps :

count (unburnt) - currently returning total count (burned included) price (of cheapest nft) sn owners: string[]

collection floor price, rn getting it requires another query over nfts

Currently possible query

query TokenEntites($userid: String) {
  tokenEntities(orderBy: updatedAt_DESC, limit:50,  ) {
    id
    name
    image
    media
    count
    updatedAt
    blockNumber

    nftForBuying: nfts(
      where: {
        currentOwner_not_eq: $userid,
        price_gt: "0",
        burned_eq: false},
        limit: 1,
      orderBy: price_ASC) 
    {
      id
      price
    }
    nftsForListing: nfts(
      where: {
        currentOwner_eq: $userid,
        price_eq: "0",
        burned_eq: false
      } )
    {
      id
      currentOwner

    }
    collection {
      floorPrice :nfts(where: {price_gt: "0"} ,orderBy: price_ASC, limit:1) {
        price
      }
      name
      id
    }

  }
}
vikiival commented 1 year ago

id, name, image, media

count (unburnt)

blockNumber (most recent NFT) - for sort

updatedAt - for sort

price (of cheapest nft) - for sort and "buy now" filter

sn - for sort

This filter does not make sense imo as there is nothing I can represent as serial number.

owners - for "own" filter

Rather cached resolver

collection

So that means TokenEntity is expecting same image under one collection right?

nftsForListing

Like count or why?

nftForBuying

same as above

Gaps :

owners: string[] collection floor price, rn getting it requires another query over nfts

daiagi commented 1 year ago

This filter does not make sense imo as there is nothing I can represent as serial number.

ok, but that will mean we will need to drop sort by sequence

So that means TokenEntity is expecting same image under one collection right?

correct, all of the nfts under one collection that share same ipfs URL are a single TokenEntity

Like count or why?

no. list of id's image

this btn will add all these ids's into listing cart it's either this or query for all of the nft's under token entity and let the FE sort out which is what right?

same as above

same for the buy now action button. need to know which nft id to buy (the cheapest)

vikiival commented 1 year ago

no. list of id's

Can be lazy loaded on demand. Do not see value to overfetch the client for that.

correct, all of the nfts under one collection that share same ipfs URL are a single TokenEntity

Collection Is already present, same as blockNumber and updated at

https://github.com/kodadot/stick/blob/f870effae7141dde9668e94dfe771f6fdd7ff596/schema.graphql#L32

kodabot commented 12 months ago

ASSIGNED - @daiagi 🔒 LOCKED -> Friday, October 13th 2023, 04:09:35 UTC -> 36 hours

daiagi commented 12 months ago

new query will look something like this


query tokenListWithSearch(
  $first: Int!
  $offset: Int
  $orderBy: [String!] = ["blockNumber_DESC"]
  $price_gte: Float
  $price_gt: Float
  $price_lte: Float
  $owner: String
  $denyList: [String!]
) {
  tokenEntities: tokenEntityList(
    owner: $owner
    denyList: $denyList
    orderBy: $orderBy
    limit: $first
    offset: $offset
    price_gte: $price_gte
    price_gt: $price_gt
    price_lte: $price_lte
  ) {
    id
    name
    image
    media
    metadata
    supply
    cheapest {
      id
      price
      currentOwner
    }
    collection {
      id
      name
    }
    meta {
      id
      image
      animationUrl
      description
    }
  }
    tokenEntityCount(
    owner: $owner
    denyList: $denyList
    price_gte: $price_gte
    price_gt: $price_gt
    price_lte: $price_lte
  ) {
    totalCount
  }

}

https://github.com/kodadot/stick/pull/110

and I am currently integrating the front end using local machine indexer + graphql server

PR will come soon

vikiival commented 12 months ago

Released Stick v5 and Speck v5 with this change

daiagi commented 12 months ago

Thanks viki I'm about a day away from releasing the pr for front end

kodabot commented 11 months ago

ASSIGNMENT EXPIRED - @daiagi has been unassigned.

vikiival commented 11 months ago

New release of v5.0.1 in kodadot/stick#115 and kodadot/stick#116