Skip to content

Commit

Permalink
Relax type of distinctAggregate (#287)
Browse files Browse the repository at this point in the history
The `Sql DBEq a` constraint on the return type of the aggregator was wrong. It also isn't quite right to have a `EqTable i` constraint on the input type of the `Aggregator`, because technically what we want is a `Sql DBEq` constraint on whichever column(s) within `i` are used by aggregation functions, but we don't know which columns were used at this point. We could give `distinctAggregate` a type like `Sql DBEq i => Aggregator (Expr i) a` and make people run it through `lmap` manually, but that makes it impractical to use with `ListTable` without exposing more machinery. So we just drop the equality constraint for now.
  • Loading branch information
shane-circuithub committed Oct 21, 2023
1 parent d30e830 commit ad60d9e
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/Rel8/Expr/Aggregate.hs
Original file line number Diff line number Diff line change
Expand Up @@ -440,9 +440,9 @@ nonEmptyCatExprOn f = lmap f nonEmptyCatExpr


-- | 'distinctAggregate' modifies an 'Aggregator' to consider only distinct
-- values of a particular column.
distinctAggregate :: Sql DBEq a
=> Aggregator' fold i (Expr a) -> Aggregator' fold i (Expr a)
-- values of each particular column. Note that this "distinction" only happens
-- within each column individually, not across all columns simultaneously.
distinctAggregate :: Aggregator' fold i a -> Aggregator' fold i a
distinctAggregate (Aggregator fallback a) =
Aggregator fallback (Opaleye.distinctAggregator a)

Expand Down

0 comments on commit ad60d9e

Please sign in to comment.