improvement: Add required!/1, ash_required/1, and ash_required!/2 (issue #261) (#707)

Author: dallingson

config/config.exs

@@ -35,7 +35,9 @@ 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
+  ]
 
   config :ash, :known_types, [AshPostgres.Timestamptz, AshPostgres.TimestamptzUsec]
 

documentation/topics/advanced/expressions.md

@@ -81,3 +81,4 @@ For example:
 ```elixir
 Ash.Query.filter(User, trigram_similarity(first_name, "fred") > 0.8)
 ```
+

lib/data_layer.ex

@@ -768,6 +768,12 @@ 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?() &&
+      "ash-functions" in AshPostgres.DataLayer.Info.repo(resource, :read).installed_extensions() &&
+      "ash-functions" in AshPostgres.DataLayer.Info.repo(resource, :mutate).installed_extensions()
+  end
+
   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() &&
@@ -888,6 +894,8 @@ defmodule AshPostgres.DataLayer do
       AshPostgres.Functions.Binding
     ]
 
+    functions = [Ash.Query.Function.RequiredError | functions]
+
     functions =
       if "pg_trgm" in (config[:installed_extensions] || []) do
         functions ++

lib/migration_generator/ash_functions.ex

@@ -3,7 +3,7 @@
 # SPDX-License-Identifier: MIT
 
 defmodule AshPostgres.MigrationGenerator.AshFunctions do
-  @latest_version 5
+  @latest_version 6
 
   def latest_version, do: @latest_version
 
@@ -76,6 +76,8 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do
 
     #{ash_raise_error()}
 
+    #{ash_required()}
+
     #{uuid_generate_v7()}
     """
   end
@@ -100,6 +102,8 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do
 
     #{ash_raise_error()}
 
+    #{ash_required()}
+
     #{uuid_generate_v7()}
 
     execute(\"\"\"
@@ -134,6 +138,8 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do
     """
     #{ash_raise_error()}
 
+    #{ash_required()}
+
     #{uuid_generate_v7()}
     """
   end
@@ -142,6 +148,8 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do
     """
     #{ash_raise_error()}
 
+    #{ash_required()}
+
     #{uuid_generate_v7()}
     """
   end
@@ -150,6 +158,9 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do
     """
     execute("ALTER FUNCTION ash_raise_error(jsonb) STABLE;")
     execute("ALTER FUNCTION ash_raise_error(jsonb, ANYCOMPATIBLE) STABLE")
+
+    #{ash_required()}
+
     #{uuid_generate_v7()}
     """
   end
@@ -158,39 +169,73 @@ defmodule AshPostgres.MigrationGenerator.AshFunctions do
     """
     execute("ALTER FUNCTION ash_raise_error(jsonb) STABLE;")
     execute("ALTER FUNCTION ash_raise_error(jsonb, ANYCOMPATIBLE) STABLE")
+
+    #{ash_required()}
+
     #{uuid_generate_v7()}
     """
   end
 
+  def install(5) do
+    """
+    #{ash_required()}
+    """
+  end
+
   def drop(4) do
     """
     execute("ALTER FUNCTION ash_raise_error(jsonb) VOLATILE;")
     execute("ALTER FUNCTION ash_raise_error(jsonb, ANYCOMPATIBLE) VOLATILE")
+
+    execute("DROP FUNCTION IF EXISTS ash_required(ANYCOMPATIBLE, jsonb)")
     """
   end
 
+  def drop(5) do
+    "execute(\"DROP FUNCTION IF EXISTS ash_required(ANYCOMPATIBLE, jsonb)\")"
+  end
+
   def drop(3) do
-    "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid)\")"
+    "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_required(ANYCOMPATIBLE, jsonb)\")"
   end
 
   def drop(2) do
     """
     #{ash_raise_error()}
 
-    "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid)\")"
+    "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_required(ANYCOMPATIBLE, jsonb)\")"
     """
   end
 
   def drop(1) do
-    "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_raise_error(jsonb), ash_raise_error(jsonb, ANYCOMPATIBLE)\")"
+    "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_raise_error(jsonb), ash_raise_error(jsonb, ANYCOMPATIBLE), ash_required(ANYCOMPATIBLE, jsonb)\")"
   end
 
   def drop(0) do
-    "execute(\"DROP FUNCTION IF EXISTS uuid_generate_v7(), timestamp_from_uuid_v7(uuid), ash_raise_error(jsonb), ash_raise_error(jsonb, 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_trim_whitespace(text[]), ash_required(ANYCOMPATIBLE, jsonb)\")"
   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_trim_whitespace(text[]), ash_required(ANYCOMPATIBLE, jsonb)\")"
+  end
+
+  defp ash_required do
+    """
+    execute(\"\"\"
+    CREATE OR REPLACE FUNCTION ash_required(value ANYCOMPATIBLE, payload jsonb)
+    RETURNS ANYCOMPATIBLE AS $$
+    BEGIN
+      IF value IS NULL THEN
+        RETURN ash_raise_error(payload, value);
+      END IF;
+
+      RETURN value;
+    END;
+    $$ LANGUAGE plpgsql
+    STABLE
+    SET search_path = '';
+    \"\"\")
+    """
   end
 
   defp ash_raise_error do

lib/sql_implementation.ex

@@ -229,6 +229,54 @@ defmodule AshPostgres.SqlImplementation do
     end
   end
 
+  def expr(
+        query,
+        %{name: :required!, arguments: [value_expr, attribute]} = required,
+        bindings,
+        embedded?,
+        acc,
+        type
+      ) do
+    pred_embedded? = Map.get(required, :embedded?, false)
+
+    {value_dyn, acc} =
+      AshSql.Expr.dynamic_expr(
+        query,
+        value_expr,
+        bindings,
+        pred_embedded? || embedded?,
+        type,
+        acc
+      )
+
+    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!")
+
+    payload =
+      %{
+        exception: inspect(Ash.Error.Changes.Required),
+        input: %{field: field, type: :attribute, resource: resource}
+      }
+      |> Jason.encode!()
+
+    {:ok,
+     Ecto.Query.dynamic(
+       fragment(
+         "CASE WHEN ? IS NULL THEN ash_raise_error(?::jsonb, ?) ELSE ? END",
+         ^value_dyn,
+         ^payload,
+         ^value_dyn,
+         ^value_dyn
+       )
+     ), acc}
+  end
+
   def expr(
         _query,
         _expr,

mix.exs

@@ -186,8 +186,8 @@ defmodule AshPostgres.MixProject do
   # Run "mix help deps" to learn about dependencies.
   defp deps do
     [
-      {:ash, ash_version("~> 3.19")},
-      {:spark, ">= 2.3.4"},
+      {:ash, ash_version("~> 3.21")},
+      {:spark, "~> 2.3 and >= 2.3.4"},
       {:ash_sql, ash_sql_version("~> 0.4 and >= 0.4.3")},
       {:igniter, "~> 0.6 and >= 0.6.29", optional: true},
       {:ecto_sql, "~> 3.13"},

priv/resource_snapshots/dev_test_repo/extensions.json

@@ -1,5 +1,5 @@
 {
-  "ash_functions_version": 5,
+  "ash_functions_version": 6,
   "installed": [
     "ash-functions",
     "uuid-ossp",

priv/resource_snapshots/test_no_sandbox_repo/extensions.json

@@ -1,5 +1,5 @@
 {
-  "ash_functions_version": 5,
+  "ash_functions_version": 6,
   "installed": [
     "ash-functions",
     "uuid-ossp",

priv/resource_snapshots/test_repo/extensions.json

@@ -1,5 +1,5 @@
 {
-  "ash_functions_version": 5,
+  "ash_functions_version": 6,
   "installed": [
     "ash-functions",
     "uuid-ossp",

test/filter_test.exs

@@ -1243,4 +1243,84 @@ defmodule AshPostgres.FilterTest do
 
     assert fetched_org.id == organization.id
   end
+
+  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
+        |> 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
+  end
 end