needs to use {{this}} format for incremental

[andrewhong5297]

Brief comments on the purpose of your changes:

We were getting duplicates with this always running with one day of history, I've changed it to only run if the block_time is in the next day.

For Dune Engine V2

I've checked that:

General checks:

• I tested the query on dune.com after compiling the model with dbt compile (compiled queries are written to the target directory)
• I used "refs" to reference other models in this repo and "sources" to reference raw or decoded tables
• if adding a new model, I added a test
• the filename is unique and ends with .sql
• each sql file is a select statement and has only one view, table or function defined
• column names are lowercase_snake_cased
• if adding a new model, I edited the dbt project YAML file with new directory path for both models and seeds (if applicable)
• if wanting to expose a model in the UI (Dune data explorer), I added a post-hook in the JINJA config to add metadata (blockchains, sector/project, name and contributor Dune usernames)

Pricing checks:

• coin_id represents the ID of the coin on coinpaprika.com
• all the coins are active on coinpaprika.com (please remove inactive ones)

Join logic:

• if joining to base table (i.e. ethereum transactions or traces), I looked to make it an inner join if possible

Incremental logic:

• I used is_incremental & not is_incremental jinja block filters on both base tables and decoded tables
  • where block_time >= date_trunc("day", now() - interval '1 week')
• if joining to base table (i.e. ethereum transactions or traces), I applied join condition where block_time >= date_trunc("day", now() - interval '1 week')
• if joining to prices view, I applied join condition where minute >= date_trunc("day", now() - interval '1 week')

[andrewhong5297]

Expected a schema version of "https://schemas.getdbt.com/dbt/manifest/v6.json" in manifest.json, but found "https://schemas.getdbt.com/dbt/manifest/v8.json". Are you running with a different version of dbt?

hmm, seems like something is wrong with config? cc @jeff-dude

[Hosuke]

{{this}} provides a more precise incremental updating option.
Original idea is having an overlapping region when incremental update, to ensure 100% coverage of new incoming records, and dbt uses unique_key in the config block to filter out the duplicated/existed records when updating.

[0xRobin]

should probably use >= here to not drop later incremental updates from the same day?

[Hosuke]

since day is derived from date_trunc(...), it represents the lower bound of the time, I think > will include all events in that date after UTC 00:00.

[andrewhong5297]

We don't want to update on same day, only next day. To prevent duplicates.

There is the edge case of missing an address if their balance changes later that day but not the next day.

But ignoring that case for now!

[0xRobin]

ah @Hosuke you're right! I was confused but indeed because we compare block_time with the day timestamp this works and there's no real difference between > and >=. 👍

[0xRobin]

CI issues should be resolved now, I've triggered a re-run

[jeff-dude]

can you confirm the failing test on duplicates?

[andrewhong5297]

can you confirm the failing test on duplicates?

Looking

[andrewhong5297]

the test schema is gone, but I really can't see any reason there would be duplicates given we're using a window function.

[andrewhong5297]

@jeff-dude I'm like 100% confident there should be no duplicates - can we merge?

[jeff-dude]

@jeff-dude I'm like 100% confident there should be no duplicates - can we merge?

if there are dupes here, there'll be dupes in prod. have you tried compiling the model, then placing in CTE and running group by/having > 1 on the unique fields? you can likely start with a subset of data for faster results, then open up more if nothing returns

[Hosuke]

I think I know the cause of the bug,
there is a tiny time diff in updated_at(1st time in full-refresh, 2nd time in incremental refresh).
Perhaps the engine is hard to compare NULL value for token_mint_address.
Here is an example:
https://dune.com/queries/2493263?d=1

[Hosuke]

I tried to fix the problem by giving a default empty string '' to NULL token_mint_address, and currently empty string is not equal to NULL in some versions of spark.

I actually found out this problem is an opening issue in dbt: dbt-labs/dbt-adapters#159

[andrewhong5297]

hmm odd. I've run a group by check and got no nulls https://dune.com/queries/2479876?d=1&q=v2+&sidebar=query-explorer

[andrewhong5297]

thank you @Hosuke !!

[jeff-dude]

I tried to fix the problem by giving a default empty string '' to NULL token_mint_address, and currently empty string is not equal to NULL in some versions of spark.

I actually found out this problem is an opening issue in dbt: dbt-labs/dbt-adapters#159

great catch! we've seen this elsewhere before too. it's a good thing to keep in mind, that unique keys can't be null or the merge statement fails the join condition, leading to duplicate inserts. i wonder if this means we need to apply not null tests to each unique key column 🤔

[jeff-dude]

i'm trying to figure out why the ci test action is picking up previously merged PR spells still. we've had the successful run in prod that should in theory make sure those don't run anymore. i've raised a ticket internally to investigate

we're getting timeouts due to all of that running at the same time

[jeff-dude]

seems like we're getting failures where not null tests were applied (assuming the unique keys)

[andrewhong5297]

seems like we're getting failures where not null tests were applied (assuming the unique keys)

Oh token_mint_address and token_balance can be null, those tests should he removed. Just address and balance should have not null tests.

[jeff-dude]

what's the move here?

[jeff-dude]

closing PRs marked as wip state, prior to the freeze announced and tracked here, we can reopen and reformat post-freeze