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

[request] add sub_tx_id to various sector-level spells #5456

Open
jeff-dude opened this issue Feb 27, 2024 · 0 comments
Open

[request] add sub_tx_id to various sector-level spells #5456

jeff-dude opened this issue Feb 27, 2024 · 0 comments
Assignees
Labels
WIP work in progress

Comments

@jeff-dude
Copy link
Member

jeff-dude commented Feb 27, 2024

initiated in #5392 to add new column in tokens_<blockchain>.transfers spells to include a distinct id for each transfer.

requirements:

  • non-null set of unique columns to use in downstream spells
    • for example, in transfers, spells currently use a surrogate key to bypass nulls which DBT doesn't like in merge statements:
      -- We have to create this unique key because evt_index and trace_address can be null
      {{dbt_utils.generate_surrogate_key(['t.block_number', 'tx.index', 't.evt_index', "array_join(t.trace_address, ',')"])}} as unique_key
      

current approach in nft.trades:

  • combine both evt_index and trace_address into sub_tx_id, use sub_tx_id as part of unique key set along with tx_hash and other standard columns
  • to deal with possible collisions, use arbitrary addition to row_number output (more info here)

decision point:

  • unique_key vs. combo of tx_hash and sub_tx_id
    • where unique_key is a random hash on top of fields to create a random surrogate key that has no meaning outside of backend ETL processing
    • where tx_hash is from raw data, sub_tx_id is built to compliment tx_hash to make rows unique

initial thoughts:

  • personally, i have no strong opinion to keep unique_key -- this was simply a solution to avoid nulls in PK values and an out-of-box solution from DBT
  • if sub_tx_id provides more value than just backend ETL processing, such as simplified testing and analysis downstream in queries, then i'm all for it

spellbook-wide impact:

  • as mentioned in original PR, this would be beneficial to apply universally to all sector-level spells. for example, any spell lineages in the models/_sector/ directory
    • it's fine to start with the big ones: dex/nft/token_transfers

edit:
before we start any work, let's get on the same page here. then we can track changes to each sector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP work in progress
Projects
None yet
Development

No branches or pull requests

5 participants