-
Notifications
You must be signed in to change notification settings - Fork 16
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
Refactor dynamodb saving, add secondary index #28
Conversation
- Some pool attributes are stored as strings in Javascript by should be numbers in DynamoDB so they can be compared with gt/lt and used as indexes. For example: totalShares and totalLiquidity. To do this we need to change the way DynamoDB inserts data to specify the exact type of each field and specify that these BigNumber fields should be numbers. - Remove a lot of console.logs and change them to debug logs instead.
64bfab2
to
283f3ca
Compare
…n't crash on undefined/null fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, not much I can really comment on here but it looks good.
case 'Number': | ||
default: | ||
unmarshalledItem[key] = Number(value.value) | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the Number case be handled here and why is there his break statement at the bottom?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Number just falls through to the default. Don't need the break at the bottom I guess
We need the ability for the GraphQL endpoint to filter by pools that have > n
totalShares
and to be able to sort bytotalLiquidity
and possibly other things in the future. The problem is these are stored as strings in DynamoDB so aren't query-able as numbers. I was previously just usingNumber()
before saving but this only allows 16 significant digits and rounds larger numbers so we lose precision.This is a refactor to insert the data with types specified for every field so that we can have strings in JavaScript but numbers in the database (and we can easily specify the types for all fields in the future). It adds a schema which is equivalent to the graphQL schema and specifies fields that are BigNumbers or Numbers. Both are stored in DynamoDB as numbers, but BigNumbers are strings in Javascript and Numbers are Numbers.
This PR also adds a secondary index to the pools table of chainid + totalLiquidity so we can easily query for all pools on a chain sorted by totalLiquidity.