-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add raw order data wrapper query #80
base: main
Are you sure you want to change the base?
Conversation
from_hex(d.solver) as solver, | ||
from_hex(d.data.quote_solver) as quote_solver, --noqa: RF01 | ||
from_hex(d.tx_hash) as tx_hash, | ||
cast(d.data.surplus_fee as decimal(38, 0)) as surplus_fee, --noqa: RF01 |
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.
Is there precedence for this number of decimals?
Our database uses decimal(78, 0) for most things, which can hold a (u)int256.
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.
If I run
select cast(10 as decimal(39,0))
I get an error Error: DECIMAL precision must be in range [1, 38]: 39
So i decided to max it out, but didn't think this through much tbh
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.
past_data_ethereum as ( | ||
select | ||
s.environment, | ||
-1 as auction_id, |
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.
Why is this encoded as -1
and not just Null
?
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.
yeah i see your point.... Will update it.
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.
Generally looks fine.
The comments on the other PR for batch rewards still apply. E.g. if this is automatically generated, it should be moved out of this repo at some point (e.g. now).
I added both queries so that there is a way to review them. I agree that we could actually just use these PRs to review the queries but not merge them. |
Similar to PR #77 , for raw order data.
https://dune.com/queries/4364122