Repository: TobikoData/sqlmesh
Pull request #1776 (2023-11-30 01:05:39)

Chore: Simplify text_diff
When diffing models/audits, we extract meta, statements, and queries from their rendered definitions and diff them separately. This PR simplifies diffing by just diffing the rendered definitions.


Patch:
diff --git a/sqlmesh/core/audit/definition.py b/sqlmesh/core/audit/definition.py
index a6267c93..afc203b5 100644
--- a/sqlmesh/core/audit/definition.py
+++ b/sqlmesh/core/audit/definition.py
@@ -3,7 +3,6 @@ from __future__ import annotations
 import pathlib
 import sys
 import typing as t
-from itertools import zip_longest
 from pathlib import Path
 
 from pydantic import Field
@@ -361,21 +360,8 @@ class StandaloneAudit(_Node, AuditMixin):
                 f"Cannot diff audit '{self.name} against a non-audit node '{other.name}'"
             )
 
-        meta_a, *statements_a = self.render_definition()
-        meta_b, *statements_b = other.render_definition()
-
-        query_a = statements_a.pop() if statements_a else None
-        query_b = statements_b.pop() if statements_b else None
-
-        return "\n".join(
-            (
-                d.text_diff(meta_a, meta_b, self.dialect),
-                *(
-                    d.text_diff(sa, sb, self.dialect)
-                    for sa, sb in zip_longest(statements_a, statements_b)
-                ),
-                d.text_diff(query_a, query_b, self.dialect),
-            )
+        return d.text_diff(
+            self.render_definition(), other.render_definition(), self.dialect, other.dialect
         ).strip()
 
     def render_definition(self, include_python: bool = True) -> t.List[exp.Expression]:
diff --git a/sqlmesh/core/dialect.py b/sqlmesh/core/dialect.py
index 30496ba0..a7789821 100644
--- a/sqlmesh/core/dialect.py
+++ b/sqlmesh/core/dialect.py
@@ -519,17 +519,23 @@ def format_model_expressions(
 
 
 def text_diff(
-    a: t.Optional[exp.Expression],
-    b: t.Optional[exp.Expression],
-    dialect: t.Optional[str] = None,
+    a: t.List[exp.Expression],
+    b: t.List[exp.Expression],
+    a_dialect: t.Optional[str] = None,
+    b_dialect: t.Optional[str] = None,
 ) -> str:
     """Find the unified text diff between two expressions."""
-    return "\n".join(
-        unified_diff(
-            a.sql(pretty=True, comments=False, dialect=dialect).split("\n") if a else "",
-            b.sql(pretty=True, comments=False, dialect=dialect).split("\n") if b else "",
-        )
-    )
+    a_sql = [
+        line
+        for expr in a
+        for line in expr.sql(pretty=True, comments=False, dialect=a_dialect).split("\n")
+    ]
+    b_sql = [
+        line
+        for expr in b
+        for line in expr.sql(pretty=True, comments=False, dialect=b_dialect).split("\n")
+    ]
+    return "\n".join(unified_diff(a_sql, b_sql))
 
 
 DIALECT_PATTERN = re.compile(
diff --git a/sqlmesh/core/model/definition.py b/sqlmesh/core/model/definition.py
index 41e45798..dd3ac7d3 100644
--- a/sqlmesh/core/model/definition.py
+++ b/sqlmesh/core/model/definition.py
@@ -7,7 +7,6 @@ import sys
 import types
 import typing as t
 from difflib import unified_diff
-from itertools import zip_longest
 from pathlib import Path
 from textwrap import indent
 
@@ -468,21 +467,8 @@ class _Model(ModelMeta, frozen=True):
                 f"Cannot diff model '{self.name} against a non-model node '{other.name}'"
             )
 
-        meta_a, *statements_a = self.render_definition()
-        meta_b, *statements_b = other.render_definition()
-
-        query_a = statements_a.pop() if statements_a else None
-        query_b = statements_b.pop() if statements_b else None
-
-        return "\n".join(
-            (
-                d.text_diff(meta_a, meta_b, self.dialect),
-                *(
-                    d.text_diff(sa, sb, self.dialect)
-                    for sa, sb in zip_longest(statements_a, statements_b)
-                ),
-                d.text_diff(query_a, query_b, self.dialect),
-            )
+        return d.text_diff(
+            self.render_definition(), other.render_definition(), self.dialect, other.dialect
         ).strip()
 
     def set_time_format(self, default_time_format: str = c.DEFAULT_TIME_COLUMN_FORMAT) -> None:
@@ -1229,19 +1215,9 @@ class SeedModel(_SqlBasedModel):
         other._ensure_hydrated()
         self._ensure_hydrated()
 
-        meta_a = self.render_definition()[0]
-        meta_b = other.render_definition()[0]
         return "\n".join(
             (
-                d.text_diff(meta_a, meta_b, self.dialect),
-                *(
-                    d.text_diff(pa, pb, self.dialect)
-                    for pa, pb in zip_longest(self.pre_statements, other.pre_statements)
-                ),
-                *(
-                    d.text_diff(pa, pb, self.dialect)
-                    for pa, pb in zip_longest(self.pre_statements, other.post_statements)
-                ),
+                super().text_diff(other),
                 *unified_diff(
                     self.seed.content.split("\n"),
                     other.seed.content.split("\n"),


Test patch:
diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py
index 43a75fd5..d8a76a88 100644
--- a/tests/core/test_dialect.py
+++ b/tests/core/test_dialect.py
@@ -146,7 +146,7 @@ def test_text_diff():
 -FROM x
 +  1
 +FROM y""" in text_diff(
-        parse_one("SELECT * FROM x"), parse_one("SELECT 1 FROM y")
+        parse("SELECT * FROM x"), parse("SELECT 1 FROM y")
     )
 
 
diff --git a/tests/integrations/github/cicd/test_integration.py b/tests/integrations/github/cicd/test_integration.py
index c606bf65..c9775a08 100644
--- a/tests/integrations/github/cicd/test_integration.py
+++ b/tests/integrations/github/cicd/test_integration.py
@@ -153,7 +153,7 @@ def test_merge_pr_has_non_breaking_change(
 
 +++ 
 
-@@ -1,7 +1,8 @@
+@@ -11,7 +11,8 @@
 
  SELECT
    CAST(o.waiter_id AS INT) AS waiter_id,
@@ -352,7 +352,7 @@ def test_merge_pr_has_non_breaking_change_diff_start(
 
 +++ 
 
-@@ -1,7 +1,8 @@
+@@ -11,7 +11,8 @@
 
  SELECT
    CAST(o.waiter_id AS INT) AS waiter_id,
@@ -866,7 +866,7 @@ def test_no_merge_since_no_deploy_signal(
 
 +++ 
 
-@@ -1,7 +1,8 @@
+@@ -11,7 +11,8 @@
 
  SELECT
    CAST(o.waiter_id AS INT) AS waiter_id,
@@ -1049,7 +1049,7 @@ def test_no_merge_since_no_deploy_signal_no_approvers_defined(
 
 +++ 
 
-@@ -1,7 +1,8 @@
+@@ -11,7 +11,8 @@
 
  SELECT
    CAST(o.waiter_id AS INT) AS waiter_id,
@@ -1225,7 +1225,7 @@ def test_deploy_comment_pre_categorized(
 
 +++ 
 
-@@ -1,7 +1,8 @@
+@@ -11,7 +11,8 @@
 
  SELECT
    CAST(o.waiter_id AS INT) AS waiter_id,