circuithub / rel8

Hey! Hey! Can u rel8?
https://rel8.readthedocs.io
Other
150 stars 38 forks source link

Unify `function` and `nullaryFunction` #258

Closed shane-circuithub closed 12 months ago

shane-circuithub commented 12 months ago

This does away with the weird variadic arguments thing we had going on with function.

Functions with no arguments are now written as:

now :: Expr UTCTime
now = function "now" ()

Functions with multiple arguments are now written as:

quot :: Sql DBIntegral a => Expr a -> Expr a -> Expr a
quot n d = function "div" (n, d)

Single-argument functions are written exactly as before.

shane-circuithub commented 12 months ago

@ocharles I know you're ambivalent about this but I figured I'd make a proper PR regardless. It's based on #257.

shane-circuithub commented 12 months ago

I knew you wouldn't like Arguments, but I really don't think it's too onerous to "understand". I think if tuples are used for multiple arguments it's pretty natural to expect () to work for zero arguments. I'm also thinking about queryFunction here, which I will rebase on top of this. Without Arguments we will need a nullaryQueryFunction too. Arguments, function and queryFunction is less "things" (3) than function, nullaryFunction, queryFunction and nullaryQueryFunction (4).

shane-circuithub commented 12 months ago

And yeah, it's not really possible to make () a Table, for two reasons. The first one is, as you said, what Context is it in? If we want to make it work with function as above, then could arbitrarily assign it the Expr context, but the deeper problem is that it doesn't contain any Exprs. Columns has to be a HTable, and a t Expr (where t is a HTable) must contain actual Exprs, because HTable's htraverse method has an Apply constraint (not Applicative), so a HTable can't be empty.

ocharles commented 12 months ago

And yeah, it's not really possible to make () a Table, for two reasons. The first one is, as you said, what Context is it in?

My thinking was actually that () is in all contexts, but that's not possible because of the fun dep on Table (a -> context), so that's a completely non-starter.

I knew you wouldn't like Arguments, but I really don't think it's too onerous to "understand".

I just don't like type classes for this kind of overloading because it's tedious to interactively understand. If you're using holes, a _ at the point of arguments will give you a useless hole ("I need an a"), and if you're in Haddock you still have to do some left-to-right parsing to get to the argument, then back to the left to find the constraint, then find the type class, then hunt down instances for it. The openness of a type class just throws everything around everywhere. Of course it's not a lot to understand and the names are intuitive, but I still think it's asking more than zero from a new user (we will of course never get to zero, but I want to get as close to it as possible)

I'm also thinking about https://github.com/circuithub/rel8/pull/241 here, which I will rebase on top of this.

Thanks, of course I wasn't quite aware of that based on what was in this PR, but that does explode the API more (though it at least still has some symmetry).


I'll present just one more alternative, which is again to ditch the type class, but to replace with a simple sum-type:

function :: Sql DBType a => QualifiedName -> Arguments a -> Expr b

data Arguments a where
  NoArguments :: Arguments ()
  Arguments :: Table Expr a => a -> Arguments a

(I wouldn't go further than this. E.g., I wouldn't go towards a fullblown HList or anything).

It's a tiny bit more work to use:

rem :: Sql DBIntegral a => Expr a -> Expr a -> Expr a
rem n d = function "rem" $ Argument (n, d)

now :: Expr UTCTime
now = function "now" NoArguments

But I think it's absolutely obvious what's going on.

With this I think we've exhausted all options:

  1. Variadic arguments (status quo)
  2. Type class overloading
  3. Separate nullary/n-ary functions
  4. A GADT to choose between nullary/n-ary.

My vote is the GADT as everything becomes very self-contained, but I'll leave it with you to pick which you think is best as my +1 is fairly weak. I think any of 2/3/4 improve the current situation.

shane-circuithub commented 12 months ago

I prefer 2 to 3 or 4.

In PostgreSQL, a function is just a function, whether it takes zero or more arguments. A nullaryFunction is not a separate type of thing to a function, they're all just functions. The only reason we had nullaryFunction as a separate thing was that the implementation of variadic arguments that function used didn't allow a way to supply zero arguments. I don't think it's a meaningful distinction or one we should seek to preserve.

As for the GADT approach, I understand your point about typed holes, but it feels pretty weak here. There's not much that GHC can tell you about what argument to supply to function anyway, because it depends on what Postgres function is being called and GHC doesn't know anything about that. In any case, function "foo" _ in GHCi results in two different errors, but one of them is the following:

ghci>  Rel8.function "foo" _

<interactive>:3:2: error:
    • Overlapping instances for Arguments arguments0
        arising from a use of ‘function’
      Matching instance:
        instance Table Expr a => Arguments a
          -- Defined at src/Rel8/Expr/Function.hs:43:10
      Potentially matching instance:
        instance [overlap ok] Arguments ()
          -- Defined at src/Rel8/Expr/Function.hs:47:27
      (The choice depends on the instantiation of ‘arguments0’
       To pick the first instance above, use IncoherentInstances
       when compiling the other instance declarations)
    • In the expression: function "foo" _
      In an equation for ‘it’: it = function "foo" _

Maybe it's a little bit confusing and indirect but GHC is basically saying "you can either put a Table Expr here or ()", which is about as much as it can possibly say. But again, I don't think typed holes are ever going to be very useful here due to the nature of function being deliberately maximally polymorphic.

Really though, I just have really hard time believing that even you would rather type function "now" NoArguments and function "div" (Arguments (n, d)) than function "now" () or function "div" (n, d). Maybe it's slightly easier for someone using typed holes to discover what to do with function, but not by much I don't think. But even then, you only discover how to use function once, but you use it many times. Having to import and type NoArguments or Arguments every time thereafter feels like an unreasonable punishment on anyone that already knows how to use function.

ocharles commented 12 months ago

Ah, I didn't remember that GHC told us about potential instances. That does weaken my argument about typed holes.