search

lpsmith / postgresql-simple

Mid-level client library for accessing PostgreSQL from Haskell
Other
206 stars 71 forks source link

Generalize the base monad of fold-like operations #195

Open bgamari opened 7 years ago

bgamari commented 7 years ago

Previously the fold-like operations were restricted to fold operations in IO, greatly limiting their usefulness. Here we generalize them to any MonadMask, provided by the widely-used exceptions library.

Resolves #9.

bgamari commented 7 years ago

Ping.

lpsmith commented 7 years ago

Ahh yes, sorry for not taking a peek at this a bit sooner.

Nothing too wrong with this patch in principle, but in practice, IIRC, the exceptions package has a problem (?) inherited from MonadCatchIO-transformers. (?)

In any case, I have finally come to the point of view that the monad-control/lifted-base approach is indeed probably better. I likely would accept this or a similiar pull request that makes the same decisions as the Snap Framework, version 1.0.

bgamari commented 7 years ago

Nothing too wrong with this patch in principle, but in practice, IIRC, the exceptions package has a problem (?) inherited from MonadCatchIO-transformers. (?)

Can you be more specific? I never quite understood what the objection to exceptions was. It's widely used, much easier to write instances for than monad-control, and fills precisely the need that postgresql-simple faces (cleanup during exception handling).

sopvop commented 7 years ago

The problem with exceptions is that MonadMask does not allow you to make instances for short-circuiting monads like ExceptT e m or Snap.

However, it is only relevant for withTransaction function in postgresq-simple, and as my experience with it shows - you would still have to provide specialized versions of withTransaction over your short-circuiting monads anyway. So that you can decide if you want to rollback or commit.

Edit: Above also applies to folds, with current api you can't exit from fold early without exceptions anyway.

And there is a valid criticism of MonadBaseControl being tricky. Writing instances for it is not that trivial either.

I think that going the exceptions route would be better for most of postgresql-simple users.

BardurArantsson commented 7 years ago

Any chance of this getting in?

I got the impression that the general problem with short-circuiting monads like ExceptT is that it's actually impossible to implement a proper bracket for such monads if you try to decompose into separate operations?

OTOH, I think I class like MonadBracket might work? Unfortunately, it seems MonadBracket hasn't actually been published as a separate library, though it seems to be included in foundation.

(FWIW, it seems to me that MonadBracket actually does the right thing at the semantic level and that it's not actually worth it trying to decompose into separate type classes.)

centromere commented 7 years ago

Hi. Is there any chance that this PR can be accepted?

BardurArantsson commented 7 years ago

Just to add to my previous comment: Even given the downsides of MonadMask, I think it would still be valuable to have this since it would mean less boilerplate if you're already using e.g. ReaderT IO ... which seems to be popular. If a better solution than MonadMask comes along later, it would still be possible to switch to that.