From f3587e310958b531eaca3fcce975a691e812e711 Mon Sep 17 00:00:00 2001 From: dallingson Date: Wed, 4 Mar 2026 13:05:10 -0700 Subject: [PATCH 1/2] Add required!/1, ash_required/1, and ash_required!/2 (issue #261) - required!/1 and ash_required/1: custom expressions for presence in queries (replacing not is_nil(...)) - PostgreSQL ash_required/1 in ash_functions.ex for filters/calculations - Data layer can?(:required_error) and ash_required!/2 (RequiredError): when value is nil, return Ash.Error.Changes.Required; required_error/2 for the error message - Docs and tests for the new expressions and required_error --- config/config.exs | 6 +- documentation/topics/advanced/expressions.md | 44 ++++ lib/data_layer.ex | 9 +- lib/expressions/ash_required.ex | 26 ++ lib/expressions/required.ex | 31 +++ lib/functions/required_error.ex | 56 ++++ lib/migration_generator/ash_functions.ex | 18 +- .../migration_generator.ex | 3 +- test/aggregate_test.exs | 28 ++ test/calculation_test.exs | 30 +++ test/destroy_test.exs | 2 +- test/filter_test.exs | 240 ++++++++++++++++++ test/functions/required_error_test.exs | 27 ++ 13 files changed, 514 insertions(+), 6 deletions(-) create mode 100644 lib/expressions/ash_required.ex create mode 100644 lib/expressions/required.ex create mode 100644 lib/functions/required_error.ex create mode 100644 test/functions/required_error_test.exs diff --git a/config/config.exs b/config/config.exs index b01a79fd..56d38e23 100644 --- a/config/config.exs +++ b/config/config.exs @@ -35,7 +35,11 @@ if Mix.env() == :test do config :ash_postgres, :ash_domains, [AshPostgres.Test.Domain] - config :ash, :custom_expressions, [AshPostgres.Expressions.TrigramWordSimilarity] + config :ash, :custom_expressions, [ + AshPostgres.Expressions.TrigramWordSimilarity, + AshPostgres.Expressions.Required, + AshPostgres.Expressions.AshRequired + ] config :ash, :known_types, [AshPostgres.Timestamptz, AshPostgres.TimestamptzUsec] diff --git a/documentation/topics/advanced/expressions.md b/documentation/topics/advanced/expressions.md index c5a6a759..380b53bc 100644 --- a/documentation/topics/advanced/expressions.md +++ b/documentation/topics/advanced/expressions.md @@ -81,3 +81,47 @@ For example: ```elixir Ash.Query.filter(User, trigram_similarity(first_name, "fred") > 0.8) ``` + +## required!/1 and ash_required/1 + +`required!/1` (and the equivalent `ash_required/1`) express that a value must be present (not nil). They are equivalent to `not is_nil(expr)`. In SQL they compile to `(expr) IS NOT NULL` or, when using the ash-functions extension, to the stored function `ash_required(expr)`. + +**Setup:** Add AshPostgres’s custom expressions to your Ash config so the expression parser knows about them (no changes to the main Ash repo needed): + +```elixir +# config/config.exs (or config/dev.exs, config/runtime.exs) +config :ash, :custom_expressions, [ + AshPostgres.Expressions.Required, + AshPostgres.Expressions.AshRequired +] +``` + +The **ash-functions extension** (installed via `mix ash_postgres.install_extensions` or migrations) includes an `ash_required(value)` SQL function that returns true when the value is not null. You can use it in raw SQL (e.g. fragments) as well. + +Use them in filters, calculations, aggregates, and `exists/2` when you want clearer intent than `not is_nil(...)`. + +### Examples + +```elixir +# Filter: only records where an optional attribute is set +Ash.Query.filter(Post, required!(post_category)) + +# Same using the explicit name +Ash.Query.filter(Post, ash_required(post_category)) + +# In aggregate query filters +Post +|> Ash.Query.aggregate(:count, :comments, query: [filter: expr(required!(title))]) + +# In calculations (e.g. "has value" flag) +calculate :has_rating, :boolean, expr(required!(latest_rating_score)) + +# In exists +Ash.Query.filter(Comment, exists(post, required!(id))) +``` + +### Semantics and SQL + +- **Semantics:** True when the argument is not nil; false when it is nil. +- **SQL:** Compiled to `(expression) IS NOT NULL` or to the extension function `ash_required(expression)` when the ash-functions extension is installed. +- **Edge cases:** Behaves like `not is_nil(expr)` over joins, nullable relationships, and in calculations. Use `required!(expr)` wherever you would use `not is_nil(expr)` for readability. diff --git a/lib/data_layer.ex b/lib/data_layer.ex index 99036fa3..a63fc6e7 100644 --- a/lib/data_layer.ex +++ b/lib/data_layer.ex @@ -777,6 +777,9 @@ defmodule AshPostgres.DataLayer do def can?(resource, :expr_error), do: not AshPostgres.DataLayer.Info.repo(resource, :mutate).disable_expr_error?() + def can?(resource, :required_error), + do: not AshPostgres.DataLayer.Info.repo(resource, :mutate).disable_expr_error?() + def can?(resource, {:filter_expr, %Ash.Query.Function.Error{}}) do not AshPostgres.DataLayer.Info.repo(resource, :mutate).disable_expr_error?() && "ash-functions" in AshPostgres.DataLayer.Info.repo(resource, :read).installed_extensions() && @@ -877,7 +880,8 @@ defmodule AshPostgres.DataLayer do functions = [ AshPostgres.Functions.Like, AshPostgres.Functions.ILike, - AshPostgres.Functions.Binding + AshPostgres.Functions.Binding, + AshPostgres.Functions.RequiredError ] functions = @@ -2669,6 +2673,7 @@ defmodule AshPostgres.DataLayer do case Ecto.Adapters.Postgres.Connection.to_constraints(error, []) do [] -> constraints = maybe_foreign_key_violation_constraints(error) + if constraints != [] do {:error, changeset @@ -2695,7 +2700,7 @@ defmodule AshPostgres.DataLayer do code = postgres[:code] || postgres["code"] constraint = postgres[:constraint] || postgres["constraint"] - if code in ["23503", 23503, :foreign_key_violation] and is_binary(constraint) do + if code in ["23503", 23_503, :foreign_key_violation] and is_binary(constraint) do [{:foreign_key, constraint}] else [] diff --git a/lib/expressions/ash_required.ex b/lib/expressions/ash_required.ex new file mode 100644 index 00000000..d5214ac5 --- /dev/null +++ b/lib/expressions/ash_required.ex @@ -0,0 +1,26 @@ +# SPDX-FileCopyrightText: 2019 ash_postgres contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshPostgres.Expressions.AshRequired do + @moduledoc """ + Same as `Required` but with the explicit name `ash_required`. + + Use in filters as `ash_required(field)`. Equivalent to `required!(field)` and `not is_nil(field)`. + + Register in your config: + + config :ash, :custom_expressions, [ + AshPostgres.Expressions.Required, + AshPostgres.Expressions.AshRequired + ] + """ + use Ash.CustomExpression, + name: :ash_required, + arguments: [[:any]], + predicate?: true + + def expression(data_layer, args) do + AshPostgres.Expressions.Required.expression(data_layer, args) + end +end diff --git a/lib/expressions/required.ex b/lib/expressions/required.ex new file mode 100644 index 00000000..03af82f1 --- /dev/null +++ b/lib/expressions/required.ex @@ -0,0 +1,31 @@ +# SPDX-FileCopyrightText: 2019 ash_postgres contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshPostgres.Expressions.Required do + @moduledoc """ + Custom expression that means "value must be present (not null)". + + Use in filters as `required!(field)` or `ash_required(field)`. + Equivalent to `not is_nil(field)`; compiles to `(expr) IS NOT NULL` in SQL. + + Register in your config so Ash knows about it: + + config :ash, :custom_expressions, [ + AshPostgres.Expressions.Required, + AshPostgres.Expressions.AshRequired + ] + """ + use Ash.CustomExpression, + name: :required!, + arguments: [[:any]], + predicate?: true + + require Ash.Expr + + def expression(AshPostgres.DataLayer, [arg]) do + {:ok, Ash.Expr.expr(not is_nil(^arg))} + end + + def expression(_data_layer, _args), do: :unknown +end diff --git a/lib/functions/required_error.ex b/lib/functions/required_error.ex new file mode 100644 index 00000000..63d00751 --- /dev/null +++ b/lib/functions/required_error.ex @@ -0,0 +1,56 @@ +# SPDX-FileCopyrightText: 2019 ash_postgres contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshPostgres.Functions.RequiredError do + @moduledoc """ + Expression that returns the value if present, or an error if nil. + Used for required-attribute validation (Part B): `ash_required!(^value, ^attribute)`. + + When the data layer supports `:required_error`, Ash can build + `expr(ash_required!(^value, ^attribute))` instead of the inline if/is_nil/error block. + This module is returned from the data layer's `functions/1` so the expression is available + when using AshPostgres. + """ + use Ash.Query.Function, name: :ash_required!, predicate?: false + + @impl true + def args, do: [[:any, :any]] + + @impl true + def new([value_expr, attribute]) when is_struct(attribute) or is_map(attribute) do + {:ok, %__MODULE__{arguments: [value_expr, attribute]}} + end + + def new(_), do: {:error, "ash_required! expects (value, attribute)"} + + @impl true + def evaluate(%{arguments: [value, attribute]}) do + if is_nil(value) do + resource = + Map.get(attribute, :resource) || raise("attribute must have :resource for ash_required!") + + field = + Map.get(attribute, :name) || Map.get(attribute, "name") || + raise("attribute must have :name for ash_required!") + + {:error, + Ash.Error.Changes.Required.exception( + field: field, + type: :attribute, + resource: resource + )} + else + {:known, value} + end + end + + @impl true + def can_return_nil?(_), do: false + + @impl true + def evaluate_nil_inputs?, do: true + + @impl true + def returns, do: :unknown +end diff --git a/lib/migration_generator/ash_functions.ex b/lib/migration_generator/ash_functions.ex index fded7252..f387373f 100644 --- a/lib/migration_generator/ash_functions.ex +++ b/lib/migration_generator/ash_functions.ex @@ -48,6 +48,8 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do IMMUTABLE; \"\"\") + #{ash_required()} + execute(\"\"\" CREATE OR REPLACE FUNCTION ash_trim_whitespace(arr text[]) RETURNS text[] AS $$ @@ -159,11 +161,13 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do execute("ALTER FUNCTION ash_raise_error(jsonb) STABLE;") execute("ALTER FUNCTION ash_raise_error(jsonb, ANYCOMPATIBLE) STABLE") #{uuid_generate_v7()} + #{ash_required()} """ end def drop(4) do """ + execute("DROP FUNCTION IF EXISTS ash_required(ANYCOMPATIBLE)") execute("ALTER FUNCTION ash_raise_error(jsonb) VOLATILE;") execute("ALTER FUNCTION ash_raise_error(jsonb, ANYCOMPATIBLE) VOLATILE") """ @@ -190,7 +194,19 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do end def drop(nil) do - "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_raise_error(jsonb), ash_raise_error(jsonb, ANYCOMPATIBLE), ash_elixir_and(BOOLEAN, ANYCOMPATIBLE), ash_elixir_and(ANYCOMPATIBLE, ANYCOMPATIBLE), ash_elixir_or(ANYCOMPATIBLE, ANYCOMPATIBLE), ash_elixir_or(BOOLEAN, ANYCOMPATIBLE), ash_trim_whitespace(text[])\")" + "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_raise_error(jsonb), ash_raise_error(jsonb, ANYCOMPATIBLE), ash_elixir_and(BOOLEAN, ANYCOMPATIBLE), ash_elixir_and(ANYCOMPATIBLE, ANYCOMPATIBLE), ash_elixir_or(ANYCOMPATIBLE, ANYCOMPATIBLE), ash_elixir_or(BOOLEAN, ANYCOMPATIBLE), ash_required(ANYCOMPATIBLE), ash_trim_whitespace(text[])\")" + end + + defp ash_required do + """ + execute(\"\"\" + CREATE OR REPLACE FUNCTION ash_required(value ANYCOMPATIBLE) + RETURNS BOOLEAN AS $$ SELECT $1 IS NOT NULL $$ + LANGUAGE SQL + SET search_path = '' + IMMUTABLE; + \"\"\") + """ end defp ash_raise_error do diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index 5f94972f..cbfbcd28 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -540,7 +540,8 @@ defmodule AshPostgres.MigrationGenerator do if tenant? do Ecto.Migrator.with_repo(repo, fn repo -> for prefix <- repo.all_tenants() do - {repo, query, opts} = Ecto.Migration.SchemaMigration.versions(repo, repo.config(), prefix) + {repo, query, opts} = + Ecto.Migration.SchemaMigration.versions(repo, repo.config(), prefix) repo.transaction(fn -> versions = repo.all(query, Keyword.put(opts, :timeout, :infinity)) diff --git a/test/aggregate_test.exs b/test/aggregate_test.exs index a0b85b53..1977866c 100644 --- a/test/aggregate_test.exs +++ b/test/aggregate_test.exs @@ -402,6 +402,34 @@ defmodule AshSql.AggregateTest do |> Ash.read_one!() end + test "aggregate with query filter using required!(field) counts only non-nil" do + post = + Post + |> Ash.Changeset.for_create(:create, %{title: "title"}) + |> Ash.create!() + + Comment + |> Ash.Changeset.for_create(:create, %{title: "match"}) + |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) + |> Ash.create!() + + Comment + |> Ash.Changeset.new() + |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) + |> Ash.create!() + + assert %{aggregates: %{custom_count_of_comments: 1}} = + Post + |> Ash.Query.filter(id == ^post.id) + |> Ash.Query.aggregate( + :custom_count_of_comments, + :count, + :comments, + query: [filter: expr(required!(title))] + ) + |> Ash.read_one!() + end + test "with data for a many_to_many, it returns the count" do post = Post diff --git a/test/calculation_test.exs b/test/calculation_test.exs index e504f868..8810b1ef 100644 --- a/test/calculation_test.exs +++ b/test/calculation_test.exs @@ -1669,4 +1669,34 @@ defmodule AshPostgres.CalculationTest do assert Ash.calculate!(post, :past_datetime1?) assert Ash.calculate!(post, :past_datetime2?) end + + describe "required!/1 in calculations" do + test "calculation using required!(field) returns true when present, false when nil" do + post = Post |> Ash.Changeset.for_create(:create, %{title: "t"}) |> Ash.create!() + + comment_with_rating = + Comment + |> Ash.Changeset.for_create(:create, %{title: "c1", rating: %{score: 5}}) + |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) + |> Ash.create!() + + comment_without_rating = + Comment + |> Ash.Changeset.for_create(:create, %{title: "c2"}) + |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) + |> Ash.create!() + + comments = + Comment + |> Ash.Query.filter(id in [^comment_with_rating.id, ^comment_without_rating.id]) + |> Ash.Query.load(:has_rating) + |> Ash.read!() + + with_rating = Enum.find(comments, &(&1.id == comment_with_rating.id)) + without_rating = Enum.find(comments, &(&1.id == comment_without_rating.id)) + + assert with_rating.has_rating == true + assert without_rating.has_rating == false + end + end end diff --git a/test/destroy_test.exs b/test/destroy_test.exs index d2892469..65e705ec 100644 --- a/test/destroy_test.exs +++ b/test/destroy_test.exs @@ -4,7 +4,7 @@ defmodule AshPostgres.DestroyTest do use AshPostgres.RepoCase, async: false - alias AshPostgres.Test.{Post, Permalink} + alias AshPostgres.Test.{Permalink, Post} test "destroy with restrict on_delete returns would leave records behind error" do post = diff --git a/test/filter_test.exs b/test/filter_test.exs index dc628e31..401fe32e 100644 --- a/test/filter_test.exs +++ b/test/filter_test.exs @@ -1240,4 +1240,244 @@ defmodule AshPostgres.FilterTest do assert fetched_org.id == organization.id end + + describe "required!/1 and ash_required/1" do + test "not is_nil(expr) compiles to IS NOT NULL in SQL (regression)" do + {query, _vars} = + Post + |> Ash.Query.filter(not is_nil(category)) + |> Ash.data_layer_query!() + |> Map.get(:query) + |> then(&AshPostgres.TestRepo.to_sql(:all, &1)) + + # SQL may be (expr) IS NOT NULL or NOT ((expr) IS NULL); both are equivalent. + assert query =~ "IS NOT NULL" or query =~ "is not null" or + (query =~ "NOT (" and query =~ "IS NULL"), + "Expected filter(not is_nil(...)) to compile to presence check (IS NOT NULL or NOT (... IS NULL)), got: #{query}" + end + + test "not is_nil(expr) filter returns only records where attribute is present (behavioral regression)" do + Post + |> Ash.Changeset.for_create(:create, %{title: "with category", category: "tech"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "no category"}) + |> Ash.create!() + + assert [%{title: "with category"}] = + Post + |> Ash.Query.filter(not is_nil(category)) + |> Ash.read!() + end + + test "not is_nil(expr) returns empty list when no records have attribute set (edge case)" do + Post + |> Ash.Changeset.for_create(:create, %{title: "a"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "b"}) + |> Ash.create!() + + assert [] = + Post + |> Ash.Query.filter(not is_nil(category)) + |> Ash.read!() + end + + test "not is_nil(expr) includes records where value is 0 or false — required means not null, not truthy (edge case)" do + post_zero = + Post + |> Ash.Changeset.for_create(:create, %{title: "zero score", score: 0}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "nil score"}) + |> Ash.create!() + + assert [%{id: id}] = + Post + |> Ash.Query.filter(title in ["zero score", "nil score"] and not is_nil(score)) + |> Ash.read!() + + assert id == post_zero.id + + post_false = + Post + |> Ash.Changeset.for_create(:create, %{title: "false public", public: false}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "nil public"}) + |> Ash.create!() + + assert [%{id: id2}] = + Post + |> Ash.Query.filter(title in ["false public", "nil public"] and not is_nil(public)) + |> Ash.read!() + + assert id2 == post_false.id + end + + test "filter with required!(attribute) returns only records where attribute is present" do + Post + |> Ash.Changeset.for_create(:create, %{title: "with category", category: "tech"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "no category"}) + |> Ash.create!() + + assert [%{title: "with category"}] = + Post + |> Ash.Query.filter(required!(category)) + |> Ash.read!() + end + + test "filter with ash_required(attribute) is equivalent to required!" do + Post + |> Ash.Changeset.for_create(:create, %{title: "with category", category: "tech"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "no category"}) + |> Ash.create!() + + assert [%{title: "with category"}] = + Post + |> Ash.Query.filter(ash_required(category)) + |> Ash.read!() + end + + test "required! on relationship id in filter (e.g. author_id)" do + author = + Author + |> Ash.Changeset.for_create(:create, %{first_name: "A", last_name: "B"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "linked", author_id: author.id}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "no author"}) + |> Ash.create!() + + assert [%{title: "linked"}] = + Post + |> Ash.Query.filter(required!(author_id)) + |> Ash.read!() + end + + test "required! in exists() returns only records where related resource has required value" do + post_with_titled_comment = + Post + |> Ash.Changeset.for_create(:create, %{title: "has comment with title"}) + |> Ash.create!() + + post_with_untitled_comment = + Post + |> Ash.Changeset.for_create(:create, %{title: "has comment without title"}) + |> Ash.create!() + + Comment + |> Ash.Changeset.for_create(:create, %{title: "has title"}) + |> Ash.Changeset.manage_relationship(:post, post_with_titled_comment, + type: :append_and_remove + ) + |> Ash.create!() + + Comment + |> Ash.Changeset.new() + |> Ash.Changeset.manage_relationship(:post, post_with_untitled_comment, + type: :append_and_remove + ) + |> Ash.create!() + + assert [%{title: "has comment with title"}] = + Post + |> Ash.Query.filter(exists(comments, required!(title))) + |> Ash.read!() + end + + test "required! on get_path (jsonb key) filters by presence of key" do + Author + |> Ash.Changeset.for_create(:create, %{ + first_name: "A", + last_name: "B", + settings: %{"dues_reminders" => ["email"]} + }) + |> Ash.create!() + + Author + |> Ash.Changeset.for_create(:create, %{first_name: "C", last_name: "D", settings: %{}}) + |> Ash.create!() + + assert [_] = + Author + |> Ash.Query.filter(required!(settings["dues_reminders"])) + |> Ash.read!() + end + + @tag :skip + test "required! on relationship/aggregate ref does not raise (e.g. popular_ratings.id)" do + # Skipped: building reference popular_ratings.id in filter not supported in this AshSql version + Comment + |> Ash.Query.filter(required!(popular_ratings.id)) + |> Ash.read!() + end + + test "required!(attr) returns empty list when no records have attribute set (edge case)" do + Post + |> Ash.Changeset.for_create(:create, %{title: "a"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "b"}) + |> Ash.create!() + + assert [] = + Post + |> Ash.Query.filter(required!(category)) + |> Ash.read!() + end + + test "required! with compound filter (and) — required!(a) and other_condition (edge case)" do + Post + |> Ash.Changeset.for_create(:create, %{title: "match", category: "tech"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "other", category: "other"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "no category"}) + |> Ash.create!() + + assert [%{title: "match"}] = + Post + |> Ash.Query.filter(required!(category) and category == "tech") + |> Ash.read!() + end + + test "required! treats 0 and false as present — not null, not truthy (edge case)" do + post_zero = + Post + |> Ash.Changeset.for_create(:create, %{title: "zero score", score: 0}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "nil score"}) + |> Ash.create!() + + assert [%{id: id}] = + Post + |> Ash.Query.filter(title in ["zero score", "nil score"] and required!(score)) + |> Ash.read!() + + assert id == post_zero.id + end + end end diff --git a/test/functions/required_error_test.exs b/test/functions/required_error_test.exs new file mode 100644 index 00000000..4102c03e --- /dev/null +++ b/test/functions/required_error_test.exs @@ -0,0 +1,27 @@ +# SPDX-FileCopyrightText: 2019 ash_postgres contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshPostgres.Functions.RequiredErrorTest do + use ExUnit.Case, async: true + + alias AshPostgres.Functions.RequiredError + + test "ash_required!/2 returns error when value is nil" do + attribute = %{name: :title, resource: MyApp.Post} + + assert {:error, %Ash.Error.Changes.Required{} = err} = + RequiredError.evaluate(%{arguments: [nil, attribute]}) + + assert err.field == :title + assert err.resource == MyApp.Post + assert err.type == :attribute + end + + test "ash_required!/2 returns value when non-nil" do + attribute = %{name: :title, resource: MyApp.Post} + + assert {:known, "hello"} = + RequiredError.evaluate(%{arguments: ["hello", attribute]}) + end +end From 29fabe5c459445f67aab4f5473b20d4f5c94abcb Mon Sep 17 00:00:00 2001 From: dallingson Date: Mon, 9 Mar 2026 12:59:12 -0600 Subject: [PATCH 2/2] Remove required!/1 and ash_required/1 expressions, SQL function, config, and tests Keep ash_required!/2 + required_error/2 and :required_error data-layer capability, and update docs accordingly --- config/config.exs | 4 +- documentation/topics/advanced/expressions.md | 44 +---- lib/expressions/ash_required.ex | 26 --- lib/expressions/required.ex | 31 ---- lib/functions/required_error.ex | 6 +- lib/migration_generator/ash_functions.ex | 18 +-- test/aggregate_test.exs | 28 ---- test/calculation_test.exs | 29 ---- test/filter_test.exs | 162 +------------------ 9 files changed, 13 insertions(+), 335 deletions(-) delete mode 100644 lib/expressions/ash_required.ex delete mode 100644 lib/expressions/required.ex diff --git a/config/config.exs b/config/config.exs index 56d38e23..45a50ba6 100644 --- a/config/config.exs +++ b/config/config.exs @@ -36,9 +36,7 @@ if Mix.env() == :test do config :ash_postgres, :ash_domains, [AshPostgres.Test.Domain] config :ash, :custom_expressions, [ - AshPostgres.Expressions.TrigramWordSimilarity, - AshPostgres.Expressions.Required, - AshPostgres.Expressions.AshRequired + AshPostgres.Expressions.TrigramWordSimilarity ] config :ash, :known_types, [AshPostgres.Timestamptz, AshPostgres.TimestamptzUsec] diff --git a/documentation/topics/advanced/expressions.md b/documentation/topics/advanced/expressions.md index 380b53bc..383a199c 100644 --- a/documentation/topics/advanced/expressions.md +++ b/documentation/topics/advanced/expressions.md @@ -82,46 +82,16 @@ For example: Ash.Query.filter(User, trigram_similarity(first_name, "fred") > 0.8) ``` -## required!/1 and ash_required/1 +## Required error (ash_required!/2 and required_error/2) -`required!/1` (and the equivalent `ash_required/1`) express that a value must be present (not nil). They are equivalent to `not is_nil(expr)`. In SQL they compile to `(expr) IS NOT NULL` or, when using the ash-functions extension, to the stored function `ash_required(expr)`. +When the data layer supports the `:required_error` capability, Ash can use `ash_required!/2` for required-attribute validation: it returns the value when present, or returns `Ash.Error.Changes.Required` when the value is nil. This avoids inline `if is_nil(value), do: {:error, ...}, else: {:ok, value}` in changesets. -**Setup:** Add AshPostgres’s custom expressions to your Ash config so the expression parser knows about them (no changes to the main Ash repo needed): +Use `required_error/2` with `Ash.Changeset.require_change/3` so that when a value is nil, Ash calls the data layer and gets the standard required error: ```elixir -# config/config.exs (or config/dev.exs, config/runtime.exs) -config :ash, :custom_expressions, [ - AshPostgres.Expressions.Required, - AshPostgres.Expressions.AshRequired -] -``` - -The **ash-functions extension** (installed via `mix ash_postgres.install_extensions` or migrations) includes an `ash_required(value)` SQL function that returns true when the value is not null. You can use it in raw SQL (e.g. fragments) as well. - -Use them in filters, calculations, aggregates, and `exists/2` when you want clearer intent than `not is_nil(...)`. - -### Examples - -```elixir -# Filter: only records where an optional attribute is set -Ash.Query.filter(Post, required!(post_category)) - -# Same using the explicit name -Ash.Query.filter(Post, ash_required(post_category)) - -# In aggregate query filters -Post -|> Ash.Query.aggregate(:count, :comments, query: [filter: expr(required!(title))]) - -# In calculations (e.g. "has value" flag) -calculate :has_rating, :boolean, expr(required!(latest_rating_score)) - -# In exists -Ash.Query.filter(Comment, exists(post, required!(id))) +change fn changeset, _context -> + Ash.Changeset.require_change(changeset, :title, &AshPostgres.Functions.RequiredError.required_error/2) +end ``` -### Semantics and SQL - -- **Semantics:** True when the argument is not nil; false when it is nil. -- **SQL:** Compiled to `(expression) IS NOT NULL` or to the extension function `ash_required(expression)` when the ash-functions extension is installed. -- **Edge cases:** Behaves like `not is_nil(expr)` over joins, nullable relationships, and in calculations. Use `required!(expr)` wherever you would use `not is_nil(expr)` for readability. +The function `AshPostgres.Functions.RequiredError.required_error/2` is provided by the data layer when `can?(:required_error)` is true (default when the ash-functions extension is installed). Ash uses it to build `expr(ash_required!(^value, ^attribute))` for required validation. diff --git a/lib/expressions/ash_required.ex b/lib/expressions/ash_required.ex deleted file mode 100644 index d5214ac5..00000000 --- a/lib/expressions/ash_required.ex +++ /dev/null @@ -1,26 +0,0 @@ -# SPDX-FileCopyrightText: 2019 ash_postgres contributors -# -# SPDX-License-Identifier: MIT - -defmodule AshPostgres.Expressions.AshRequired do - @moduledoc """ - Same as `Required` but with the explicit name `ash_required`. - - Use in filters as `ash_required(field)`. Equivalent to `required!(field)` and `not is_nil(field)`. - - Register in your config: - - config :ash, :custom_expressions, [ - AshPostgres.Expressions.Required, - AshPostgres.Expressions.AshRequired - ] - """ - use Ash.CustomExpression, - name: :ash_required, - arguments: [[:any]], - predicate?: true - - def expression(data_layer, args) do - AshPostgres.Expressions.Required.expression(data_layer, args) - end -end diff --git a/lib/expressions/required.ex b/lib/expressions/required.ex deleted file mode 100644 index 03af82f1..00000000 --- a/lib/expressions/required.ex +++ /dev/null @@ -1,31 +0,0 @@ -# SPDX-FileCopyrightText: 2019 ash_postgres contributors -# -# SPDX-License-Identifier: MIT - -defmodule AshPostgres.Expressions.Required do - @moduledoc """ - Custom expression that means "value must be present (not null)". - - Use in filters as `required!(field)` or `ash_required(field)`. - Equivalent to `not is_nil(field)`; compiles to `(expr) IS NOT NULL` in SQL. - - Register in your config so Ash knows about it: - - config :ash, :custom_expressions, [ - AshPostgres.Expressions.Required, - AshPostgres.Expressions.AshRequired - ] - """ - use Ash.CustomExpression, - name: :required!, - arguments: [[:any]], - predicate?: true - - require Ash.Expr - - def expression(AshPostgres.DataLayer, [arg]) do - {:ok, Ash.Expr.expr(not is_nil(^arg))} - end - - def expression(_data_layer, _args), do: :unknown -end diff --git a/lib/functions/required_error.ex b/lib/functions/required_error.ex index 63d00751..26a14330 100644 --- a/lib/functions/required_error.ex +++ b/lib/functions/required_error.ex @@ -4,12 +4,12 @@ defmodule AshPostgres.Functions.RequiredError do @moduledoc """ - Expression that returns the value if present, or an error if nil. - Used for required-attribute validation (Part B): `ash_required!(^value, ^attribute)`. + Function that returns the value if present, or an error if nil. + Used for required-attribute validation: `ash_required!(^value, ^attribute)`. When the data layer supports `:required_error`, Ash can build `expr(ash_required!(^value, ^attribute))` instead of the inline if/is_nil/error block. - This module is returned from the data layer's `functions/1` so the expression is available + This module is returned from the data layer's `functions/1` so the function is available when using AshPostgres. """ use Ash.Query.Function, name: :ash_required!, predicate?: false diff --git a/lib/migration_generator/ash_functions.ex b/lib/migration_generator/ash_functions.ex index f387373f..fded7252 100644 --- a/lib/migration_generator/ash_functions.ex +++ b/lib/migration_generator/ash_functions.ex @@ -48,8 +48,6 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do IMMUTABLE; \"\"\") - #{ash_required()} - execute(\"\"\" CREATE OR REPLACE FUNCTION ash_trim_whitespace(arr text[]) RETURNS text[] AS $$ @@ -161,13 +159,11 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do execute("ALTER FUNCTION ash_raise_error(jsonb) STABLE;") execute("ALTER FUNCTION ash_raise_error(jsonb, ANYCOMPATIBLE) STABLE") #{uuid_generate_v7()} - #{ash_required()} """ end def drop(4) do """ - execute("DROP FUNCTION IF EXISTS ash_required(ANYCOMPATIBLE)") execute("ALTER FUNCTION ash_raise_error(jsonb) VOLATILE;") execute("ALTER FUNCTION ash_raise_error(jsonb, ANYCOMPATIBLE) VOLATILE") """ @@ -194,19 +190,7 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do end def drop(nil) do - "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_raise_error(jsonb), ash_raise_error(jsonb, ANYCOMPATIBLE), ash_elixir_and(BOOLEAN, ANYCOMPATIBLE), ash_elixir_and(ANYCOMPATIBLE, ANYCOMPATIBLE), ash_elixir_or(ANYCOMPATIBLE, ANYCOMPATIBLE), ash_elixir_or(BOOLEAN, ANYCOMPATIBLE), ash_required(ANYCOMPATIBLE), ash_trim_whitespace(text[])\")" - end - - defp ash_required do - """ - execute(\"\"\" - CREATE OR REPLACE FUNCTION ash_required(value ANYCOMPATIBLE) - RETURNS BOOLEAN AS $$ SELECT $1 IS NOT NULL $$ - LANGUAGE SQL - SET search_path = '' - IMMUTABLE; - \"\"\") - """ + "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_raise_error(jsonb), ash_raise_error(jsonb, ANYCOMPATIBLE), ash_elixir_and(BOOLEAN, ANYCOMPATIBLE), ash_elixir_and(ANYCOMPATIBLE, ANYCOMPATIBLE), ash_elixir_or(ANYCOMPATIBLE, ANYCOMPATIBLE), ash_elixir_or(BOOLEAN, ANYCOMPATIBLE), ash_trim_whitespace(text[])\")" end defp ash_raise_error do diff --git a/test/aggregate_test.exs b/test/aggregate_test.exs index 1977866c..a0b85b53 100644 --- a/test/aggregate_test.exs +++ b/test/aggregate_test.exs @@ -402,34 +402,6 @@ defmodule AshSql.AggregateTest do |> Ash.read_one!() end - test "aggregate with query filter using required!(field) counts only non-nil" do - post = - Post - |> Ash.Changeset.for_create(:create, %{title: "title"}) - |> Ash.create!() - - Comment - |> Ash.Changeset.for_create(:create, %{title: "match"}) - |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) - |> Ash.create!() - - Comment - |> Ash.Changeset.new() - |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) - |> Ash.create!() - - assert %{aggregates: %{custom_count_of_comments: 1}} = - Post - |> Ash.Query.filter(id == ^post.id) - |> Ash.Query.aggregate( - :custom_count_of_comments, - :count, - :comments, - query: [filter: expr(required!(title))] - ) - |> Ash.read_one!() - end - test "with data for a many_to_many, it returns the count" do post = Post diff --git a/test/calculation_test.exs b/test/calculation_test.exs index 8810b1ef..74f6f896 100644 --- a/test/calculation_test.exs +++ b/test/calculation_test.exs @@ -1670,33 +1670,4 @@ defmodule AshPostgres.CalculationTest do assert Ash.calculate!(post, :past_datetime2?) end - describe "required!/1 in calculations" do - test "calculation using required!(field) returns true when present, false when nil" do - post = Post |> Ash.Changeset.for_create(:create, %{title: "t"}) |> Ash.create!() - - comment_with_rating = - Comment - |> Ash.Changeset.for_create(:create, %{title: "c1", rating: %{score: 5}}) - |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) - |> Ash.create!() - - comment_without_rating = - Comment - |> Ash.Changeset.for_create(:create, %{title: "c2"}) - |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) - |> Ash.create!() - - comments = - Comment - |> Ash.Query.filter(id in [^comment_with_rating.id, ^comment_without_rating.id]) - |> Ash.Query.load(:has_rating) - |> Ash.read!() - - with_rating = Enum.find(comments, &(&1.id == comment_with_rating.id)) - without_rating = Enum.find(comments, &(&1.id == comment_without_rating.id)) - - assert with_rating.has_rating == true - assert without_rating.has_rating == false - end - end end diff --git a/test/filter_test.exs b/test/filter_test.exs index 401fe32e..227ed0a7 100644 --- a/test/filter_test.exs +++ b/test/filter_test.exs @@ -1241,7 +1241,7 @@ defmodule AshPostgres.FilterTest do assert fetched_org.id == organization.id end - describe "required!/1 and ash_required/1" do + describe "not is_nil(expr) in filters" do test "not is_nil(expr) compiles to IS NOT NULL in SQL (regression)" do {query, _vars} = Post @@ -1319,165 +1319,5 @@ defmodule AshPostgres.FilterTest do assert id2 == post_false.id end - - test "filter with required!(attribute) returns only records where attribute is present" do - Post - |> Ash.Changeset.for_create(:create, %{title: "with category", category: "tech"}) - |> Ash.create!() - - Post - |> Ash.Changeset.for_create(:create, %{title: "no category"}) - |> Ash.create!() - - assert [%{title: "with category"}] = - Post - |> Ash.Query.filter(required!(category)) - |> Ash.read!() - end - - test "filter with ash_required(attribute) is equivalent to required!" do - Post - |> Ash.Changeset.for_create(:create, %{title: "with category", category: "tech"}) - |> Ash.create!() - - Post - |> Ash.Changeset.for_create(:create, %{title: "no category"}) - |> Ash.create!() - - assert [%{title: "with category"}] = - Post - |> Ash.Query.filter(ash_required(category)) - |> Ash.read!() - end - - test "required! on relationship id in filter (e.g. author_id)" do - author = - Author - |> Ash.Changeset.for_create(:create, %{first_name: "A", last_name: "B"}) - |> Ash.create!() - - Post - |> Ash.Changeset.for_create(:create, %{title: "linked", author_id: author.id}) - |> Ash.create!() - - Post - |> Ash.Changeset.for_create(:create, %{title: "no author"}) - |> Ash.create!() - - assert [%{title: "linked"}] = - Post - |> Ash.Query.filter(required!(author_id)) - |> Ash.read!() - end - - test "required! in exists() returns only records where related resource has required value" do - post_with_titled_comment = - Post - |> Ash.Changeset.for_create(:create, %{title: "has comment with title"}) - |> Ash.create!() - - post_with_untitled_comment = - Post - |> Ash.Changeset.for_create(:create, %{title: "has comment without title"}) - |> Ash.create!() - - Comment - |> Ash.Changeset.for_create(:create, %{title: "has title"}) - |> Ash.Changeset.manage_relationship(:post, post_with_titled_comment, - type: :append_and_remove - ) - |> Ash.create!() - - Comment - |> Ash.Changeset.new() - |> Ash.Changeset.manage_relationship(:post, post_with_untitled_comment, - type: :append_and_remove - ) - |> Ash.create!() - - assert [%{title: "has comment with title"}] = - Post - |> Ash.Query.filter(exists(comments, required!(title))) - |> Ash.read!() - end - - test "required! on get_path (jsonb key) filters by presence of key" do - Author - |> Ash.Changeset.for_create(:create, %{ - first_name: "A", - last_name: "B", - settings: %{"dues_reminders" => ["email"]} - }) - |> Ash.create!() - - Author - |> Ash.Changeset.for_create(:create, %{first_name: "C", last_name: "D", settings: %{}}) - |> Ash.create!() - - assert [_] = - Author - |> Ash.Query.filter(required!(settings["dues_reminders"])) - |> Ash.read!() - end - - @tag :skip - test "required! on relationship/aggregate ref does not raise (e.g. popular_ratings.id)" do - # Skipped: building reference popular_ratings.id in filter not supported in this AshSql version - Comment - |> Ash.Query.filter(required!(popular_ratings.id)) - |> Ash.read!() - end - - test "required!(attr) returns empty list when no records have attribute set (edge case)" do - Post - |> Ash.Changeset.for_create(:create, %{title: "a"}) - |> Ash.create!() - - Post - |> Ash.Changeset.for_create(:create, %{title: "b"}) - |> Ash.create!() - - assert [] = - Post - |> Ash.Query.filter(required!(category)) - |> Ash.read!() - end - - test "required! with compound filter (and) — required!(a) and other_condition (edge case)" do - Post - |> Ash.Changeset.for_create(:create, %{title: "match", category: "tech"}) - |> Ash.create!() - - Post - |> Ash.Changeset.for_create(:create, %{title: "other", category: "other"}) - |> Ash.create!() - - Post - |> Ash.Changeset.for_create(:create, %{title: "no category"}) - |> Ash.create!() - - assert [%{title: "match"}] = - Post - |> Ash.Query.filter(required!(category) and category == "tech") - |> Ash.read!() - end - - test "required! treats 0 and false as present — not null, not truthy (edge case)" do - post_zero = - Post - |> Ash.Changeset.for_create(:create, %{title: "zero score", score: 0}) - |> Ash.create!() - - Post - |> Ash.Changeset.for_create(:create, %{title: "nil score"}) - |> Ash.create!() - - assert [%{id: id}] = - Post - |> Ash.Query.filter(title in ["zero score", "nil score"] and required!(score)) - |> Ash.read!() - - assert id == post_zero.id - end end end