Fix: external models hash and is_breaking (#897)

* Fix: external models hash and is_breaking

* move order

Author: tobymao

sqlmesh/core/config/categorizer.py

@@ -27,6 +27,7 @@ class CategorizerConfig(BaseConfig):
         seed: the auto categorization mode for Seed models.
     """
 
+    external: AutoCategorizationMode = AutoCategorizationMode.FULL
     python: AutoCategorizationMode = AutoCategorizationMode.OFF
     sql: AutoCategorizationMode = AutoCategorizationMode.FULL
     seed: AutoCategorizationMode = AutoCategorizationMode.FULL

sqlmesh/core/model/definition.py

@@ -487,7 +487,7 @@ def is_breaking_change(self, previous: Model) -> t.Optional[bool]:
             True if this model instance represents a breaking change, False if it's a non-breaking change
             and None if the nature of the change can't be determined.
         """
-        return None
+        raise NotImplementedError
 
     def _run_hooks(
         self,
@@ -885,6 +885,9 @@ def render_definition(self, include_python: bool = True) -> t.List[exp.Expressio
     def is_python(self) -> bool:
         return True
 
+    def is_breaking_change(self, previous: Model) -> t.Optional[bool]:
+        return None
+
     def __repr__(self) -> str:
         return f"Model<name: {self.name}, entrypoint: {self.entrypoint}>"
 
@@ -894,6 +897,13 @@ class ExternalModel(_Model):
 
     source_type: Literal["external"] = "external"
 
+    def is_breaking_change(self, previous: Model) -> t.Optional[bool]:
+        if not isinstance(previous, ExternalModel):
+            return None
+        if not previous.columns_to_types.items() - self.columns_to_types.items():
+            return False
+        return None
+
 
 Model = Annotated[
     t.Union[SqlModel, SeedModel, PythonModel, ExternalModel], Field(discriminator="source_type")

sqlmesh/core/snapshot/definition.py

@@ -517,7 +517,7 @@ def __eq__(self, other: t.Any) -> bool:
         return isinstance(other, Snapshot) and self.fingerprint == other.fingerprint
 
     def __hash__(self) -> int:
-        return hash((self.__class__, self.fingerprint))
+        return hash((self.__class__, self.name, self.fingerprint))
 
     def add_interval(self, start: TimeLike, end: TimeLike, is_dev: bool = False) -> None:
         """Add a newly processed time interval to the snapshot.
@@ -915,16 +915,14 @@ def serialize_hooks(hooks: t.List[HookCall]) -> t.Iterable[str]:
                 data.append(macro.definition)
     elif isinstance(model, PythonModel):
         data.append(model.entrypoint)
-        for column_name, column_type in model.columns_to_types.items():
-            data.append(column_name)
-            data.append(str(column_type))
     elif isinstance(model, SeedModel):
         for column_name, column_hash in model.column_hashes.items():
             data.append(column_name)
             data.append(column_hash)
-        for column_name, column_type in (model.columns_to_types_ or {}).items():
-            data.append(column_name)
-            data.append(column_type.sql())
+
+    for column_name, column_type in (model.columns_to_types_ or {}).items():
+        data.append(column_name)
+        data.append(column_type.sql())
 
     if isinstance(model.kind, kind.IncrementalByTimeRangeKind):
         data.append(model.kind.time_column.column)

tests/core/test_model.py

@@ -16,6 +16,7 @@
     SeedKind,
     SqlModel,
     TimeColumn,
+    create_external_model,
     create_seed_model,
     load_model,
     model,
@@ -1088,3 +1089,19 @@ def test_model_ctas_query():
     )
 
     assert load_model(expressions).ctas_query({}).sql() == "SELECT 1 AS a FROM b AS b WHERE FALSE"
+
+
+def test_is_breaking_change():
+    model = create_external_model("a", columns={"a": "int", "b": "int"})
+    assert model.is_breaking_change(create_external_model("a", columns={"a": "int"})) is False
+    assert model.is_breaking_change(create_external_model("a", columns={"a": "text"})) is None
+    assert (
+        model.is_breaking_change(create_external_model("a", columns={"a": "int", "b": "int"}))
+        is False
+    )
+    assert (
+        model.is_breaking_change(
+            create_external_model("a", columns={"a": "int", "b": "int", "c": "int"})
+        )
+        is None
+    )