Kamirus / purescript-selda

A type-safe, high-level SQL library for PureScript
MIT License
89 stars 3 forks source link

Enable value-level codecs #61

Closed JordanMartinez closed 3 years ago

JordanMartinez commented 3 years ago

Fixes #60

Currently a WIP. I've opted to make PostgreSQL work first before working on SQLite3.

JordanMartinez commented 3 years ago

Ok. Pretty sure this code works, but testing it is going to be it's own major pain. I essentially need to change testWith_ to include a decodeRows argument, so that I can pass that into genericQuery when testQueryWith_ is called. However, generating the ReadForeign / FromSQLRow codecs is difficult.

JordanMartinez commented 3 years ago

I couldn't figure out how to update the tests, so that a test is defined in only one location and can tested against multiple backends. While I get why you're doing this, I think it's a better tradeoff to define tests separately for each backend.

I'd like to proceed in that direction (i.e. duplicating Common.purs for each backend and implementing each on separately). I'm going to stop working on this PR until I get some feedback from you. Otherwise, if I make such a change, you might not merge the PR.

Kamirus commented 3 years ago

I'd really want to keep Common.purs in one place, otherwise these duplicates will surely go out of sync. I'll try to figure out a solution and give you feedback about your comments as soon as I can.

JordanMartinez commented 3 years ago

Ok. I've removed the last commit that started to duplicate Common across each backend.

Kamirus commented 3 years ago

Nice work @JordanMartinez , I'm not yet ready to push this any further, but I have some insights.

primGenericQuery ∷
  ∀ s a i.
  GetCols i ⇒
  String →  -- query parameter placeholder, e.g. '$' for pg, and '?' for sqlite
  Int →     -- first query parameter index, usually `1`
            -- both of these parameters determine how we generate query
            -- parameter names, e.g. '$1', '$2', ... for pg
  (String → Array Foreign → a) →
  FullQuery s { | i } →
  a
primGenericQuery ph i exec q = do
  -- transform Query AST `q` into a query string and query parameters
  let { strQuery, params } = showM ph i $ showQuery q

  -- execute the query with the parameters
  exec strQuery params

pgPrimGenericQuery ∷
  ∀ s a i m.
  GetCols i ⇒             -- needed by query string generation
  MonadSeldaPG m ⇒        -- PG specific monad constraints
  (Array Foreign → m a) → -- generic decoder, errors are handled by monad `m`
  FullQuery s { | i } →   -- query AST
  m (Array a)             -- result rows of output records, record type `{ | o }` is determined by `i` via the type-function
pgPrimGenericQuery decodeRow = primGenericQuery "$" 1 exec
  where
  exec strQuery params = do
    conn ← ask
    errOrResult ← liftAff $ PostgreSQL.unsafeQuery conn strQuery params
    result ← either throwError pure errOrResult
    traverse decodeRow result.rows

pgGenericQuery ∷
  ∀ s o i m.
  GetCols i ⇒
  MapR UnCol_ i o ⇒             -- type-level function that maps over `{ | i }`
                                -- and removes `Col s` from each record member
                                -- thus `o` is now determined by the input `i`
  MonadSeldaPG m ⇒
  (Array Foreign → m { | o }) →
  FullQuery s { | i } →
  m (Array { | o })             -- the additional constraint forces the
                                -- appropriate result type
pgGenericQuery = pgPrimGenericQuery
JordanMartinez commented 3 years ago

@Kamirus I'm not likely to get to this anytime soon. But, perhaps those changes your mentioning should be done in a separate PR first before this one would be continued?

Kamirus commented 3 years ago

Yes, no problem, I just wanted to write these ideas here so we don't lose them. You're right - ideally in this separate PR we would only add these alternative query operations without changing the old api for now

JordanMartinez commented 3 years ago

I've opened #64 to track that idea.

JordanMartinez commented 3 years ago

I don't foresee getting back to this anytime soon, so I'm going to close this PR. I won't delete my branch though.