Skip to content
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

Migrate dodo #4043

Merged
merged 32 commits into from
Aug 25, 2023
Merged

Migrate dodo #4043

merged 32 commits into from
Aug 25, 2023

Conversation

owen05
Copy link
Contributor

@owen05 owen05 commented Aug 11, 2023

Migrate DODO to DuneSQL and change the name of DODO Aggregator from 'DODO' to 'DODO X'

Thank you for contributing to Spellbook!

Please refer to the top of the readme in the root of Spellbook to learn how to contribute to Spellbook on DuneSQL.

@github-actions
Copy link

github-actions bot commented Aug 11, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@dune-eng
Copy link

Workflow run id 5831069201 approved.

@dune-eng
Copy link

Workflow run id 5831069430 approved.

@owen05
Copy link
Contributor Author

owen05 commented Aug 11, 2023

I have read the CLA Document and I hereby sign the CLA

@dune-eng
Copy link

Workflow run id 5831505926 approved.

@dune-eng
Copy link

Workflow run id 5831506061 approved.

@owen05
Copy link
Contributor Author

owen05 commented Aug 11, 2023

recheck

@dune-eng
Copy link

Workflow run id 5832014811 approved.

@dune-eng
Copy link

Workflow run id 5832014641 approved.

@dune-eng
Copy link

Workflow run id 5832401343 approved.

@dune-eng
Copy link

Workflow run id 5832401563 approved.

@dune-eng
Copy link

Workflow run id 5832559346 approved.

@dune-eng
Copy link

Workflow run id 5832559416 approved.

@dune-eng
Copy link

Workflow run id 5832706047 approved.

@dune-eng
Copy link

Workflow run id 5832706275 approved.

@dune-eng
Copy link

Workflow run id 5832833242 approved.

@dune-eng
Copy link

Workflow run id 5832833362 approved.

github-actions bot added a commit that referenced this pull request Aug 11, 2023
@dune-eng
Copy link

Workflow run id 5832984891 approved.

@dune-eng
Copy link

Workflow run id 5832984419 approved.

@dune-eng
Copy link

Workflow run id 5833129258 approved.

@dune-eng
Copy link

Workflow run id 5833129491 approved.

@dune-eng
Copy link

Workflow run id 5867276001 approved.

@dune-eng
Copy link

Workflow run id 5867275884 approved.

@owen05
Copy link
Contributor Author

owen05 commented Aug 17, 2023

Hey @jeff-dude @Hosuke , How to identify the issue of repeated line, and how to debug it? Currently, I'm stuck at this point.

@Hosuke
Copy link
Collaborator

Hosuke commented Aug 17, 2023

Hey @jeff-dude @Hosuke , How to identify the issue of repeated line, and how to debug it? Currently, I'm stuck at this point.

I will take a look.

@Hosuke
Copy link
Collaborator

Hosuke commented Aug 17, 2023

Hi @owen05

Please take a look at these records:
https://dune.com/queries/2906707

They are duped records with exactly same in all columns.

You may need to add either distinct or add new columns into unique_key to distinguish them.

{% endif %}

UNION ALL

-- DODOFeeRouteProxy
SELECT
evt_block_time AS block_time,
'DODO' AS project,
'DODO X' AS project,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@owen05 I would suggest that you add a proxy field here, to distinguish records.
The proxy field may help you to identify where the dupes come from.

There may be overlapping records, like proxy calls each other.

@jeff-dude
Copy link
Member

hi @owen05 -- we are looking to finalize the migration of dex.trades to dunesql and this is part of that lineage. are you actively looking to finish this, or do you need someone on our side to help finalize?

@Hosuke
Copy link
Collaborator

Hosuke commented Aug 24, 2023

The dupes may be produced during left join, as there are no dupes in source table.
https://dune.com/queries/2965909

The simplest way may be an UNION to solve.

@jeff-dude
Copy link
Member

The dupes may be produced during left join, as there are no dupes in source table. https://dune.com/queries/2965909

The simplest way may be an UNION to solve.

if you have time to pick this up and try to fix, please do, otherwise we'll take it up on our side soon

fromAmount AS token_bought_amount_raw,
returnAmount AS token_sold_amount_raw,
cast(NULL as double) AS amount_usd,
fromToken AS token_bought_address,
toToken AS token_sold_address,
contract_address AS project_contract_address,
evt_tx_hash AS tx_hash,
CAST(ARRAY() as array<bigint>) AS trace_address,
CAST(NULL as array<bigint>) AS trace_address,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem identified:
null comparison involved in the incremental updates.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirmed with @owen05, all trace_address in dodo_aggregator.trades are null.

@Hosuke
Copy link
Collaborator

Hosuke commented Aug 24, 2023

The dupes may be produced during left join, as there are no dupes in source table. https://dune.com/queries/2965909
The simplest way may be an UNION to solve.

if you have time to pick this up and try to fix, please do, otherwise we'll take it up on our side soon

I have tried to fix the dupes issue.

@jeff-dude
Copy link
Member

The dupes may be produced during left join, as there are no dupes in source table. https://dune.com/queries/2965909
The simplest way may be an UNION to solve.

if you have time to pick this up and try to fix, please do, otherwise we'll take it up on our side soon

I have tried to fix the dupes issue.

amazing! much appreciated. i will look to finalize soon

fromAmount AS token_bought_amount_raw,
returnAmount AS token_sold_amount_raw,
cast(NULL as double) AS amount_usd,
fromToken AS token_bought_address,
toToken AS token_sold_address,
contract_address AS project_contract_address,
evt_tx_hash AS tx_hash,
CAST(ARRAY() as array<bigint>) AS trace_address,
CAST(NULL AS ARRAY<bigint>) AS trace_address,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think rather than force null and have incremental logic break, we should standardize to match other models in the dex_aggregator.trades lineage to default to ARRAY[-1] as trace_address, then re-add to the unique key, so that it's consistent across all models in lineage

this is based on other aggregators i've seen migrated recently, where the -1 was used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it

@dune-eng
Copy link

Workflow run id 5977336515 approved.

@dune-eng
Copy link

Workflow run id 5977336402 approved.

Copy link
Member

@jeff-dude jeff-dude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed a final commit to add trace_address back to unique keys/tests for aggregators specifically, to keep consistent with other projects in the lineage

@jeff-dude jeff-dude added ready-for-merging and removed in review Assignee is currently reviewing the PR labels Aug 25, 2023
@jeff-dude jeff-dude merged commit 8742f54 into duneanalytics:main Aug 25, 2023
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants