feat: Automatically switch to DROP / CREATE trigger if PG is <= v13

Author: cheerfulstoic

config/config.exs

@@ -0,0 +1,7 @@
+import Config
+
+config :ecto_watch, EctoWatch.DB, mod: EctoWatch.DB.Live
+
+if Mix.env() == :test do
+  config :ecto_watch, EctoWatch.DB, mod: EctoWatch.DB.Mock
+end

guides/introduction/Getting Started.md

@@ -10,7 +10,6 @@ To use EctoWatch, you need to add it to your supervision tree and specify watche
    repo: MyApp.Repo,
    pub_sub: MyApp.PubSub,
    # Set to true if using PostgreSQL versions older than 13.3.4
-   legacy_postgres_support?: false,
    watchers: [
      {User, :inserted},
      {User, :updated},
@@ -80,4 +79,4 @@ Once subscribed, messages can be handled like so (LiveView example are given her
 
 ### PostgreSQL Version Compatibility
 
-EctoWatch uses `CREATE OR REPLACE TRIGGER` which was introduced in PostgreSQL 13.3.4. If you're using an older version of PostgreSQL, set the `legacy_postgres_support?: true` option. This will use `DROP TRIGGER IF EXISTS` followed by `CREATE TRIGGER` instead.
+EctoWatch uses `CREATE OR REPLACE TRIGGER` which was introduced in PostgreSQL 13. If you're using an older version of PostgreSQL, `ecto_watch` will automatically use `DROP TRIGGER IF EXISTS` followed by `CREATE TRIGGER` instead.  This can have [potential problems with permissions](https://github.com/cheerfulstoic/ecto_watch/issues/50), so if you experience those it's recommended to upgrade to at least PostgreSQL 14.

lib/ecto_watch/db.ex

@@ -0,0 +1,47 @@
+defmodule EctoWatch.DB do
+  @callback max_identifier_length(repo_mod :: atom()) :: integer()
+  @callback major_version(repo_mod :: atom()) :: integer()
+
+  def supports_create_or_replace_trigger?(repo_mod) do
+    major_version(repo_mod) >= 14
+  end
+
+  # TODO
+  def max_identifier_length(repo_mod), do: mod().max_identifier_length(repo_mod)
+  def major_version(repo_mod), do: mod().major_version(repo_mod)
+
+  defp mod do
+    Application.get_env(:ecto_watch, EctoWatch.DB)[:mod]
+  end
+end
+
+defmodule EctoWatch.DB.Live do
+  @behaviour EctoWatch.DB
+
+  def max_identifier_length(repo_mod) do
+    query_for_value(repo_mod, "SHOW max_identifier_length")
+    |> String.to_integer()
+  end
+
+  def major_version(repo_mod) do
+    version_string = query_for_value(repo_mod, "SHOW server_version")
+
+    case Integer.parse(version_string) do
+      {major_version, _} ->
+        major_version
+
+      _ ->
+        raise "Unable to parse PostgreSQL major version number from version string: #{inspect(version_string)}"
+    end
+  end
+
+  defp query_for_value(repo_mod, query) do
+    case Ecto.Adapters.SQL.query!(repo_mod, query, []) do
+      %Postgrex.Result{rows: [[value]]} ->
+        value
+
+      other ->
+        raise "Unexpected result when making query `#{query}`: #{inspect(other)}"
+    end
+  end
+end

lib/ecto_watch/options.ex

@@ -3,17 +3,16 @@ defmodule EctoWatch.Options do
 
   alias EctoWatch.Options.WatcherOptions
 
-  defstruct [:repo_mod, :pub_sub_mod, :watchers, :debug?, :legacy_postgres_support?]
+  defstruct [:repo_mod, :pub_sub_mod, :watchers, :debug?]
 
   def new(opts) do
     %__MODULE__{
       repo_mod: opts[:repo],
       pub_sub_mod: opts[:pub_sub],
       debug?: opts[:debug?],
-      legacy_postgres_support?: opts[:legacy_postgres_support?],
       watchers:
         Enum.map(opts[:watchers], fn watcher_opts ->
-          WatcherOptions.new(watcher_opts, opts[:debug?], opts[:legacy_postgres_support?])
+          WatcherOptions.new(watcher_opts, opts[:debug?])
         end)
     }
   end
@@ -36,13 +35,6 @@ defmodule EctoWatch.Options do
         type: :boolean,
         required: false,
         default: false
-      ],
-      legacy_postgres_support?: [
-        type: :boolean,
-        required: false,
-        default: false,
-        doc:
-          "Set to true to use DROP/CREATE instead of CREATE OR REPLACE for trigger creation (only needed for PostgreSQL versions older than 13.3.4)"
       ]
     ]
 

lib/ecto_watch/options/watcher_options.ex

@@ -9,8 +9,7 @@ defmodule EctoWatch.Options.WatcherOptions do
     :label,
     :trigger_columns,
     :extra_columns,
-    :debug?,
-    :legacy_postgres_support?
+    :debug?
   ]
 
   def validate_list(list) do
@@ -179,13 +178,6 @@ defmodule EctoWatch.Options.WatcherOptions do
         type: :boolean,
         required: false,
         default: false
-      ],
-      legacy_postgres_support?: [
-        type: :boolean,
-        required: false,
-        default: false,
-        doc:
-          "Set to true to use DROP/CREATE instead of CREATE OR REPLACE for trigger creation (only needed for PostgreSQL versions older than 13.3.4)"
       ]
     ]
 
@@ -231,7 +223,7 @@ defmodule EctoWatch.Options.WatcherOptions do
     new({schema_definition, update_type, []}, debug?)
   end
 
-  def new({schema_definition, update_type, opts}, debug?, legacy_postgres_support? \\ false) do
+  def new({schema_definition, update_type, opts}, debug?) do
     schema_definition = SchemaDefinition.new(schema_definition)
 
     %__MODULE__{
@@ -240,8 +232,7 @@ defmodule EctoWatch.Options.WatcherOptions do
       label: opts[:label],
       trigger_columns: opts[:trigger_columns] || [],
       extra_columns: opts[:extra_columns] || [],
-      debug?: debug? || opts[:debug?],
-      legacy_postgres_support?: opts[:legacy_postgres_support?] || legacy_postgres_support?
+      debug?: debug? || opts[:debug?]
     }
   end
 end

lib/ecto_watch/watcher_server.ex

@@ -5,6 +5,7 @@ defmodule EctoWatch.WatcherServer do
   Used internally, but you'll see it in your application supervision tree.
   """
 
+  alias EctoWatch.DB
   alias EctoWatch.Helpers
   alias EctoWatch.Options.WatcherOptions
 
@@ -101,29 +102,29 @@ defmodule EctoWatch.WatcherServer do
       []
     )
 
-    if options.legacy_postgres_support? do
+    if DB.supports_create_or_replace_trigger?(repo_mod) do
       Ecto.Adapters.SQL.query!(
         repo_mod,
         """
-        DROP TRIGGER IF EXISTS #{details.trigger_name} on \"#{options.schema_definition.schema_prefix}\".\"#{options.schema_definition.table_name}\";
+        CREATE OR REPLACE TRIGGER #{details.trigger_name}
+          AFTER #{update_keyword} ON \"#{options.schema_definition.schema_prefix}\".\"#{options.schema_definition.table_name}\" FOR EACH ROW
+          EXECUTE PROCEDURE \"#{options.schema_definition.schema_prefix}\".#{details.function_name}();
         """,
         []
       )
-
+    else
       Ecto.Adapters.SQL.query!(
         repo_mod,
         """
-        CREATE TRIGGER #{details.trigger_name}
-          AFTER #{update_keyword} ON \"#{options.schema_definition.schema_prefix}\".\"#{options.schema_definition.table_name}\" FOR EACH ROW
-          EXECUTE PROCEDURE \"#{options.schema_definition.schema_prefix}\".#{details.function_name}();
+        DROP TRIGGER IF EXISTS #{details.trigger_name} on \"#{options.schema_definition.schema_prefix}\".\"#{options.schema_definition.table_name}\";
         """,
         []
       )
-    else
+
       Ecto.Adapters.SQL.query!(
         repo_mod,
         """
-        CREATE OR REPLACE TRIGGER #{details.trigger_name}
+        CREATE TRIGGER #{details.trigger_name}
           AFTER #{update_keyword} ON \"#{options.schema_definition.schema_prefix}\".\"#{options.schema_definition.table_name}\" FOR EACH ROW
           EXECUTE PROCEDURE \"#{options.schema_definition.schema_prefix}\".#{details.function_name}();
         """,
@@ -266,39 +267,34 @@ defmodule EctoWatch.WatcherServer do
   end
 
   defp validate_watcher_details!(watcher_details, watcher_options) do
-    case Ecto.Adapters.SQL.query!(watcher_details.repo_mod, "SHOW max_identifier_length", []) do
-      %Postgrex.Result{rows: [[max_identifier_length]]} ->
-        max_identifier_length = String.to_integer(max_identifier_length)
+    max_identifier_length =
+      DB.max_identifier_length(watcher_details.repo_mod)
 
-        max_byte_size =
-          max(
-            byte_size(watcher_details.function_name),
-            byte_size(watcher_details.trigger_name)
-          )
-
-        if max_byte_size > max_identifier_length do
-          difference = max_byte_size - max_identifier_length
+    max_byte_size =
+      max(
+        byte_size(watcher_details.function_name),
+        byte_size(watcher_details.trigger_name)
+      )
 
-          if watcher_options.label do
-            raise """
-              Error for watcher: #{inspect(identifier(watcher_options))}
+    if max_byte_size > max_identifier_length do
+      difference = max_byte_size - max_identifier_length
 
-              Label is #{difference} character(s) too long to be part of the Postgres trigger name.
-            """
-          else
-            raise """
-              Error for watcher: #{inspect(identifier(watcher_options))}
+      if watcher_options.label do
+        raise """
+          Error for watcher: #{inspect(identifier(watcher_options))}
 
-              Schema module name is #{difference} character(s) too long for the auto-generated Postgres trigger name.
+          Label is #{difference} character(s) too long to be part of the Postgres trigger name.
+        """
+      else
+        raise """
+          Error for watcher: #{inspect(identifier(watcher_options))}
 
-              You may want to use the `label` option
+          Schema module name is #{difference} character(s) too long for the auto-generated Postgres trigger name.
 
-            """
-          end
-        end
+          You may want to use the `label` option
 
-      other ->
-        raise "Unexpected result when querying for max_identifier_length: #{inspect(other)}"
+        """
+      end
     end
   end
 

mix.exs

@@ -49,7 +49,8 @@ defmodule EctoWatch.MixProject do
       {:ecto_sql, ">= 3.0.0"},
       {:ex_doc, ">= 0.0.0", only: :dev, runtime: false},
       {:mix_test_watch, "~> 1.0", only: [:dev, :test]},
-      {:credo, "~> 1.7", only: [:dev, :test], runtime: false}
+      {:credo, "~> 1.7", only: [:dev, :test], runtime: false},
+      {:mox, "~> 1.2", only: [:dev, :test]}
     ]
   end
 

mix.lock

@@ -13,7 +13,9 @@
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
   "makeup_erlang": {:hex, :makeup_erlang, "1.0.0", "6f0eff9c9c489f26b69b61440bf1b238d95badae49adac77973cbacae87e3c2e", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "ea7a9307de9d1548d2a72d299058d1fd2339e3d398560a0e46c27dab4891e4d2"},
   "mix_test_watch": {:hex, :mix_test_watch, "1.2.0", "1f9acd9e1104f62f280e30fc2243ae5e6d8ddc2f7f4dc9bceb454b9a41c82b42", [:mix], [{:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}], "hexpm", "278dc955c20b3fb9a3168b5c2493c2e5cffad133548d307e0a50c7f2cfbf34f6"},
+  "mox": {:hex, :mox, "1.2.0", "a2cd96b4b80a3883e3100a221e8adc1b98e4c3a332a8fc434c39526babafd5b3", [:mix], [{:nimble_ownership, "~> 1.0", [hex: :nimble_ownership, repo: "hexpm", optional: false]}], "hexpm", "c7b92b3cc69ee24a7eeeaf944cd7be22013c52fcb580c1f33f50845ec821089a"},
   "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},
+  "nimble_ownership": {:hex, :nimble_ownership, "1.0.1", "f69fae0cdd451b1614364013544e66e4f5d25f36a2056a9698b793305c5aa3a6", [:mix], [], "hexpm", "3825e461025464f519f3f3e4a1f9b68c47dc151369611629ad08b636b73bb22d"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
   "phoenix_pubsub": {:hex, :phoenix_pubsub, "2.1.3", "3168d78ba41835aecad272d5e8cd51aa87a7ac9eb836eabc42f6e57538e3731d", [:mix], [], "hexpm", "bba06bc1dcfd8cb086759f0edc94a8ba2bc8896d5331a1e2c2902bf8e36ee502"},
   "postgrex": {:hex, :postgrex, "0.18.0", "f34664101eaca11ff24481ed4c378492fed2ff416cd9b06c399e90f321867d7e", [:mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "a042989ba1bc1cca7383ebb9e461398e3f89f868c92ce6671feb7ef132a252d1"},

test/ecto_watch_test.exs

@@ -5,6 +5,11 @@ defmodule EctoWatchTest do
 
   import ExUnit.CaptureLog
 
+  import Mox
+
+  setup :set_mox_from_context
+  setup :verify_on_exit!
+
   # TODO: Long module names (testing for limits of postgres labels)
   # TODO: More tests for label option
   # TODO: Pass non-lists to `extra_columns`
@@ -74,7 +79,18 @@ defmodule EctoWatchTest do
     end
   end
 
+  defmodule EctoWatchDBStub do
+    @moduledoc false
+
+    @behaviour EctoWatch.DB
+
+    def max_identifier_length(_), do: 63
+    def major_version(_), do: 17
+  end
+
   setup do
+    Mox.stub_with(EctoWatch.DB.Mock, EctoWatchDBStub)
+
     start_supervised!(TestRepo)
 
     start_supervised!({Phoenix.PubSub, name: TestPubSub})
@@ -1888,14 +1904,15 @@ defmodule EctoWatchTest do
   end
 
   describe "trigger creation" do
-    test "uses drop/create trigger when legacy_postgres_support? is true" do
+    test "uses drop/create trigger when PG major version is <= 13" do
+      expect(EctoWatch.DB.Mock, :major_version, fn _ -> 13 end)
+
       log =
         capture_log(fn ->
           start_supervised!(
             {EctoWatch,
              repo: TestRepo,
              pub_sub: TestPubSub,
-             legacy_postgres_support?: true,
              watchers: [
                {Thing, :inserted}
              ]}
@@ -1916,20 +1933,23 @@ defmodule EctoWatchTest do
         )
     end
 
-    test "uses create or replace trigger when legacy_postgres_support? is false" do
+    test "uses create or replace trigger when PG major version is >= 14" do
+      expect(EctoWatch.DB.Mock, :major_version, fn _ -> 14 end)
+
       log =
         capture_log(fn ->
           start_supervised!(
             {EctoWatch,
              repo: TestRepo,
              pub_sub: TestPubSub,
-             legacy_postgres_support?: false,
              watchers: [
                {Thing, :inserted}
              ]}
           )
         end)
 
+      assert log =~ "CREATE OR REPLACE TRIGGER ew_inserted_for_ectowatchtest_thing_trigger"
+
       refute log =~ "DROP TRIGGER IF EXISTS ew_inserted_for_ectowatchtest_thing_trigger"
 
       %Postgrex.Result{rows: [["ew_inserted_for_ectowatchtest_thing_trigger"]]} =

test/test_helper.exs

@@ -1 +1,3 @@
+Mox.defmock(EctoWatch.DB.Mock, for: EctoWatch.DB)
+
 ExUnit.start()