bitemyapp / esqueleto

New home of Esqueleto, please file issues so we can get things caught up!
BSD 3-Clause "New" or "Revised" License
376 stars 108 forks source link

Arithmetic operators don't represent SQL overloading #134

Open mossprescott opened 5 years ago

mossprescott commented 5 years ago

In SQL (Postgres, at least), arithmetic operators such +, -, *, and / are overloaded for date/time types. for example, the expression timestamp '2001-09-29 03:00' - timestamp '2001-09-27 12:00 results in interval '1 day 15:00:00' (see https://www.postgresql.org/docs/9.2/interactive/functions-datetime.html).

However, Esqueleto defines (-.) :: PersistField a => SqlExpr (Value a) -> SqlExpr (Value a) -> SqlExpr (Value a), which will generate the SQL needed to subtract timestamps, but gives the result the wrong type. Letting these operators be applied to just any type is incorrect anyway (e.g. you can't subtract strings in SQL), and forcing the argument and result types to all be the same is both unsound and prevents valid queries from being written.

I'm not sure how this mess would best be represented in Esqueleto, but perhaps a set of additional operators, including one for (TimeLike a, TimeLike b, IntervalLike c) => SqlExpr (Value a) -> SqlExpr (Value b) -> SqlExpr (Value c) amongst several others. I'm not sure how many types would inhabit TimeLike, IntervalLike, etc. This isn't sounding super promising.

Also open to gross hacks that let me do the unsafe cast or something.

parsonsmatt commented 5 years ago

You can definitely work around this with, eg,

veryUnsafeCoerceSqlExpr :: SqlExpr (Value a) -> SqlExpr (Value b)

veryUnsafeCoerceSqlExpr (time1 -. time2) :: SqlExpr (Value Interval)

:sweat_smile:

mossprescott commented 5 years ago

I'm intrigued, but that doesn't compile. There's veryUnsafeCoerceSqlExprValue but it's not exported. Can I write my own?

parsonsmatt commented 5 years ago

That should be exported, if from an internal module - I don't want anything hidden in this package.

mossprescott commented 5 years ago

I tried the other thing that should have been obvious, and it compiles:

    subtractTimestamps :: SqlExpr (Value UTCTime) -> SqlExpr (Value UTCTime) -> SqlExpr (Value NominalDiffTime)
    subtractTimestamps = unsafeSqlBinOp "-"

Presumably this will work for me for now. If I find out otherwise I'll update here.

Vlix commented 5 years ago

The - operator is used for tons of types in PostgreSQL. All sorts of time types, geometric types, json, network addresses, etc. These are not SQL-wide, but very PostgreSQL specific, so it'd make more sense for the (-) :: Num a => SqlExpr (Value a) -> SqlExpr (Value a) -> SqlExpr (Value a) to be the standard one, and any specific implementation that has more, can be put in seperate modules.