lpsmith / postgresql-simple

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

Fix fold so it can be nested #63

Closed joeyadams closed 11 years ago

joeyadams commented 11 years ago

This fixes a couple bugs in fold and company:

I almost considered releasing a Database.PostgreSQL.Simple.Cursor module (see my cursor branch). I'm not sure it's worthwhile, though, at least not in its current form.

lpsmith commented 11 years ago

Thanks!

lpsmith commented 11 years ago

Though I wholeheartedly support this change, I'm curious of a use case for nested folds. Why would you want to do that instead of say, a join?

joeyadams commented 11 years ago

It allows me to write combinators for my traversal functions:

foreachSession :: Connection -> Id Client -> (Id Session -> Session -> IO ()) -> IO ()
foreachMessage :: Connection -> Id Session -> (Id Message -> Message -> IO ()) -> IO ()

foreachSession conn clientId $ \sessionId session -> do
    putStrLn $ "session " ++ show sessionId
    foreachMessage conn sessionId $ \messageId message -> do
        putStrLn $ "  message " ++ show messageId
        ...

With a join, I wouldn't be able to split the code up like this, and I wouldn't be able to see Sessions that have no Messages without a left join and some awkward code to handle nulls.

Also, if retrieving a Session were expensive (in my case, it isn't), the join approach would mean fetching the same Session over and over, rather than just once.

lpsmith commented 11 years ago

Yes, unfortunately joins can rather substantially inflate the amount of data transferred, and this is nice clean code! This is definitely better if most sessions have many messages associated with them.

The downside is that you seem to be querying the database for each session whether or not they have any messages associated with them. (in three round trips no less!) So if most sessions only have zero or one messages, a left outer join would be more appropriate.

And I have been meaning to figure out how to improve postgresql-simple's handling of left outer joins, which I do admit can be awkward. The answer (in my mind) is that we really need a generic instance (FromRow a) => FromRow (Maybe a) that returns Nothing if all of the fields that would have otherwise been read are null, which AFAICT isn't actually possible with the current interface. Perhaps an acceptable approximation would be to return Nothing if the conversion fails without anything but UnexpectedNull values; but this isn't semantically very clean as it wouldn't return an error when some of the values are not null, and would return something when the FromRow instance would accept all nulls.

The better solution I think would be to somehow restrict the FromRow interface so that the number of columns it is expecting is knowable ahead of time. But I think this would also need to be able to account for the current instance FromField a => FromRow [a], which seems pretty useful at times.