Skip to content

Feat(sqlmesh_dbt): Select based on dbt name, not sqlmesh name#5420

Merged
erindru merged 2 commits intomainfrom
erin/dbt-selectors
Sep 24, 2025
Merged

Feat(sqlmesh_dbt): Select based on dbt name, not sqlmesh name#5420
erindru merged 2 commits intomainfrom
erin/dbt-selectors

Conversation

@erindru
Copy link
Collaborator

@erindru erindru commented Sep 21, 2025

This builds on the dbt node info exposed in #5412 to enable its usage in the selector engine.

Previously, to use sqlmesh_dbt --select, you had to use the sqlmesh-native model names.

This PR:

  • Adds dbt_mode flag to the Context to pass to the Selector engine
  • Sets this flag in the sqlmesh_dbt CLI when constructing the Context
  • Uses it to indicate that dbt fqn's should be matched against instead of the sqlmesh model name

The selectors still resolve to SQLMesh native fqn's so everything continues to match up downstream, they just match based on dbt fqn instead of sqlmesh model name

load: bool = True,
users: t.Optional[t.List[User]] = None,
config_loader_kwargs: t.Optional[t.Dict[str, t.Any]] = None,
dbt_mode: bool = False,
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 called this generically dbt_mode instead of something more specific to selectors incase we discovered other behaviour that we wanted to attach to it

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we need the extra flag instead of using the project_type, for when self._project_type == c.DBT

Copy link
Contributor

Choose a reason for hiding this comment

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

actually never mind I realise this is for when someone runs sqlmesh_dbt rather than a dbt project with the native sqlmesh commands so this is indeed needed

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like this flag at all. Can we instead have a base Selector class and 2 implementations and then we can configure the implementation used when creating a Context instance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, i've refactored Selector to be an abstract base class and then implemented it as NativeSelector and DbtSelector

@erindru erindru force-pushed the erin/dbt-node-info branch 2 times, most recently from 0007661 to e551d67 Compare September 23, 2025 02:59
Base automatically changed from erin/dbt-node-info to main September 23, 2025 03:23
fqn
for fqn, model in all_models.items()
if fnmatch.fnmatchcase(model.name, node.this)
if fnmatch.fnmatchcase(self._model_name(model), node.this)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the wildcard matching happens outside _pattern_to_model_fqns which seems straightforward and should work, but could you add a test for this as well to ensure nothing breaks in the future if someone modifies this? unless if I missed it and there is a wildcard test for dbt model names already

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well spotted, I added some tests to exercise this codepath based on the dbt docs.

Note that the patterns currently need a * in them to trigger this branch (even though ?, [ and ] could be used as well) so this will need more work in future if we find people using these in the wild

Copy link
Contributor

@themisvaltinos themisvaltinos left a comment

Choose a reason for hiding this comment

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

lrgtm! I was wondering of ways for the _pattern_to_model_fqns logic to be reduced to a regex, but it will look complicated and I do like your approach a lot better, it is more readable and makes it clear what is happening

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.

Thanks!

@erindru erindru merged commit 368c5dd into main Sep 24, 2025
35 of 38 checks passed
@erindru erindru deleted the erin/dbt-selectors branch September 24, 2025 21:07
@github-christophe-oudar

Just noticed it implemented #3583 which was quite painful! Thanks 🙌

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.

4 participants