Repository: TobikoData/sqlmesh
Pull request #2501 (2024-04-24 02:03:41)

Fix: Make sure that we don't attempt to create two seed records with the same version in state
This may happen when a metadata change is applied to a seed model, in which case the version will remain the same.

Without this fix we may violate the primary key constraint for databases that support it, or otherwise create duplicated records.


Patch:
diff --git a/.circleci/continue_config.yml b/.circleci/continue_config.yml
index a8a416af..137dd3ff 100644
--- a/.circleci/continue_config.yml
+++ b/.circleci/continue_config.yml
@@ -231,7 +231,8 @@ jobs:
         - run:
             name: Trigger private tests
             command: |
-              echo "export COMMIT_MESSAGE=\"$(git log --format=%s -n 1 $CIRCLE_SHA1)\"" >> "$BASH_ENV"
+              echo 'export COMMIT_MESSAGE="$(git log --format=%s -n 1 $CIRCLE_SHA1)"' >> "$BASH_ENV"
+              echo 'export FORMATTED_COMMIT_MESSAGE="${COMMIT_MESSAGE//\"/\\\"}"' >> "$BASH_ENV"
               source "$BASH_ENV"
               curl --request POST \
                 --url $TOBIKO_PRIVATE_CIRCLECI_URL \
@@ -245,7 +246,7 @@ jobs:
                     "sqlmesh_branch":"'$CIRCLE_BRANCH'",
                     "sqlmesh_commit_author":"'$CIRCLE_USERNAME'",
                     "sqlmesh_commit_hash":"'$CIRCLE_SHA1'",
-                    "sqlmesh_commit_message":"'"$COMMIT_MESSAGE"'"
+                    "sqlmesh_commit_message":"'"$FORMATTED_COMMIT_MESSAGE"'"
                     }
                 }'
 
diff --git a/sqlmesh/core/state_sync/engine_adapter.py b/sqlmesh/core/state_sync/engine_adapter.py
index b2960aed..82bc6a9f 100644
--- a/sqlmesh/core/state_sync/engine_adapter.py
+++ b/sqlmesh/core/state_sync/engine_adapter.py
@@ -39,6 +39,7 @@ from sqlmesh.core.snapshot import (
     Intervals,
     Node,
     Snapshot,
+    SnapshotChangeCategory,
     SnapshotFingerprint,
     SnapshotId,
     SnapshotIdLike,
@@ -215,7 +216,10 @@ class EngineAdapterStateSync(CommonStateSyncMixin, StateSync):
         snapshots_to_store = []
 
         for snapshot in snapshots:
-            if isinstance(snapshot.node, SeedModel):
+            if isinstance(snapshot.node, SeedModel) and snapshot.change_category in (
+                SnapshotChangeCategory.BREAKING,
+                SnapshotChangeCategory.NON_BREAKING,
+            ):
                 seed_model = t.cast(SeedModel, snapshot.node)
                 seed_contents.append(
                     {


Test patch:
diff --git a/tests/core/test_state_sync.py b/tests/core/test_state_sync.py
index c0d9c032..f4fef1fe 100644
--- a/tests/core/test_state_sync.py
+++ b/tests/core/test_state_sync.py
@@ -2225,3 +2225,40 @@ def test_snapshot_batching(state_sync, mocker, make_snapshot):
     assert len(snapshots) == 3
     calls = mock.fetchall.call_args_list
     assert len(calls) == 2
+
+
+def test_seed_model_metadata_update(
+    state_sync: EngineAdapterStateSync,
+    make_snapshot: t.Callable,
+):
+    model = SeedModel(
+        name="a",
+        kind=SeedKind(path="./path/to/seed"),
+        seed=Seed(content="header\n1\n2"),
+        column_hashes={"header": "hash"},
+        depends_on=set(),
+    )
+    snapshot = make_snapshot(model)
+    snapshot.categorize_as(SnapshotChangeCategory.BREAKING)
+
+    state_sync.push_snapshots([snapshot])
+
+    seed_query = (
+        exp.select("COUNT(*)")
+        .from_("sqlmesh._seeds")
+        .where(exp.column("version").eq(snapshot.version))
+    )
+
+    assert state_sync.engine_adapter.fetchone(seed_query)[0] == 1
+
+    model = model.copy(update={"owner": "jen"})
+    new_snapshot = make_snapshot(model)
+    new_snapshot.previous_versions = snapshot.all_versions
+    new_snapshot.categorize_as(SnapshotChangeCategory.FORWARD_ONLY)
+
+    assert snapshot.fingerprint != new_snapshot.fingerprint
+    assert snapshot.version == new_snapshot.version
+
+    state_sync.push_snapshots([new_snapshot])
+    assert state_sync.engine_adapter.fetchone(seed_query)[0] == 1
+    assert len(state_sync.get_snapshots([new_snapshot, snapshot])) == 2