Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent ash_postgres v2.0.10 from being used #9

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

hwuethrich
Copy link
Contributor

As mentioned in #8 I noticed that ash_postgres v2.0.10 will break the tests and possibly other apps using this extension with the following error:

  6) test AshUUID.PrefixedV4 testing pineapples (AshUUIDTest)
     test/ash_uuid_test.exs:176
     ** (Ash.Error.Unknown) Unknown Error

     * ** (Postgrex.Error) ERROR 22P02 (invalid_text_representation) invalid input syntax for type uuid: "pineapple-smoothie_02wsgCff7yup2N8ZhJUxLc". If you are trying to query a JSON field, the parameter may need to be interpolated. Instead of

         p.json["field"] != "value"

     do

         p.json["field"] != ^"value"


         query: SELECT p0."id", p0."secondary_id", p0."inserted_at", p0."pineapple_smoothie_id" FROM "pineapples" AS p0 WHERE (p0."pineapple_smoothie_id"::uuid IN ('pineapple-smoothie_02wsgCff7yup2N8ZhJUxLc'))
         at pineapples

This PR is just here to reproduce the errors in CI with updated dependencies.

@hwuethrich
Copy link
Contributor Author

@zachdaniel Sorry for bothering you, but I'm trying to fix this but it might actually be a bug in ash_postgres.

I used git bisect and it looks like this issue was introduced in ash-project/ash_postgres@526020e. I suspect the latest changes to AshPostgres.SqlImplementation.determine_types/2 might have introduced this bug.

It only fails when loading a relationship (like Ash.load!(lime_smoothie, :limes)) and I'm currenty dbging the return value of the function:

diff --git a/lib/sql_implementation.ex b/lib/sql_implementation.ex
index d037d2d..7310431 100644
--- a/lib/sql_implementation.ex
+++ b/lib/sql_implementation.ex
@@ -293,6 +293,7 @@ defmodule AshPostgres.SqlImplementation do
           end
         end)
     end
+    |> dbg
   end

   defp closest_fitting_type(types, values) do

Here the output when running the a single test and the result of determine_types/2:


ash_postgres v2.0.9
$ mix test test/ash_uuid_test.exs:75
Running ExUnit with seed: 265972, max_cases: 16
Excluding tags: [:test]
Including tags: [location: {"test/ash_uuid_test.exs", 75}]

[(ash_postgres 2.0.10) lib/sql_implementation.ex:268: AshPostgres.SqlImplementation.determine_types/2]
cond do
  :erlang.function_exported(mod, :types, 0) -> mod.types()
  :erlang.function_exported(mod, :args, 0) -> mod.args()
  true -> [:any]
end #=> [[:any, {:array, :same}]]
|> Enum.concat(Map.keys(Ash.Query.Operator.operator_overloads(name) || %{})) #=> [[:any, {:array, :same}]]
|> Enum.map(fn types ->
  case types do
    :same ->
      types =
        for _ <- values do
          :same
        end

      closest_fitting_type(types, values)

    :any ->
      for _ <- values do
        :any
      end

    types ->
      closest_fitting_type(types, values)
  end
end) #=> [
  [
    {:parameterized, AshUUID.UUID.EctoType,
     [
       migration_default?: false,
       prefix: "lime-smoothie",
       version: 4,
       encoded?: true,
       prefixed?: false,
       strict?: true
     ]},
    {:in,
     {:parameterized, AshUUID.UUID.EctoType,
      [
        migration_default?: false,
        prefix: "lime-smoothie",
        version: 4,
        encoded?: true,
        prefixed?: false,
        strict?: true
      ]}}
  ]
]
|> Enum.filter(fn types -> Enum.all?(types, &(vagueness(&1) == 0)) end) #=> [
  [
    {:parameterized, AshUUID.UUID.EctoType,
     [
       migration_default?: false,
       prefix: "lime-smoothie",
       version: 4,
       encoded?: true,
       prefixed?: false,
       strict?: true
     ]},
    {:in,
     {:parameterized, AshUUID.UUID.EctoType,
      [
        migration_default?: false,
        prefix: "lime-smoothie",
        version: 4,
        encoded?: true,
        prefixed?: false,
        strict?: true
      ]}}
  ]
]
|> case do
  [type] ->
    if type == :any || type == {:in, :any} do
      nil
    else
      type
    end

  _ ->
    Enum.map(values, fn _ -> nil end)
end #=> [
  {:parameterized, AshUUID.UUID.EctoType,
   [
     migration_default?: false,
     prefix: "lime-smoothie",
     version: 4,
     encoded?: true,
     prefixed?: false,
     strict?: true
   ]},
  {:in,
   {:parameterized, AshUUID.UUID.EctoType,
    [
      migration_default?: false,
      prefix: "lime-smoothie",
      version: 4,
      encoded?: true,
      prefixed?: false,
      strict?: true
    ]}}
]
.
Finished in 0.4 seconds (0.00s async, 0.4s sync)
14 tests, 0 failures, 13 excluded
[
  {:parameterized, AshUUID.UUID.EctoType,
   [
     migration_default?: false,
     prefix: "lime-smoothie",
     version: 4,
     encoded?: true,
     prefixed?: false,
     strict?: true
   ]},
  {:in,
   {:parameterized, AshUUID.UUID.EctoType,
    [
      migration_default?: false,
      prefix: "lime-smoothie",
      version: 4,
      encoded?: true,
      prefixed?: false,
      strict?: true
    ]}}
]

ash_postgres v2.0.10
mix test test/ash_uuid_test.exs:75
Running ExUnit with seed: 651359, max_cases: 16
Excluding tags: [:test]
Including tags: [location: {"test/ash_uuid_test.exs", 75}]

[(ash_postgres 2.0.9) lib/sql_implementation.ex:296: AshPostgres.SqlImplementation.determine_types/2]
cond do
  :erlang.function_exported(mod, :types, 0) -> mod.types()
  :erlang.function_exported(mod, :args, 0) -> mod.args()
  true -> [:any]
end #=> [[:any, {:array, :same}]]
|> then(fn types ->
  Enum.concat(Map.keys(Ash.Query.Operator.operator_overloads(name) || %{}), types)
end) #=> [[:any, {:array, :same}]]
|> Enum.flat_map(fn types ->
  case types do
    :same ->
      types =
        for _ <- values do
          :same
        end

      [closest_fitting_type(types, values)]

    :any ->
      []

    types ->
      [types]
  end
end) #=> [[:any, {:array, :same}]]
|> Enum.filter(fn types -> Enum.all?(types, &(vagueness(&1) == 0)) end) #=> []
|> case do
  [types] ->
    types

  types ->
    Enum.find_value(types, Enum.map(values, fn _ -> nil end), fn types ->
      if length(types) == length(values) do
        types
        |> Enum.zip(values)
        |> Enum.reduce_while([], fn {type, value}, vals ->
          type = Ash.Type.get_type(type)

          if Ash.Type.ash_type?(type) do
            {type, constraints} =
              case type do
                {type, constraints} -> {type, constraints}
                type -> {type, []}
              end

            case value do
              %Ash.Query.Function.Type{arguments: [_, ^type | _]} ->
                {:cont, vals ++ [:any]}

              %Ash.Query.Ref{attribute: %{type: ^type}} ->
                {:cont, vals ++ [:any]}

              _ ->
                if Ash.Type.matches_type?(type, value, constraints) do
                  {:cont, vals ++ [parameterized_type(type, constraints)]}
                else
                  {:halt, nil}
                end
            end
          else
            {:halt, nil}
          end
        end)
      end
    end)
end #=> [nil, nil]



  1) test AshUUID.PrefixedV4 testing limes (AshUUIDTest)
     test/ash_uuid_test.exs:75
     ** (Ash.Error.Unknown) Unknown Error

     * ** (Postgrex.Error) ERROR 22P02 (invalid_text_representation) invalid input syntax for type uuid: "5AbK9g2dZBR6DMhp7zg7vF". If you are trying to query a JSON field, the parameter may need to be interpolated. Instead of

         p.json["field"] != "value"

     do

         p.json["field"] != ^"value"


         query: SELECT l0."id", l0."inserted_at", l0."lime_smoothie_id" FROM "limes" AS l0 WHERE (l0."lime_smoothie_id"::uuid IN ('5AbK9g2dZBR6DMhp7zg7vF'))
         at limes
       (ecto_sql 3.11.3) lib/ecto/adapters/sql.ex:1054: Ecto.Adapters.SQL.raise_sql_call_error/1
       (ecto_sql 3.11.3) lib/ecto/adapters/sql.ex:952: Ecto.Adapters.SQL.execute/6
       (ecto 3.11.2) lib/ecto/repo/queryable.ex:232: Ecto.Repo.Queryable.execute/4
       (ecto 3.11.2) lib/ecto/repo/queryable.ex:19: Ecto.Repo.Queryable.all/3
       (ash_postgres 2.0.10) lib/data_layer.ex:758: anonymous fn/3 in AshPostgres.DataLayer.run_query/2
       (ash_postgres 2.0.10) lib/data_layer.ex:756: AshPostgres.DataLayer.run_query/2
       (ash 3.0.15) lib/ash/actions/read/read.ex:2423: Ash.Actions.Read.run_query/4
       (ash 3.0.15) lib/ash/actions/read/read.ex:447: anonymous fn/5 in Ash.Actions.Read.do_read/4
       (ash 3.0.15) lib/ash/actions/read/read.ex:777: Ash.Actions.Read.maybe_in_transaction/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:248: Ash.Actions.Read.do_run/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:66: anonymous fn/3 in Ash.Actions.Read.run/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:65: Ash.Actions.Read.run/3
       (ash 3.0.15) lib/ash/actions/read/relationships.ex:496: anonymous fn/3 in Ash.Actions.Read.Relationships.do_fetch_related_records/3
       (elixir 1.17.0) lib/enum.ex:1711: anonymous fn/3 in Enum.map/2
       (elixir 1.17.0) lib/enum.ex:4423: anonymous fn/3 in Enum.map/2
       (elixir 1.17.0) lib/stream.ex:1879: anonymous fn/3 in Enumerable.Stream.reduce/3
       (elixir 1.17.0) lib/enum.ex:4858: Enumerable.List.reduce/3
       (elixir 1.17.0) lib/stream.ex:1891: Enumerable.Stream.do_each/4
       (elixir 1.17.0) lib/enum.ex:4423: Enum.map/2
       (ash 3.0.15) lib/ash/actions/read/relationships.ex:43: Ash.Actions.Read.Relationships.fetch_related_records/2
     stacktrace:
       (elixir 1.17.0) lib/process.ex:864: Process.info/2
       (ash 3.0.15) lib/ash/error/unknown.ex:3: Ash.Error.Unknown."exception (overridable 2)"/1
       (ash 3.0.15) /Users/hw/Projects/ash_uuid/deps/splode/lib/splode.ex:211: Ash.Error.to_class/2
       (ash 3.0.15) lib/ash/error/error.ex:66: Ash.Error.to_error_class/2
       (ash 3.0.15) lib/ash/actions/read/read.ex:319: anonymous fn/2 in Ash.Actions.Read.do_run/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:258: Ash.Actions.Read.do_run/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:66: anonymous fn/3 in Ash.Actions.Read.run/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:65: Ash.Actions.Read.run/3
       (ash 3.0.15) lib/ash/actions/read/relationships.ex:496: anonymous fn/3 in Ash.Actions.Read.Relationships.do_fetch_related_records/3
       (elixir 1.17.0) lib/enum.ex:1711: anonymous fn/3 in Enum.map/2
       (elixir 1.17.0) lib/enum.ex:4423: anonymous fn/3 in Enum.map/2
       (elixir 1.17.0) lib/stream.ex:1879: anonymous fn/3 in Enumerable.Stream.reduce/3
       (elixir 1.17.0) lib/enum.ex:4858: Enumerable.List.reduce/3
       (elixir 1.17.0) lib/stream.ex:1891: Enumerable.Stream.do_each/4
       (elixir 1.17.0) lib/enum.ex:4423: Enum.map/2
       (ash 3.0.15) lib/ash/actions/read/relationships.ex:43: Ash.Actions.Read.Relationships.fetch_related_records/2
       (ash 3.0.15) lib/ash/actions/read/relationships.ex:24: Ash.Actions.Read.Relationships.load/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:270: Ash.Actions.Read.do_run/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:66: anonymous fn/3 in Ash.Actions.Read.run/3
       (ash 3.0.15) lib/ash/actions/read/read.ex:65: Ash.Actions.Read.run/3


Finished in 0.3 seconds (0.00s async, 0.3s sync)
14 tests, 1 failure, 13 excluded
[nil, nil]

It looks like you were maybe right with your comment? 😆 When I remove the vagueness-filter, the function returns [:any, {:array, :same}] which results in another error:

  1) test AshUUID.PrefixedV4 testing limes (AshUUIDTest)
     test/ash_uuid_test.exs:75
     ** (Ash.Error.Unknown) Unknown Error

     * ** (UndefinedFunctionError) function :same.cast/1 is undefined (module :same is not available)
         at limes
       :same.cast("4yWxSasWegNNwmFy4dGH21")
       (ecto 3.11.2) lib/ecto/type.ex:1339: Ecto.Type.array/4
       (elixir 1.17.0) lib/enum.ex:2531: Enum."-reduce/3-lists^foldl/2-0-"/3
       (elixir 1.17.0) lib/enum.ex:1829: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3
       (elixir 1.17.0) lib/enum.ex:2531: Enum."-reduce/3-lists^foldl/2-0-"/3
       (ecto 3.11.2) lib/ecto/repo/queryable.ex:214: Ecto.Repo.Queryable.execute/4
       (ecto 3.11.2) lib/ecto/repo/queryable.ex:19: Ecto.Repo.Queryable.all/3
       (ash_postgres 2.0.10) lib/data_layer.ex:758: anonymous fn/3 in AshPostgres.DataLayer.run_query/2
       (ash_postgres 2.0.10) lib/data_layer.ex:756: AshPostgres.DataLayer.run_query/2

Any help would be appreciated! I can also report an issue in ash_postgres if you think this belongs there.

@hwuethrich
Copy link
Contributor Author

Actually, if I change the following line, the tests in ash_uuid pass!

diff --git a/lib/sql_implementation.ex b/lib/sql_implementation.ex
index b3242b6..08558fa 100644
--- a/lib/sql_implementation.ex
+++ b/lib/sql_implementation.ex
@@ -248,7 +248,7 @@ defmodule AshPostgres.SqlImplementation do
           []

         types ->
-          [types]
+          [closest_fitting_type(types, values)]
       end
     end)
     # this doesn't seem right to me

@moissela
Copy link
Member

Hi @hwuethrich
I'll try to fix this tomorrow (Italy work time).

I think the issue is that AshUUID.UUID should implement the new "load" callback of the Ash.Type behaviour.

Thank you for notice this bug, opening the PR and your contribution!

@zachdaniel
Copy link
Contributor

This is definitely a bug in ash_postgres, I will fix. There should be no changes required on your end.

@zachdaniel
Copy link
Contributor

I've just pushed a new version of ash_postgres that should resolve these issues.

@zachdaniel
Copy link
Contributor

Apologies for the breaking change 🙇

@hwuethrich
Copy link
Contributor Author

@zachdaniel No worries and thanks for the quick fix!

@moissela Should I close this PR or would you like to merge my changes anyway? Maybe adding a dependency constraint for ash_postgres > 2.0.10?

@hwuethrich hwuethrich changed the title Support for ash_postgres v2.0.10 Prevent ash_postgres v2.0.10 from being used Jun 20, 2024
@hwuethrich hwuethrich marked this pull request as ready for review June 20, 2024 08:06
@hwuethrich
Copy link
Contributor Author

@moissela I updated the dependencies to require ash_postgres 2.0.11 or later and the tests are now passing.

Not sure if the Ash.Type.load/4 is required. Ash.Type.UUID doesn't implement it.

@moissela moissela changed the base branch from main to zebbra/update_deps June 20, 2024 10:51
@moissela moissela deleted the branch zoonect-oss:zebbra/update_deps June 20, 2024 10:54
@moissela moissela closed this Jun 20, 2024
@moissela moissela reopened this Jun 20, 2024
@moissela moissela merged commit 643ac21 into zoonect-oss:zebbra/update_deps Jun 20, 2024
12 checks passed
@moissela
Copy link
Member

moissela commented Jun 20, 2024

Thanks @hwuethrich and @zachdaniel, I'm merging this into main this through #11 shortly.

I'm releasing to hex today.

@moissela
Copy link
Member

Released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants