Skip to content

Feat!: bring dbt node information through to SQLMesh#5412

Merged
erindru merged 2 commits intomainfrom
erin/dbt-node-info
Sep 23, 2025
Merged

Feat!: bring dbt node information through to SQLMesh#5412
erindru merged 2 commits intomainfrom
erin/dbt-node-info

Conversation

@erindru
Copy link
Collaborator

@erindru erindru commented Sep 19, 2025

This PR does the following:

  • Remove dbt_name from _Node and replace it with dbt_node_info (as opposed to adding more top-level fields directly to _Node)
  • Since this gets saved to state, I also added dbt_node_info to the model metadata values to help ensure that state stays in sync with reality
  • Added a migration to migrate dbt_name -> dbt_node_info
    • This is an incomplete migration because dbt_name only represents the unique_id and not the other fields
    • However, since it's part of the metadata hash, after migration, on the next plan, the user will get something like:
      Screenshot From 2025-09-19 11-27-45
      (that is, a low impact virtual update to synchronise state with reality)
  • Exposes the dbt fqn's in sqlmesh_dbt list to show that theyre working
    Screenshot From 2025-09-19 11-05-05

Also, some housekeeping in the test suite:

  • I noticed @eakmanrq and myself had independently implemented create_empty_project fixtures, so I combined them and moved them to a common location
  • I also moved the jaffle_shop_duckdb fixture out of the sqlmesh_dbt cli conftest and into the main dbt conftest so I could use it in test_integration to verify the dbt names matched the output of dbt list

In a follow-up PR, I have built on this to enable the selector engine to utilise the dbt names if they are available.

This PR also puts some structure in place to be able to implement things like the dbt graph in terms of Context.snapshots, rather than storing the entire graph in every model JinjaMacroRegistry that uses it

metadata.append(f"{k}:{gen(v)}")

if self.dbt_node_info:
metadata.append(self.dbt_node_info.json(sort_keys=True))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to extend the ./.circleci/test_migration.sh test to also run on sushi_dbt project and not just sushi, since we now have dbt-specific logic when hashing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added this and I think it's failing in the way you expected it to:

**Metadata Updated:**
- `sushi.customer_revenue_by_day`
- `sushi.customers`
- `sushi.top_waiters`
- `sushi.waiter_as_customer_by_day`
- `sushi.waiter_revenue_by_day_v1`
- `sushi.waiter_revenue_by_day_v2`
- `sushi.waiters`
- `sushi_raw.items`
- `sushi_raw.order_items`
- `sushi_raw.orders`
- `sushi_raw.waiter_names`

Exited with code exit status 1

This is correct because after the migration in this PR runs and a new plan is made, the extra fields get read from disk and show as a metadata change because they go from being empty in state to being populated in state

Copy link
Collaborator

@izeigerman izeigerman left a comment

Choose a reason for hiding this comment

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

Please consider extending the migration test. LGTM othterwise

@erindru erindru merged commit 34dc9fd into main Sep 23, 2025
35 of 36 checks passed
@erindru erindru deleted the erin/dbt-node-info branch September 23, 2025 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants