From 72c7c465ec87a9a8000fedece7272658deffe22c Mon Sep 17 00:00:00 2001
From: Ilia Borovitinov <ilia@electric-sql.com>
Date: Fri, 22 Nov 2024 21:49:03 +0300
Subject: [PATCH] fix: don't alter table on every shape creation (#2033)

---
 .changeset/mean-keys-draw.md                  |  5 ++++
 .../lib/electric/postgres/configuration.ex    | 13 ++++++++-
 .../electric/postgres/configuration_test.exs  | 27 +++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/mean-keys-draw.md

diff --git a/.changeset/mean-keys-draw.md b/.changeset/mean-keys-draw.md
new file mode 100644
index 0000000000..1bea7340e3
--- /dev/null
+++ b/.changeset/mean-keys-draw.md
@@ -0,0 +1,5 @@
+---
+"@core/sync-service": patch
+---
+
+fix: don't execute `ALTER TABLE` statements if not necessary
diff --git a/packages/sync-service/lib/electric/postgres/configuration.ex b/packages/sync-service/lib/electric/postgres/configuration.ex
index 7c74fb5aa7..04e050e87b 100644
--- a/packages/sync-service/lib/electric/postgres/configuration.ex
+++ b/packages/sync-service/lib/electric/postgres/configuration.ex
@@ -3,6 +3,7 @@ defmodule Electric.Postgres.Configuration do
   Module for functions that configure Postgres in some way using
   a provided connection.
   """
+  require Logger
   alias Electric.Utils
   alias Electric.Shapes.Shape
 
@@ -103,7 +104,17 @@ defmodule Electric.Postgres.Configuration do
   defp set_replica_identity!(conn, relations) do
     for {relation, _} <- relations,
         table = Utils.relation_to_sql(relation) do
-      Postgrex.query!(conn, "ALTER TABLE #{table} REPLICA IDENTITY FULL", [])
+      %Postgrex.Result{rows: [[correct_identity?]]} =
+        Postgrex.query!(
+          conn,
+          "SELECT relreplident = 'f' FROM pg_class JOIN pg_namespace ON relnamespace = pg_namespace.oid WHERE nspname = $1 AND relname = $2;",
+          Tuple.to_list(relation)
+        )
+
+      if not correct_identity? do
+        Logger.info("Altering identity of #{table} to FULL")
+        Postgrex.query!(conn, "ALTER TABLE #{table} REPLICA IDENTITY FULL", [])
+      end
     end
   end
 
diff --git a/packages/sync-service/test/electric/postgres/configuration_test.exs b/packages/sync-service/test/electric/postgres/configuration_test.exs
index dd67f42d79..e902210ab7 100644
--- a/packages/sync-service/test/electric/postgres/configuration_test.exs
+++ b/packages/sync-service/test/electric/postgres/configuration_test.exs
@@ -1,5 +1,6 @@
 defmodule Electric.Postgres.ConfigurationTest do
   use ExUnit.Case, async: true
+  import ExUnit.CaptureLog
 
   alias Electric.Postgres.Configuration
 
@@ -72,6 +73,32 @@ defmodule Electric.Postgres.ConfigurationTest do
                )
     end
 
+    test "doesn't execute `ALTER TABLE` if table identity is already full",
+         %{pool: conn, publication_name: publication, get_pg_version: get_pg_version} do
+      assert get_table_identity(conn, {"public", "items"}) == "d"
+      assert list_tables_in_publication(conn, publication) == []
+
+      assert capture_log(fn ->
+               Configuration.configure_tables_for_replication!(
+                 conn,
+                 [{{"public", "items"}, "(value ILIKE 'yes%')"}],
+                 get_pg_version,
+                 publication
+               )
+             end) =~ "Altering identity"
+
+      assert get_table_identity(conn, {"public", "items"}) == "f"
+
+      refute capture_log(fn ->
+               Configuration.configure_tables_for_replication!(
+                 conn,
+                 [{{"public", "items"}, "(value ILIKE 'no%')"}],
+                 get_pg_version,
+                 publication
+               )
+             end) =~ "Altering identity"
+    end
+
     test "works with multiple tables", %{
       pool: conn,
       publication_name: publication,