fix: remove Agent "convenience" for determining min pg version

We need to require that users provide this function. To that end we're
adding a warning in a minor release branch telling users to define this.
The agent was acting as a bottleneck that all queries must go through,
causing nontrivial performance issues at scale.

Author: zachdaniel

lib/data_layer/info.ex

@@ -42,8 +42,15 @@ defmodule AshPostgres.DataLayer.Info do
   @doc "Gets the resource's repo's postgres version"
   def min_pg_version(resource) do
     case repo(resource, :read).min_pg_version() do
-      %Version{} = version -> version
-      string when is_binary(string) -> Version.parse!(string)
+      %Version{} = version ->
+        version
+
+      string when is_binary(string) ->
+        IO.warn(
+          "Got a `string` for min_pg_version, expected a `Version` struct. Got: #{inspect(string)}. Please call `Version.parse!` before returning the value."
+        )
+
+        Version.parse!(string)
     end
   end
 

lib/repo.ex

@@ -86,10 +86,10 @@ defmodule AshPostgres.Repo do
           otp_app: otp_app
       end
 
-      @agent __MODULE__.AshPgVersion
       @behaviour AshPostgres.Repo
       @warn_on_missing_ash_functions Keyword.get(opts, :warn_on_missing_ash_functions?, true)
       @after_compile __MODULE__
+      @before_compile AshPostgres.Repo.BeforeCompile
       require Logger
 
       defoverridable insert: 2, insert: 1, insert!: 2, insert!: 1
@@ -123,18 +123,6 @@ defmodule AshPostgres.Repo do
       end
 
       def init(type, config) do
-        if type == :supervisor do
-          try do
-            Agent.stop(@agent)
-          rescue
-            _ ->
-              :ok
-          catch
-            _, _ ->
-              :ok
-          end
-        end
-
         new_config =
           config
           |> Keyword.put(:installed_extensions, installed_extensions())
@@ -208,97 +196,6 @@ defmodule AshPostgres.Repo do
         end
       end
 
-      def min_pg_version do
-        if version = cached_version() do
-          version
-        else
-          lookup_version()
-        end
-      end
-
-      defp cached_version do
-        if config()[:pool] == Ecto.Adapters.SQL.Sandbox do
-          Agent.start_link(
-            fn ->
-              nil
-            end,
-            name: @agent
-          )
-
-          case Agent.get(@agent, fn state -> state end) do
-            nil ->
-              version = lookup_version()
-
-              Agent.update(@agent, fn _ ->
-                version
-              end)
-
-              version
-
-            version ->
-              version
-          end
-        else
-          Agent.start_link(
-            fn ->
-              lookup_version()
-            end,
-            name: @agent
-          )
-
-          Agent.get(@agent, fn state -> state end)
-        end
-      end
-
-      defp lookup_version do
-        version_string =
-          try do
-            query!("SELECT version()").rows |> Enum.at(0) |> Enum.at(0)
-          rescue
-            error ->
-              reraise """
-                      Got an error while trying to read postgres version
-
-                      Error:
-
-                      #{inspect(error)}
-                      """,
-                      __STACKTRACE__
-          end
-
-        try do
-          version_string
-          |> String.split(" ")
-          |> Enum.at(1)
-          |> String.split(".")
-          |> case do
-            [major] ->
-              "#{major}.0.0"
-
-            [major, minor] ->
-              "#{major}.#{minor}.0"
-
-            other ->
-              Enum.join(other, ".")
-          end
-          |> Version.parse!()
-        rescue
-          error ->
-            reraise(
-              """
-              Could not parse postgres version from version string: "#{version_string}"
-
-              You may need to define the `min_version/0` callback yourself.
-
-              Error:
-
-              #{inspect(error)}
-              """,
-              __STACKTRACE__
-            )
-        end
-      end
-
       def from_ecto(other), do: other
 
       def to_ecto(nil), do: nil
@@ -336,7 +233,6 @@ defmodule AshPostgres.Repo do
       defoverridable init: 2,
                      on_transaction_begin: 1,
                      installed_extensions: 0,
-                     min_pg_version: 0,
                      all_tenants: 0,
                      tenant_migrations_path: 0,
                      default_prefix: 0,

lib/repo/before_compile.ex

@@ -0,0 +1,25 @@
+defmodule AshPostgres.Repo.BeforeCompile do
+  @moduledoc false
+
+  defmacro __before_compile__(_env) do
+    quote do
+      unless Module.defines?(__MODULE__, {:min_pg_version, 0}, :def) do
+        IO.warn("""
+        Please define `min_pg_version/0` in repo module: #{inspect(__MODULE__)}
+
+        For example:
+
+            def min_pg_version do
+              %Version{major: 16, minor: 0, patch: 0}
+            end
+
+        The lowest compatible version is being assumed.
+        """)
+
+        def min_pg_version do
+          %Version{major: 13, minor: 0, patch: 0}
+        end
+      end
+    end
+  end
+end

mix.lock

@@ -39,7 +39,7 @@
   "simple_sat": {:hex, :simple_sat, "0.1.3", "f650fc3c184a5fe741868b5ac56dc77fdbb428468f6dbf1978e14d0334497578", [:mix], [], "hexpm", "a54305066a356b7194dc81db2a89232bacdc0b3edaef68ed9aba28dcbc34887b"},
   "sobelow": {:hex, :sobelow, "0.13.0", "218afe9075904793f5c64b8837cc356e493d88fddde126a463839351870b8d1e", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "cd6e9026b85fc35d7529da14f95e85a078d9dd1907a9097b3ba6ac7ebbe34a0d"},
   "sourceror": {:hex, :sourceror, "1.6.0", "9907884e1449a4bd7dbaabe95088ed4d9a09c3c791fb0103964e6316bc9448a7", [:mix], [], "hexpm", "e90aef8c82dacf32c89c8ef83d1416fc343cd3e5556773eeffd2c1e3f991f699"},
-  "spark": {:hex, :spark, "2.2.11", "6589ac0e50d69e5095871a5e8f3bb6107755b1cc71f05a31d7398902506dab9a", [:mix], [{:igniter, ">= 0.2.6 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.2", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "662d297d0ad49a5990a72cbf342d70e90894218062da2893f2df529f70ecc2b4"},
+  "spark": {:hex, :spark, "2.2.16", "221f18b302f8b2df28ac5664c4a1abc8b9c1ba493676d9dc77234a8dbfcd07ae", [:mix], [{:igniter, ">= 0.2.6 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.2", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "143d3f78d717556041a1b89f523ba7f3004f90351a567e9ea9044f90afcd1085"},
   "spitfire": {:hex, :spitfire, "0.1.3", "7ea0f544005dfbe48e615ed90250c9a271bfe126914012023fd5e4b6b82b7ec7", [:mix], [], "hexpm", "d53b5107bcff526a05c5bb54c95e77b36834550affd5830c9f58760e8c543657"},
   "splode": {:hex, :splode, "0.2.4", "71046334c39605095ca4bed5d008372e56454060997da14f9868534c17b84b53", [:mix], [], "hexpm", "ca3b95f0d8d4b482b5357954fec857abd0fa3ea509d623334c1328e7382044c2"},
   "statistex": {:hex, :statistex, "1.0.0", "f3dc93f3c0c6c92e5f291704cf62b99b553253d7969e9a5fa713e5481cd858a5", [:mix], [], "hexpm", "ff9d8bee7035028ab4742ff52fc80a2aa35cece833cf5319009b52f1b5a86c27"},

test/aggregate_test.exs

@@ -345,8 +345,6 @@ defmodule AshSql.AggregateTest do
       |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove)
       |> Ash.create!()
 
-      Logger.configure(level: :debug)
-
       assert Comment
              |> Ash.Query.filter(post.has_comment_called_match)
              |> Ash.read_one!()

test/support/test_repo.ex

@@ -12,6 +12,10 @@ defmodule AshPostgres.TestRepo do
       Application.get_env(:ash_postgres, :no_extensions, [])
   end
 
+  def min_pg_version do
+    %Version{major: 16, minor: 0, patch: 0}
+  end
+
   def all_tenants do
     Code.ensure_compiled(AshPostgres.MultitenancyTest.Org)