Skip to content

Commit 8c6284d

Browse files
committed
refactor: accept only table name strings in _resolve_table
Every caller of BaseExpressionRenderer._resolve_table passes a normalized model name string: this_model resolution passes the model's FQN, and the resolve_table macro normalizes its argument with normalize_model_name first. The exp.Expr branch was reachable only from a test that called the private method directly, so narrow the signature to str and delete that test. Signed-off-by: mday-io <mdaytn@gmail.com>
1 parent 21adbfb commit 8c6284d

2 files changed

Lines changed: 2 additions & 34 deletions

File tree

‎sqlmesh/core/renderer.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ def update_cache(self, expression: t.Optional[exp.Expr]) -> None:
384384

385385
def _resolve_table(
386386
self,
387-
table_name: str | exp.Expr,
387+
table_name: str,
388388
snapshots: t.Optional[t.Dict[str, Snapshot]] = None,
389389
table_mapping: t.Optional[t.Dict[str, str]] = None,
390390
deployability_index: t.Optional[DeployabilityIndex] = None,
@@ -405,7 +405,7 @@ def _resolve_table(
405405

406406
if not mapping and snapshots:
407407
# An exact FQN match avoids scanning unrelated snapshots.
408-
snapshot = snapshots.get(table_name) if isinstance(table_name, str) else None
408+
snapshot = snapshots.get(table_name)
409409
# Keys normalized under different dialects may differ in casing or quoting.
410410
# Fall back to the full mapping so exp.replace_tables can reconcile them.
411411
mapping = self._to_table_mapping(

‎tests/core/test_model.py‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9900,38 +9900,6 @@ def resolve_named(evaluator, name):
99009900
)
99019901

99029902

9903-
def test_resolve_table_non_string_expr_path(make_snapshot: t.Callable):
9904-
"""When `table_name` is an `exp.Expr` (not a `str`), `_resolve_table` falls back to building
9905-
the full snapshot mapping (the `else` branch of the new code). This exercises that branch --
9906-
which the `this_model`/`resolve_table` macro call sites never hit, since they always pass a
9907-
pre-normalized string -- directly at the renderer level, to make sure it's still reachable
9908-
and correct, and not dead code that silently bit-rots."""
9909-
9910-
from sqlmesh.core.renderer import ExpressionRenderer
9911-
9912-
parent = load_sql_based_model(d.parse("MODEL (name parent); SELECT 1 AS c"))
9913-
parent_snapshot = make_snapshot(parent)
9914-
parent_snapshot.categorize_as(SnapshotChangeCategory.BREAKING)
9915-
9916-
other = load_sql_based_model(d.parse("MODEL (name other); SELECT 1 AS c"))
9917-
other_snapshot = make_snapshot(other)
9918-
other_snapshot.categorize_as(SnapshotChangeCategory.BREAKING)
9919-
9920-
expr_renderer = ExpressionRenderer(
9921-
exp.select("*"),
9922-
dialect="",
9923-
macro_definitions=[],
9924-
path=Path("."),
9925-
)
9926-
9927-
table_expr = exp.to_table('"parent"')
9928-
resolved = expr_renderer._resolve_table(
9929-
table_expr,
9930-
snapshots={'"parent"': parent_snapshot, '"other"': other_snapshot},
9931-
)
9932-
assert resolved.sql(comments=False) == f'"sqlmesh__default"."parent__{parent_snapshot.version}"'
9933-
9934-
99359903
def test_resolve_tables_expand_reveals_table_after_find_check(make_snapshot: t.Callable):
99369904
"""Embedded-model expansion (`expand=`) runs as an `expression.transform` *before* the new
99379905
`expression.find(exp.Table)` short-circuit in `_resolve_tables`, so a table reference that

0 commit comments

Comments
 (0)