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

MonadBaseControl IO Session #144

Closed
andremarianiello opened this issue Dec 28, 2021 · 15 comments
Closed

MonadBaseControl IO Session #144

andremarianiello opened this issue Dec 28, 2021 · 15 comments

Comments

@andremarianiello
Copy link

Session could be given a MonadBaseControl instance (and I would be happy to do it), but since it used to have one and no longer does I assume it is intentionally missing. I tried finding information on why it was removed, but to no avail. Since I couldn't find any info, and because such an instance would be useful to me, I thought I would file an issue to ask: should Session have a MonadBaseControl instance, and if not, why?

@nikita-volkov
Copy link
Owner

The "monad-control" hierarchy has a controversial reputation in the community.

Instead of implanting it in the "hasql" lib, it can be provided as an orphan instance. You can have it in your codebase or release it as one of the extension libraries.

@andremarianiello
Copy link
Author

Can an orphan instance actually be provided? I thought it would not be possible since the constructor for Session is not exported.

@nikita-volkov
Copy link
Owner

It's safe to execute run multiple times and that is essentially what you need for such instance.

Now, in many words. Session is defined as

newtype Session a =
  Session (ReaderT Connection (ExceptT QueryError IO) a)

So run is simply a complete unpacking of it and of the layers of transformers inside it. All the state is maintained in the Connection, which is provided by the caller.

@andremarianiello
Copy link
Author

run let's me unwrap Session, but without the Session constructor I don't have a way to wrap it back up. Unless I'm missing something...

@andremarianiello
Copy link
Author

This is what I thought the instance would look like

newtype MySess a = MySess (ReaderT Connection (ExceptT QueryError IO) a)
  deriving newtype (Functor, Applicative, Monad, MonadIO)

runMySess :: MySess a -> Connection -> IO (Either QueryError a)
runMySess (MySess impl) connection = runExceptT $ runReaderT impl connection

instance MonadBase IO MySess where
  liftBase = liftIO

instance MonadBaseControl IO MySess where
  type StM MySess a = Either QueryError a
  liftBaseWith runInIO = MySess $ ReaderT $ \conn -> lift $ runInIO $ flip runMySess conn
  restoreM = MySess . lift . ExceptT . pure

For Hasql.Session I would ofc use run in place of runMySess, but I don't have anything to use in place of the MySess constructor.

@nikita-volkov
Copy link
Owner

All you need now is the following function:

liftSession :: Session a -> MySess a

Which is trivial to implement in terms of run.

Yeah. Not exactly the orphan instance implementation, but should work.

@andremarianiello
Copy link
Author

andremarianiello commented Dec 30, 2021

So your recommendation is to use my own session abstraction for this instead of the one provided by hasql? If so, would you feel differently about a MonadUnliftIO instance?

@nikita-volkov
Copy link
Owner

If you happen to need these abstractions it's likely that you have some monad transformer stack there. You can just add ReaderT Connection (ExceptT QueryError IO) inside of it and call into run.

I'm failing to see the real need for binding "hasql" with any one of those abstractions. I'm open for discussion, of course, but I must say that I'm quite skeptical on the matter.

@andremarianiello
Copy link
Author

Perhaps it would help I describe the use-case a bit, then. I was looking into adding support for streaming results to rel8, something like https://hackage.haskell.org/package/rel8-1.2.1.0/docs/Rel8.html#g:38 but returning an effectful stream instead of a list. It would be effectful because it would stream results from the db using a cursor to avoid materializing all the rows in memory at once. (The streaming library I am currently working with is streamly, but I plan on attempting this with others as well).

Following this idea, I found hasql-cursor-query but realized that building a stream with a CursorQuery would require loading all the results into memory before the stream can be consumed. From there I went further down and looked at hasql-cursor-transaction. I thought I could use this until I learned that it was designed analogously to hasql-transaction, i.e. to support retryable transactions (but with cursors). I don't want retry semantics a la STM, I want to be able to interleave IO actions between calls to the cursor. This means the stream monad needs to be a MonadIO, so naturally Hasql.Session is the choice for that.

So now I want to write a function which will take a query and returns a stream of values produced by Session. This is doable, and would be a pretty nice API I think, because it would allow nice interop with the rest of the hasql ecosystem. However, I want the cursor to be a managed resource that is closed when no longer needed by the stream. This is the classic bracket pattern, and takes us into MonadMask/MonadBaseControl/MonadUnliftIO territory. Trying to implement these classes correctly for an opaque type life Session is, AFAICT, impossible (without unsafeCoerce).

And that I think should catch you up to where I am in this exercise. If I took a wrong turn somewhere, please let me know. And just to concretely state what my current goal is, I wish to produce a resource-safe SerialT Session a (SerialT is a type of stream from streamly) from a query.

@nikita-volkov
Copy link
Owner

Interleaving IO with streaming from cursor is something best avoided, because that requires keeping a transaction open for the duration. Transactions better be as short as possible, because they come with overhead. Locks and etc.

But if you still need that, why not SerialT (ExceptT QueryError IO)? No need for the mentioned typeclasses at all.

@andremarianiello
Copy link
Author

andremarianiello commented Dec 30, 2021

I believe that using Session will provide a better user experience when interoperating with other parts of the hasql ecosystem because Session is one of the central abstractions provided by hasql. The question "Why not use X instead of Session could be asked of all of the hasql ecosystem, could it not? We could replace Session with ReaderT Connection (ExceptT QueryError IO) everywhere and everything would still work, but we don't because it would make the hasql libraries harder to understand and use. I just want the hypothetical user of this hasql streams library to enjoy those benefits as well, without feeling like the pieces barely fit together.

EDIT: FWIW thank you for taking the time to discuss this.

@nikita-volkov
Copy link
Owner

nikita-volkov commented Dec 31, 2021

I agree. How about if I add a function like

accessConnection :: Session Connection

In that case in combination with throwError you'll have everything needed for interleaving and for the instances that you wanted

@andremarianiello
Copy link
Author

andremarianiello commented Dec 31, 2021

Yup, accessConnection gives me enough power to witness the isomorphism between Session and ReaderT Connection (ExceptT QueryError IO), which allows me to write the necessary instances. Great idea! That gives me a another idea, though. How would you feel about providing a full-blown MonadReader Connection instance for Session? It would make sense for it to be provided in tandem with the existing MonadError QueryError instance.

@nikita-volkov
Copy link
Owner

Released in 1.5

@andremarianiello
Copy link
Author

andremarianiello commented Jan 2, 2022

Thanks so much! In case anyone shows up to this issue looking for a MonadBaseControl instance, here it is (and others), in case I don't end up putting them in a library.

-- pseudo-newtype coercion helpers

wrapSession :: ReaderT Connection (ExceptT QueryError IO) a -> Session a
wrapSession m = do
  conn <- ask
  res <- liftIO $ runExceptT $ runReaderT m conn
  either throwError pure res

unwrapSession :: Session a -> ReaderT Connection (ExceptT QueryError IO) a
unwrapSession m = ReaderT $ \conn -> ExceptT $ run m conn

-- monad-control

instance MonadBase IO Session where
  liftBase = liftIO

instance MonadBaseControl IO Session where
  type StM Session a = StM (ReaderT Connection (ExceptT QueryError IO)) a
  liftBaseWith runInIO = wrapSession $ liftBaseWith $ \m -> runInIO (m . unwrapSession)
  restoreM = wrapSession . restoreM
 
-- exceptions

instance MonadThrow Session where
  throwM = wrapSession . throwM

instance MonadCatch Session where
  catch action handler = wrapSession $ catch (unwrapSession action) (unwrapSession . handler)

instance MonadMask Session where
  mask f = wrapSession $ mask $ \u -> unwrapSession $ f (wrapSession . u . unwrapSession)
  uninterruptibleMask f = wrapSession $ uninterruptibleMask $ \u -> unwrapSession $ f (wrapSession . u . unwrapSession)
  generalBracket acquire release use = wrapSession $ generalBracket (unwrapSession acquire) ((unwrapSession .) . release) (unwrapSession . use)
  
-- unliftio. 
-- didn't use the wrapper helpers here because there is no generally-accepted ExceptT instance for MonadUnliftIO. instead I turn MonadError errors into exceptions which are then turned back into errors (see https://github.com/fpco/unliftio/issues/68 for details)
instance MonadUnliftIO Session where
  withRunInIO inner = do
    conn <- ask
    res <- liftIO $ try $ inner $ \sess -> do
      run sess conn >>= either throwIO pure
    case res of
      Left e -> throwError e
      Right a -> pure a
      

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants