Skip to content

Commit feafd50

Browse files
Fix: allow --select-model to plan a model deletion
Refactor select_models to return matched FQNs alongside the model dict so context._plan can detect deletions without a separate env lookup. Remove expand_model_selections_with_env, add mixed selection test coverage. Fixes #5741 Signed-off-by: vinicius <viniciusnunes.tavares@gmail.com>
1 parent ef31bd1 commit feafd50

File tree

3 files changed

+47
-82
lines changed

3 files changed

+47
-82
lines changed

sqlmesh/core/context.py

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,13 +1605,11 @@ def plan_builder(
16051605
backfill_models = None
16061606

16071607
models_override: t.Optional[UniqueKeyDict[str, Model]] = None
1608-
# FQNs of models that are selected for deletion (present in the deployed environment but
1609-
# absent from local files). These are models whose files have been deleted; they will
1610-
# appear in context_diff.removed_snapshots rather than as backfill candidates.
1608+
selected_fqns: t.Set[str] = set()
16111609
selected_deletion_fqns: t.Set[str] = set()
16121610
if select_models:
16131611
try:
1614-
models_override = model_selector.select_models(
1612+
models_override, selected_fqns = model_selector.select_models(
16151613
select_models,
16161614
environment,
16171615
fallback_env_name=create_from or c.PROD,
@@ -1627,16 +1625,9 @@ def plan_builder(
16271625
backfill_models = model_selector.expand_model_selections(select_models)
16281626

16291627
if not backfill_models:
1630-
# The selection matched nothing locally. Check whether it matched models that exist
1631-
# in the deployed environment but have been deleted locally. If so, the selection is
1632-
# valid — the deletions will surface in context_diff.removed_snapshots.
1633-
env_selected = model_selector.expand_model_selections_with_env(
1634-
select_models,
1635-
environment,
1636-
fallback_env_name=create_from or c.PROD,
1637-
ensure_finalized_snapshots=self.config.plan.use_finalized_state,
1638-
)
1639-
selected_deletion_fqns = env_selected - set(self._models)
1628+
# The selection matched nothing locally. Check whether it matched models
1629+
# in the deployed environment that were deleted locally.
1630+
selected_deletion_fqns = selected_fqns - set(self._models)
16401631

16411632
expanded_restate_models = None
16421633
if restate_models is not None:

sqlmesh/core/selector.py

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def select_models(
6262
target_env_name: str,
6363
fallback_env_name: t.Optional[str] = None,
6464
ensure_finalized_snapshots: bool = False,
65-
) -> UniqueKeyDict[str, Model]:
65+
) -> t.Tuple[UniqueKeyDict[str, Model], t.Set[str]]:
6666
"""Given a set of selections returns models from the current state with names matching the
6767
selection while sourcing the remaining models from the target environment.
6868
@@ -76,7 +76,7 @@ def select_models(
7676
the environment is not finalized.
7777
7878
Returns:
79-
A dictionary of models.
79+
A tuple of (models dict, set of all matched FQNs including env models).
8080
"""
8181
env_models = self._load_env_models(
8282
target_env_name, fallback_env_name, ensure_finalized_snapshots
@@ -148,36 +148,7 @@ def get_model(fqn: str) -> t.Optional[Model]:
148148
if needs_update:
149149
update_model_schemas(dag, models=models, cache_dir=self._cache_dir)
150150

151-
return models
152-
153-
def expand_model_selections_with_env(
154-
self,
155-
model_selections: t.Iterable[str],
156-
target_env_name: str,
157-
fallback_env_name: t.Optional[str] = None,
158-
ensure_finalized_snapshots: bool = False,
159-
) -> t.Set[str]:
160-
"""Expands model selections against both local models and the target environment.
161-
162-
This allows selections to match models that have been deleted locally but still
163-
exist in the deployed environment.
164-
165-
Args:
166-
model_selections: A set of selections.
167-
target_env_name: The name of the target environment.
168-
fallback_env_name: The name of the fallback environment that will be used if the target
169-
environment doesn't exist.
170-
ensure_finalized_snapshots: Whether to source environment snapshots from the latest finalized
171-
environment state, or to use whatever snapshots are in the current environment state even if
172-
the environment is not finalized.
173-
174-
Returns:
175-
A set of matched model FQNs.
176-
"""
177-
env_models = self._load_env_models(
178-
target_env_name, fallback_env_name, ensure_finalized_snapshots
179-
)
180-
return self.expand_model_selections(model_selections, models={**env_models, **self._models})
151+
return models, all_selected_models
181152

182153
def _load_env_models(
183154
self,

tests/core/test_selector_native.py

Lines changed: 39 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ def test_select_change_schema(mocker: MockerFixture, make_snapshot):
309309

310310
selector = NativeSelector(state_reader_mock, local_models)
311311

312-
selected = selector.select_models(["db.parent"], env_name)
312+
selected, _ = selector.select_models(["db.parent"], env_name)
313313
assert selected[local_child.fqn].render_query() != child.render_query()
314314

315315
_assert_models_equal(
@@ -320,7 +320,7 @@ def test_select_change_schema(mocker: MockerFixture, make_snapshot):
320320
},
321321
)
322322

323-
selected = selector.select_models(["db.child"], env_name)
323+
selected, _ = selector.select_models(["db.child"], env_name)
324324
assert selected[local_child.fqn].data_hash == child.data_hash
325325

326326
_assert_models_equal(
@@ -343,12 +343,12 @@ def test_select_models_missing_env(mocker: MockerFixture, make_snapshot):
343343

344344
selector = NativeSelector(state_reader_mock, local_models)
345345

346-
assert selector.select_models([model.name], "missing_env").keys() == {model.fqn}
347-
assert not selector.select_models(["missing"], "missing_env")
346+
assert selector.select_models([model.name], "missing_env")[0].keys() == {model.fqn}
347+
assert not selector.select_models(["missing"], "missing_env")[0]
348348

349349
assert selector.select_models(
350350
[model.name], "missing_env", fallback_env_name="another_missing_env"
351-
).keys() == {model.fqn}
351+
)[0].keys() == {model.fqn}
352352

353353
state_reader_mock.get_environment.assert_has_calls(
354354
[
@@ -789,7 +789,7 @@ def test_select_models_local_tags_take_precedence_over_remote(
789789

790790
selector = NativeSelector(state_reader_mock, local_models)
791791

792-
selected = selector.select_models(["tag:a"], env_name)
792+
selected, _ = selector.select_models(["tag:a"], env_name)
793793

794794
# both should get selected because they both now have the 'a' tag locally, even though one exists in remote state without the 'a' tag
795795
_assert_models_equal(
@@ -801,9 +801,9 @@ def test_select_models_local_tags_take_precedence_over_remote(
801801
)
802802

803803

804-
def test_expand_model_selections_with_env(mocker: MockerFixture, make_snapshot):
805-
"""expand_model_selections_with_env should include models from the deployed environment,
806-
even if they have been deleted locally."""
804+
def test_select_models_returns_selected_fqns(mocker: MockerFixture, make_snapshot):
805+
"""select_models should return the set of all matched FQNs (including env-only models)
806+
alongside the model dict."""
807807
local_model = SqlModel(
808808
name="db.local_model",
809809
query=d.parse_one("SELECT 1 AS a"),
@@ -835,34 +835,31 @@ def test_expand_model_selections_with_env(mocker: MockerFixture, make_snapshot):
835835

836836
selector = NativeSelector(state_reader_mock, local_models)
837837

838-
# Expanding against local models only should NOT find the deleted model.
839-
assert selector.expand_model_selections(["db.deleted_model"]) == set()
838+
# Selecting a deleted model: selected_fqns includes it even though models dict won't.
839+
_, selected_fqns = selector.select_models(["db.deleted_model"], env_name)
840+
assert deleted_model.fqn in selected_fqns
840841

841-
# Expanding with env should find the deleted model.
842-
result = selector.expand_model_selections_with_env(["db.deleted_model"], env_name)
843-
assert deleted_model.fqn in result
842+
# Selecting a local model: selected_fqns includes it.
843+
_, selected_fqns = selector.select_models(["db.local_model"], env_name)
844+
assert local_model.fqn in selected_fqns
844845

845-
# Local model should also be reachable.
846-
result = selector.expand_model_selections_with_env(["db.local_model"], env_name)
847-
assert local_model.fqn in result
848-
849-
# Selecting both should return both.
850-
result = selector.expand_model_selections_with_env(
846+
# Mixed selection (active + deleted): both appear in selected_fqns.
847+
_, selected_fqns = selector.select_models(
851848
["db.deleted_model", "db.local_model"], env_name
852849
)
853-
assert result == {deleted_model.fqn, local_model.fqn}
850+
assert selected_fqns == {deleted_model.fqn, local_model.fqn}
854851

855-
# Wildcard should match env models too.
856-
result = selector.expand_model_selections_with_env(["*_model"], env_name)
857-
assert result == {deleted_model.fqn, local_model.fqn}
852+
# Wildcard should match both local and env models.
853+
_, selected_fqns = selector.select_models(["*_model"], env_name)
854+
assert selected_fqns == {deleted_model.fqn, local_model.fqn}
858855

859-
# Non-existent model should return empty.
860-
result = selector.expand_model_selections_with_env(["db.nonexistent"], env_name)
861-
assert result == set()
856+
# Non-existent model should not appear.
857+
_, selected_fqns = selector.select_models(["db.nonexistent"], env_name)
858+
assert selected_fqns == set()
862859

863860

864-
def test_expand_model_selections_with_env_fallback(mocker: MockerFixture, make_snapshot):
865-
"""expand_model_selections_with_env should fall back to the fallback environment."""
861+
def test_select_models_selected_fqns_fallback(mocker: MockerFixture, make_snapshot):
862+
"""select_models selected_fqns should include env models found via fallback environment."""
866863
deleted_model = SqlModel(
867864
name="db.deleted_model",
868865
query=d.parse_one("SELECT 1 AS a"),
@@ -890,14 +887,14 @@ def test_expand_model_selections_with_env_fallback(mocker: MockerFixture, make_s
890887
local_models: UniqueKeyDict[str, Model] = UniqueKeyDict("models")
891888
selector = NativeSelector(state_reader_mock, local_models)
892889

893-
result = selector.expand_model_selections_with_env(
890+
_, selected_fqns = selector.select_models(
894891
["db.deleted_model"], "missing_env", fallback_env_name="prod"
895892
)
896-
assert deleted_model.fqn in result
893+
assert deleted_model.fqn in selected_fqns
897894

898895

899-
def test_expand_model_selections_with_env_expired(mocker: MockerFixture, make_snapshot):
900-
"""expand_model_selections_with_env should ignore expired environments."""
896+
def test_select_models_selected_fqns_expired(mocker: MockerFixture, make_snapshot):
897+
"""select_models should not match env models from expired environments."""
901898
deleted_model = SqlModel(
902899
name="db.deleted_model",
903900
query=d.parse_one("SELECT 1 AS a"),
@@ -924,11 +921,17 @@ def test_expand_model_selections_with_env_expired(mocker: MockerFixture, make_sn
924921
local_models: UniqueKeyDict[str, Model] = UniqueKeyDict("models")
925922
selector = NativeSelector(state_reader_mock, local_models)
926923

927-
result = selector.expand_model_selections_with_env(["db.deleted_model"], "test_env")
928-
assert result == set()
924+
_, selected_fqns = selector.select_models(["db.deleted_model"], "test_env")
925+
assert selected_fqns == set()
929926

930927

931-
def _assert_models_equal(actual: t.Dict[str, Model], expected: t.Dict[str, Model]) -> None:
928+
def _assert_models_equal(
929+
actual: t.Union[t.Dict[str, Model], t.Tuple[t.Dict[str, Model], t.Set[str]]],
930+
expected: t.Dict[str, Model],
931+
) -> None:
932+
# select_models returns a tuple; unwrap if needed.
933+
if isinstance(actual, tuple):
934+
actual = actual[0]
932935
assert set(actual) == set(expected)
933936
for name, model in actual.items():
934937
# Use dict() to make Pydantic V2 happy.

0 commit comments

Comments
 (0)