Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow newtype-wrapped columns #38

Open
eviefp opened this issue Aug 13, 2020 · 3 comments
Open

Allow newtype-wrapped columns #38

eviefp opened this issue Aug 13, 2020 · 3 comments
Labels
add to guide Use case worth including in the guide

Comments

@eviefp
Copy link

eviefp commented Aug 13, 2020

Allowing newtype wrapped columns would be especially useful for (foreign) keys.

For example, it's easy to mistype and join on the wrong fields if they're all ints, but if we add a newtype, then this should be a lot safer. For example, I would love if this worked:

newtype PersonId = PersonId Int
derive instance personIdNewtype :: Newtype PersonId _
derive instance personEq :: Eq PersonId

people  Table
  ( id  PersonId
  , name  String
  , age  Maybe Int
  )
people = Table { name: "people" }

bankAccounts  Table
  ( id  Auto Int
  , personId  PersonId
  , balance  Default Int
  )
bankAccounts = Table { name: "bank_accounts" }

qNamesWithBalance
    s. FullQuery s { nameCol s String , balanceCol s (Maybe Int) }
qNamesWithBalance =
  selectFrom people \{ id, name, age } → do      -- FROM people
    { balance } ← leftJoin bankAccounts          -- LEFT JOIN bank_accounts
                    \acc → id .== acc.personId   -- ON people.id = bank_accounts.personId
    restrict $ id .> lit 1                       -- WHERE people.id > 1
    pure { name, balance }                       -- SELECT people.name, bank_accounts.balance

So we're wrapping the person id in PersonId Int. Right now this gives an error on selectFrom people since it's looking for an Int but finding a PersonId. Ideally, it would work on any Newtype t Int by wrapping/unwrapping the values.

Would this be possible to implement? I might have a go, given some direction.

@Kamirus
Copy link
Owner

Kamirus commented Aug 14, 2020

Hi @Vladciobanu! Thanks for your interest!

I believe what you (almost) want is already implemented on the master branch, but the problem is that we haven't yet published it.
Here is a link to the guide which explains how to deal with custom types just in case you've missed it

The error comes from the use of id .> lit 1. So changing the lit to handle Newtyped values would do the trick - but this kind of implicit dispatch does not really fit IMO (partly because of the current experimental version of Lit on the scope-as-backend branch which would not cope with it, I could elaborate on it more if want to hear more)

A simple solution is to use litPG (it will change to just lit eventually when we'll decide to commit to the changes on the scope-as-backend branch)

...
    restrict $ id .> litPG (PersonId 1)

and provide needed instances

instance toSQLValuePersonIdToSQLValue PersonId where
  toSQLValue = un PersonId >>> toSQLValue

instance fromSQLValueFromSQLValue PersonId where
  fromSQLValue = fromSQLValue >>> map PersonId

@eviefp
Copy link
Author

eviefp commented Aug 16, 2020

Thank you, that works like a charm. I think that's pretty much all I wanted. 👍

@Kamirus
Copy link
Owner

Kamirus commented Aug 17, 2020

Cool, anytime, always happy to help! ;)

I'll leave this issue open for now as I'd like to update the guide with an explicit 'newtype' example (and reference it in the basic guide)

@Kamirus Kamirus added the add to guide Use case worth including in the guide label Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to guide Use case worth including in the guide
Projects
None yet
Development

No branches or pull requests

2 participants