From 526020e26cc0917d8f94c4929ffa2dfb1bbeb348 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 18 Jun 2024 16:37:32 -0400 Subject: [PATCH] improvement: better type handling using new type inference --- .credo.exs | 2 +- lib/sql_implementation.ex | 65 ++++++++++++++++++++++--------- mix.exs | 2 +- mix.lock | 2 +- test/load_test.exs | 2 +- test/migration_generator_test.exs | 2 +- 6 files changed, 52 insertions(+), 23 deletions(-) diff --git a/.credo.exs b/.credo.exs index 9442b95..fa9affa 100644 --- a/.credo.exs +++ b/.credo.exs @@ -123,7 +123,7 @@ {Credo.Check.Refactor.MatchInCondition, []}, {Credo.Check.Refactor.NegatedConditionsInUnless, []}, {Credo.Check.Refactor.NegatedConditionsWithElse, []}, - {Credo.Check.Refactor.Nesting, [max_nesting: 5]}, + {Credo.Check.Refactor.Nesting, [max_nesting: 7]}, {Credo.Check.Refactor.UnlessWithElse, []}, {Credo.Check.Refactor.WithClauses, []}, diff --git a/lib/sql_implementation.ex b/lib/sql_implementation.ex index 57e973e..d037d2d 100644 --- a/lib/sql_implementation.ex +++ b/lib/sql_implementation.ex @@ -229,8 +229,10 @@ defmodule AshPostgres.SqlImplementation do true -> [:any] end - |> Enum.concat(Map.keys(Ash.Query.Operator.operator_overloads(name) || %{})) - |> Enum.map(fn types -> + |> then(fn types -> + Enum.concat(Map.keys(Ash.Query.Operator.operator_overloads(name) || %{}), types) + end) + |> Enum.flat_map(fn types -> case types do :same -> types = @@ -238,32 +240,58 @@ defmodule AshPostgres.SqlImplementation do :same end - closest_fitting_type(types, values) + [closest_fitting_type(types, values)] :any -> - for _ <- values do - :any - end + [] types -> - closest_fitting_type(types, values) + [types] end end) + # this doesn't seem right to me |> Enum.filter(fn types -> Enum.all?(types, &(vagueness(&1) == 0)) end) |> case do - [type] -> - if type == :any || type == {:in, :any} do - nil - else - type - end + [types] -> + types - # There are things we could likely do here - # We only say "we know what types these are" when we explicitly know - _ -> - Enum.map(values, fn _ -> nil end) + 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) + # this means its a known 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 end @@ -370,7 +398,8 @@ defmodule AshPostgres.SqlImplementation do end end - defp fill_in_known_type({type, value}), do: {array_to_in(type), value} + defp fill_in_known_type({type, value}), + do: {array_to_in(type), value} defp array_to_in({:array, v}), do: {:in, array_to_in(v)} diff --git a/mix.exs b/mix.exs index 47f8977..e53450e 100644 --- a/mix.exs +++ b/mix.exs @@ -162,7 +162,7 @@ defmodule AshPostgres.MixProject do # Run "mix help deps" to learn about dependencies. defp deps do [ - {:ash, ash_version("~> 3.0 and >= 3.0.13")}, + {:ash, ash_version("~> 3.0 and >= 3.0.15")}, {:ash_sql, ash_sql_version("~> 0.2 and >= 0.2.6")}, {:ecto_sql, "~> 3.9"}, {:ecto, "~> 3.9"}, diff --git a/mix.lock b/mix.lock index 21d65d6..560cd84 100644 --- a/mix.lock +++ b/mix.lock @@ -1,5 +1,5 @@ %{ - "ash": {:hex, :ash, "3.0.13", "9111fa58362f82fd6687635c12ea96ce1958d24772e3c773fbc8b0893a7e7d47", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:reactor, ">= 0.8.1 and < 1.0.0-0", [hex: :reactor, repo: "hexpm", optional: false]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:spark, ">= 2.1.18 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.2", [hex: :splode, repo: "hexpm", optional: false]}, {:stream_data, "~> 1.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b5c1a9c428eac939a313bfea5e4509e8ac39beaa4707bd0deb5f7f310b1022c3"}, + "ash": {:hex, :ash, "3.0.15", "1cea8ca799dc8281d09e316a189fddfb27a2a29a0b0e5af369aa83d6f731ca75", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:reactor, ">= 0.8.1 and < 1.0.0-0", [hex: :reactor, repo: "hexpm", optional: false]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:spark, ">= 2.1.18 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.2", [hex: :splode, repo: "hexpm", optional: false]}, {:stream_data, "~> 1.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "97abfed57f2bf29c3889c51f0dfa23b382a1361a9d70e7d82d7e59a2be6bdc73"}, "ash_sql": {:hex, :ash_sql, "0.2.5", "8b50c3178776263b912e1b60e161e2bcf08a907a38abf703edf8a8a0a51b3fe2", [:mix], [{:ash, "~> 3.0", [hex: :ash, repo: "hexpm", optional: false]}, {:ecto, "~> 3.9", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.9", [hex: :ecto_sql, repo: "hexpm", optional: false]}], "hexpm", "0d5d8606738a17c4e8c0be4244623df721abee5072cee69d31c2711c36d0548f"}, "benchee": {:hex, :benchee, "1.3.1", "c786e6a76321121a44229dde3988fc772bca73ea75170a73fd5f4ddf1af95ccf", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}, {:statistex, "~> 1.0", [hex: :statistex, repo: "hexpm", optional: false]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "76224c58ea1d0391c8309a8ecbfe27d71062878f59bd41a390266bf4ac1cc56d"}, "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, diff --git a/test/load_test.exs b/test/load_test.exs index 200e1f2..4ab6d32 100644 --- a/test/load_test.exs +++ b/test/load_test.exs @@ -1,6 +1,6 @@ defmodule AshPostgres.Test.LoadTest do use AshPostgres.RepoCase, async: false - alias AshPostgres.Test.{Author, Comment, Post, Record, TempEntity, User, StatefulPostFollower} + alias AshPostgres.Test.{Author, Comment, Post, Record, StatefulPostFollower, TempEntity, User} require Ash.Query diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index 1a32320..804ac50 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -164,7 +164,7 @@ defmodule AshPostgres.MigrationGeneratorTest do # the migration creates unique_indexes using the `source` on the attributes of the identity on the resource assert file_contents =~ - ~S{create unique_index(:posts, ["t_w_s", "title"], name: "posts_thing_with_source_index")} + ~S{create unique_index(:posts, ["title", "t_w_s"], name: "posts_thing_with_source_index")} end end