-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 arbswap to dex.trades_beta
#4869
Add arbswap to dex.trades_beta
#4869
Conversation
Workflow run id 6972573753 approved. |
Workflow run id 6972574012 approved. |
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.
LGTM.✅
Thank you @tomfutago
models/_sector/dex/trades/arbitrum/platforms/arbswap_arbitrum_base_trades.sql
Show resolved
Hide resolved
Workflow run id 7017377274 approved. |
Workflow run id 7017377186 approved. |
{% endif %} | ||
|
||
UNION ALL | ||
|
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.
We may start trying to replace the part before UNION ALL
with the macro.
Workflow run id 7018126055 approved. |
Workflow run id 7018126165 approved. |
Combo of macro + straight select applied, but as macro already contains cte (and already returns final list of columns) - it can't be a direct replacement of current select, but needs to go as a separate nested cte and then union outputs together. If there's better way to go about it - lmk. |
I just checked the compiled code of
|
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.
LGTM.✅ for the macro-hybrid spell
Sample macro-hybrid arbswap_arbitrum_base_trades
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.
i'm good with this approach -- thank you for finding a solution!
@Hosuke let's keep in mind for other projects moving forward.
linked to #4759