valderman / selda

A type-safe, high-level SQL library for Haskell
https://selda.link
MIT License
478 stars 58 forks source link

Selda is newtype unfriendly #156

Closed TomMD closed 3 years ago

TomMD commented 3 years ago

I'm trying to figure out how to make Selda more friendly to newtypes. Take one of my real-world examples:

newtype Owner = Owner Text  deriving (lots of stuff...)

This is a pretty typical use of newtype and one that is good to preserve across the DB boundary (Rather than going to/from Text). Using Selda there is quite a bit of boilerplate to make things work.

First we need to SqlType. This can't be derived by GHC (yet) because the LCustom which is needed.

instance SqlType Owner where
    mkLit = LCustom TText . LText . coerce
    sqlType _ = TText
    fromSql (SqlString l) = coerce l
    defaultValue = mkLit ""

Second we need queries that make sense for the context:

getOwnedBy owner = do
    rows <- select theTable
    restrict (lower (rows ! owner_field) .== toLower owner)
    pure rows

Such a query assumed not one, but two functions which the newtype makes hard. The toLower function is just an unfortunate mirror problem in Haskell more generally:

toLower :: (Coercable thenewtype Text, Coercabe thenewtype Text) => thenewtype -> thenewtype
toLower = coerce . Text.toLower . coerce
-- aka toLower = coerce Text.toLower

The second, lower is the same problem but specific to Selda. Perhaps we should have something like:

-- | A somewhat unsafe function to case text fields to lower case for case insensitive comparison.
-- This has constraints which are indicative of correctness but do not actually enforce the correct behavior,
-- which depends on the SqlType instance and has no type level information available
unsafeLower :: (Coercable a Text, Coercabe a Text) => Col s a -> Col s a
unsafeLower = fun "LOWER"

I'm making this issue hoping to hear thoughts and dialog on how best to handle the problem of newtypes and boilerplate.

valderman commented 3 years ago

I don't really have a good solution from the top of my head (or I'd have implemented it already :P), but I wouldn't mind rethinking SqlType and LCustom et al from scratch if that helps. Just being able to derive SqlType instances automatically would be reason enough to break backwards compatibility here IMO.

As for the rest, perhaps it would make sense to make the equality comparison part of the SqlType class (with the current behavior as the default implementation)? If nothing else, it would make types like this nicer to use once the initial boilerplate code is written.

TomMD commented 3 years ago

I'm going to close this. A GADT seems very necessary for any sort of type safety and type propagation. Current GHC is very unable to coerce GADTs. For anyone interested in enhancing GHC in this manner please consider the below distillation:

data D a where
    coerceConstr :: D a -> D b
    makeIntConstr :: Int -> D Int
    makeBoolConstr :: Bool -> D Bool

class C a where
    doThing :: a -> D a

instance C Int where
    doThing = coerceConstr . makeIntConstr

newtype NewInt = NewInt Int deriving (Eq, C)

That last deriving will generate (-fdefer-type-errors, -ddump-deriv) something like:

instance C NewInt where
    doThing = coerce @Int doThing

Which GHC can't handle. Notice, though, that the actual AST from the instance C Int is valid for C NewInt. We can define C NewInt with doThing = coerceConstr . makeIntConstr. So while the run time representations are the same, and the code is the same, the coerce is not smart enough.