-
Notifications
You must be signed in to change notification settings - Fork 302
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
pindexer: dex-explorer: implement low high in summary #4931
Conversation
24d4b0c
to
75af13a
Compare
Locally I've run |
@@ -443,7 +443,7 @@ mod summary { | |||
SELECT | |||
COALESCE(SUM(direct_volume), 0.0) AS direct_volume_over_window, | |||
COALESCE(SUM(swap_volume), 0.0) AS swap_volume_over_window, | |||
COALESCE(SUM(trades), 0.0) AS trades_over_window | |||
COALESCE(SUM(trades), 0.0) AS trades_over_window, |
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.
N.B. if we moved these large queries to a discrete text file and used include_str
on them, then we could lint them with sqlfluff
cf. #4759 (comment). I tried on this specific error and the tool reported:
L: 40 | P: 5 | PRS | Line 40, Position 5: Found unparsable section:
| 'COALESCE(MIN(price), 0.0) AS low,\n CO...'
Not convinced that it's worth moving the queries out of the code just to aid a linter, though. I'd thought that sqlx
would catch that syntax error at compile time, but no.
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.
Describe your changes
Closes #4929.
This was surprisingly simple, we were already doing an aggregate calculation over the window (summing values) so this was just a matter of adding a low and a high.
To test, index against main and check that nothing else has changed besides the addition of two new fields.
Checklist before requesting a review
I have added guiding text to explain how a reviewer should test these changes.
If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: