fix: properly handle limit/offset for aggregates

lib/ash/actions/aggregate.ex

@@ -5,8 +5,6 @@ defmodule Ash.Actions.Aggregate do
   def run(api, query, aggregates, opts) do
     query = %{query | api: api}
     {query, opts} = Ash.Actions.Helpers.add_process_context(query.api, query, opts)
-    action = query.action || Ash.Resource.Info.primary_action!(query.resource, :read)
-    opts = Keyword.put_new(opts, :read_action, action.name)
 
     with %{valid?: true} = query <- Ash.Actions.Read.handle_attribute_multitenancy(query) do
       aggregates
@@ -15,19 +13,24 @@ defmodule Ash.Actions.Aggregate do
           agg_authorize? = aggregate.authorize? && opts[:authorize?]
 
           read_action =
-            aggregate.read_action || query.action ||
+            aggregate.read_action || (query.action && query.action.name) ||
               Ash.Resource.Info.primary_action!(query.resource, :read).name
 
           {agg_authorize?, read_action}
 
         {_name, _kind} ->
-          {!!opts[:authorize?], opts[:read_action]}
+          {!!opts[:authorize?],
+           opts[:read_action] || opts[:action] || (query.action && query.action.name) ||
+             Ash.Resource.Info.primary_action!(query.resource, :read).name}
 
         {_name, _kind, agg_opts} ->
           authorize? =
             Keyword.get(agg_opts, :authorize?, true) && opts[:authorize?]
 
-          {authorize?, agg_opts[:read_action] || opts[:read_action] || action.name}
+          {authorize?,
+           agg_opts[:read_action] || opts[:read_action] || agg_opts[:action] || opts[:action] ||
+             (query.action && query.action.name) ||
+             Ash.Resource.Info.primary_action!(query.resource, :read).name}
       end)
       |> Enum.reduce_while({:ok, %{}}, fn
         {{agg_authorize?, read_action}, aggregates}, {:ok, acc} ->
@@ -54,7 +57,7 @@ defmodule Ash.Actions.Aggregate do
               aggregates: List.wrap(aggregates),
               actor: opts[:actor],
               tenant: opts[:tenant],
-              action: action.name,
+              action: read_action,
               authorize?: opts[:authorize?]
             }
 
@@ -65,7 +68,11 @@ defmodule Ash.Actions.Aggregate do
               with {:ok, query} <- authorize_query(query, opts, agg_authorize?),
                    {:ok, aggregates} <- validate_aggregates(query, aggregates, opts),
                    {:ok, data_layer_query} <-
-                     Ash.Query.data_layer_query(%Ash.Query{resource: query.resource}),
+                     Ash.Query.data_layer_query(%Ash.Query{
+                       resource: query.resource,
+                       limit: query.limit,
+                       offset: query.offset
+                     }),
                    {:ok, result} <-
                      Ash.DataLayer.run_aggregate_query(
                        data_layer_query,
@@ -88,16 +95,10 @@ defmodule Ash.Actions.Aggregate do
     |> Ash.Query.do_filter(right.filter)
     |> Ash.Query.sort(right.sort, prepend?: true)
     |> Ash.Query.distinct_sort(right.distinct_sort, prepend?: true)
-    |> Ash.Query.limit(right.limit)
     |> Ash.Query.set_tenant(right.tenant)
-    |> merge_offset(right.offset)
     |> Ash.Query.set_context(right.context)
   end
 
-  defp merge_offset(query, offset) do
-    Ash.Query.offset(query, query.offset + (offset || 0))
-  end
-
   defp authorize_query(query, opts, agg_authorize?) do
     if agg_authorize? do
       case query.api.can(query, opts[:actor],
@@ -155,6 +156,7 @@ defmodule Ash.Actions.Aggregate do
   end
 
   defp set_opts(query, specified, others) do
+    query = Ash.Query.unset(query, [:limit, :offset])
     {agg_opts, _} = Ash.Query.Aggregate.split_aggregate_opts(others)
 
     agg_opts = Keyword.merge(agg_opts, specified)

lib/ash/query/aggregate.ex

@@ -384,17 +384,35 @@ defmodule Ash.Query.Aggregate do
   defp build_query(_resource, nil), do: {:ok, nil}
 
   defp build_query(resource, build_opts) when is_list(build_opts) do
-    case Ash.Query.build(resource, build_opts) do
-      %{valid?: true} = query ->
-        build_query(resource, query)
+    cond do
+      build_opts[:limit] ->
+        {:error, "Cannot set limit on aggregate query"}
 
-      %{valid?: false} = query ->
-        {:error, query.errors}
+      build_opts[:offset] && build_opts[:offset] != 0 ->
+        {:error, "Cannot set offset on aggregate query"}
+
+      true ->
+        case Ash.Query.build(resource, build_opts) do
+          %{valid?: true} = query ->
+            build_query(resource, query)
+
+          %{valid?: false} = query ->
+            {:error, query.errors}
+        end
     end
   end
 
   defp build_query(_resource, %Ash.Query{} = query) do
-    {:ok, Ash.Query.unset(query, [:load, :limit, :offset])}
+    cond do
+      query.limit ->
+        {:error, "Cannot set limit on aggregate query"}
+
+      query.offset && query.offset != 0 ->
+        {:error, "Cannot set offset on aggregate query"}
+
+      true ->
+        {:ok, Ash.Query.unset(query, [:load, :limit, :offset])}
+    end
   end
 
   @doc false

test/actions/aggregate_test.exs

@@ -195,6 +195,8 @@ defmodule Ash.Test.Actions.AggregateTest do
       assert %{count: 0} = Api.aggregate!(Post, {:count, :count}, authorize?: true)
       assert 0 = Api.count!(Post, authorize?: true)
 
+      Application.put_env(:foo, :bar, true)
+
       assert %{count: 1} =
                Post
                |> Ash.Query.for_read(:unpublic)