bitemyapp / esqueleto

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

Missing docs for SQL functions #93

Open saurabhnanda opened 6 years ago

saurabhnanda commented 6 years ago

If I'm not missing something completely obvious, the core tutorial at https://www.stackage.org/haddock/lts-10.10/esqueleto-2.5.3/Database-Esqueleto.html doesn't really teach how to use SQL functions/expressions, such as date, random, now, substr, etc.

Coupled with the fact that the modules Database.Esqueleto.SQLite and Database.Esqueleto.MySQL don't show up in the package contents, I was under the impression that Esqueleto had absolutely no way to groupBy date(timestampCol) in SQLIte. After going through https://github.com/bitemyapp/esqueleto/blob/master/src/Database/Esqueleto/PostgreSQL.hs I realise that unsafeSQLFuntion is that escape hatch.

bitemyapp commented 6 years ago

@saurabhnanda those modules are provisional workarounds, they're not intended to grow any more than necessary. I wanted a way to deal with the random()/rand() split. Anything that works in common across PostgreSQL, MySQL, and SQLite can go into the main module, only particularities or things that vary between the databases should go into the database specific modules.

Does that fit what you had in mind for them? If so, then yes, but I don't much for or about Haddocks and I don't want people landing in those modules expecting them to be complete, mostly duplicate, Haddocks when it's really just the main module you're meant to look at. So on the docs front, I don't think I'd want more than a blurb about them being db-specific-only functions and to point the user back to the main module so that people don't get turned around like you did.

Does that sound copacetic?

saurabhnanda commented 6 years ago

@saurabhnanda those modules are provisional workarounds, they're not intended to grow any more than necessary. I wanted a way to deal with the random()/rand() split. Anything that works in common across PostgreSQL, MySQL, and SQLite can go into the main module, only particularities or things that vary between the databases should go into the database specific modules.

What's the overarching idea for things like string/substring functions, date/time functions, etc? Are there standard SQL functions for this, or does every DB vendor pretty-much implement their own?

A cursory glance at the PG date/time docs and the Sqlite date/time docs makes it seem like they are pretty vendor-specific.

So, there are two ideas here:

bitemyapp commented 6 years ago

I'd be in favor of adding unsafeSqlFunction to the tutorial if it came with loud disclaimers and demonstrated the proper pattern: using unsafeSqlFunction once to define a helper, preferably one added to a fork/vendor of Esqueleto itself and later upstreamed.

Are there standard SQL functions for this

Here's a 2003 SQL BNF: https://ronsavage.github.io/SQL/sql-2003-2.bnf.html

and a PDF for one of the SQL standards: http://web.cecs.pdx.edu/~len/sql1999.pdf

Date/time/datetime types are mentioned, as is INTERVAL but it's not known to me if the standard ever specified the functions for converting a specially formatted string to a datetime object. SQL implementations for standard database servers like MySQL, PostgreSQL, Oracle, and MSSQL don't go out of their way to break the standard but I wouldn't consider the standards to be more than a hint pertaining to a small subset of the SQL people actually use.

evolving the library to support more SQL functions out-of-the-box

As long as it's safe and in the appropriate DB-specific modules I support this.

jezen commented 6 years ago

I don't think it's worth me opening a new issue for this, but I'd like to chime in by saying I also ran into the need for the unsafe functions recently, as I needed to use the earth_box and ll_to_earth functions for a georadius query.

Here's what I came up with:

whereWithinRadiusOf :: (Integral a, Show a) => a -> Location -> E.SqlQuery ()
whereWithinRadiusOf r userLocation =
  E.where_ $ earthBox origin radius @>. llToEarth latField lonField
  where
    earthBox a b  = unsafeSqlFunction "earth_box" (a, b)
    llToEarth a b = unsafeSqlFunction "ll_to_earth" (a, b)
    (@>.)         = unsafeSqlBinOp "@>"
    toVal         = unsafeSqlValue . fromText
    origin        = llToEarth originLat originLon
    originLat     = toVal . tshow $ locationLat userLocation
    originLon     = toVal . tshow $ locationLon userLocation
    radius        = toVal . tshow $ r
    latField      = toVal $ "location.lat"
    lonField      = toVal $ "location.lon"
parsonsmatt commented 5 years ago

Using unsafeSqlFunction is something that's somewhat common to do -- you need to use it for any custom-SQL functions you've defined, and for anything that the specific database supports that other databases don't.

A PR that added documentation for this purpose would be great!