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

Add Eq, Show instances, use safe foreign imports, and add utility module #2

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

joeyadams
Copy link

Note: This is my first time doing a pull request.

These commits do essentially three things:

  • Add Eq, Show instances for enumeration types

  • Use safe for all foreign imports. Safe calls are more expensive, but not terribly so. With -threaded, the foreign call overhead is increased by a factor of almost two. If I'm not mistaken, a safe foreign call does not require spawning a new thread; it just needs to hold the current OS thread hostage until the foreign call is complete.

    Some foreign imports definitely need to be marked safe. For example, if the connection to the server hangs, calls such as PQexec and PQcancel can cause the entire RTS to hang if they are marked unsafe. Other foreign imports appear trivial, but do sneaky stuff. PQgetvalue should be a simple array access, but if the row and column are out of bounds, it will invoke error handling code.

    It's better to use safe by default, and only use unsafe when there is clear evidence of a performance bottleneck.

  • Added Database.PQ.Utils, a utility module that provides exception-based error handling and convenient conversion to/from SQL values. I suppose this functionality could be provided in a separate package (e.g. libpq-extra), but I don't think it adds too much in the way of dependencies.

The first three commits are tweaks that I would really like to be applied. The last three commits introduce a utility module that could just as well be a package of its own.

I did not change the version number. However, per the Hackage version number policy, I think A.B needs to be incremented because of the new instances. I suppose the new version would be 0.5.0 .

@joeyadams
Copy link
Author

I am actively developing Database.PQ.Utils for an application I am working on, and plan to add more features (such as a Record class, and a convenient way to listen for database notifications). It should probably be a separate module, as the API will be less stable than that of haskell-libpq. Thus, I would suggest only applying the first three commits.

I'm not sure Utils is a good name. What I'm writing is more of a high-level frontend to Database.PQ. It provides error handling and convenient conversion, but it still uses direct query strings.

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

Successfully merging this pull request may close these issues.

1 participant