Todo list for 0.3 #41

Closed
lpsmith opened this Issue Sep 26, 2012 · 10 comments

2 participants

@lpsmith
Owner
  • pull in sopvop's enhancements to the SqlError type
  • pull in jochu's enhancements to the ResultError type
  • pull in the work regarding array support, courtesy of Jason Dusek, Bas van Dijk, and Joey Adams
  • evaluate mightybyte's alternative interface
  • add support for range types
  • overhaul/deprecate the BuiltinTypes module

Am I missing anything?

@lpsmith
Owner

Oh, and fill in some missing documentation for Database.PostgreSQL.Simple.FromField, issue #38

@wiz

"hstore" type support?

@lpsmith
Owner

@wiz, perhaps. A pull request would make this more likely.

@lpsmith
Owner

Ok, I've created an 0.3 branch and pulled in all the 0.3-related pull requests into it.

@wiz

@lpsmith, while working out my hstore columns I've accidentaly made a generic attoparsec-parseable field typeclass: https://gist.github.com/3972626

If someone's interested i can wrap that into a proper pull request with tests/docs.

@lpsmith
Owner

@wiz, I think your gist would be better as a higher order helper function, something like

convertAttoparsec :: BS.ByteString -> Maybe a -> Parser a -> FromField a
convertAttoparsec typename nullValue parser = ...

Then you can write:

instance FromField HStore where
    fromField = convertAttoparsec "hstore" (Just mempty) hstoreParser

Though I really question the merit of turning nulls into empty maps; PostgreSQL does distinguish nulls from empty hstores, and no other FromField conversion provided by postgresql-simple does this.

How robust is your hstore Parser? Are there values that it fails to parse correctly? I am interested in that definition.

@lpsmith
Owner

Updating the TODO list, (and moving the goalposts a bit), here are things that I currently think should be done before release:

  1. Code reorganization: move Simple and Simple.Internal into a Simple.Implementation mega-module, reexport the relevant bits of code from Implementation from Simple and Simple.Internal. This is desperately needed for a variety of reasons

  2. Move many of the transaction-related functions into a new Simple.Transaction module, and remove most of them from Simple. This depends on the reorganization.

  3. Change the FromField and FromRow classes to allow for IO, so that typenames (and other potentially interesting meta-information, such as the table name of the associated column) don't have to be pre-computed. I think the result type needs to change from Ok a to an (abstract) ReaderT DB.Connection IO (Ok a)

  4. Add in a serialization error predicate to the Errors module; re-export it from the new Transaction module

  5. Overhaul the TypeInfo system, and tie the seperator character from the Array module into it.

Here are things I would like to get done, and would prefer not to release without:

  1. Write documentation for the FromField module, including some sample code.

Here are things that would be nice to get done, but I would be happy releasing without:

  1. Add support for range types

  2. Add support for hstore types

@lpsmith
Owner

Regarding the code reorganization; I took another crack at it but ended up stuck, again. Perhaps writing some hs-boot files would be the way to go here.

It seems that some of the conceptual namespaces I'd like to have and some of the implementations thereof are inherently mutually recursive: for example I'd like a transaction module that has all the specialized transaction-related stuff, and reexport one or a few of the most commonly used stuff from Simple, such as withTransaction. Now, these are currently defined in terms of e.g. execute_, which is defined in the Simple module.

This particular case isn't difficult to work around, however: as Simple doesn't use withTransaction, we can simply move those functions from a new Implementation module to a new Transaction module and then have the Simple module import both and export what it wants.

Another complication is that Simple depends on FromField, which both depend on Internal. Thus collapsing Simple and Internal into one module introduces circular dependencies between the new module and FromField. (This paragraph also is true of FromRow.)

The naive approach needs to also bring the FromField and FromRow modules into the new mega-module. I'm reluctant to do that as those are two modules that people tend to study the source of, moreso than Simple and Simple.Internal. I really wouldn't want to make those parts of the source code harder to navigate.

Thus this becomes a rather more difficult situation: I could spend a fair bit of time trying to tease these apart into a selection of Implementation modules, and possibly reduce the interdependencies by not using FromField and FromRow to query the metaschema for example. But this seem a pointless duplication of code, not to mention that this problem will get worse once I get (3) done in my preceding comment. Thus I think it's likely that hs-boot files are the better solution here. I'm not sure if I should write hs-boot files for the main module or some of the smaller modules.

@lpsmith
Owner

Ok, 1, 2, and 3 of the essentials list is now done, as well as fixing up the FromField documentation. So 0.3 is a lot closer to reality now.

I still need to add a serialization error predicate to the transaction and errors module and some other minor cleanups, and overhaul the typeinfo system. The latter is the biggest task, though I've already have a start on it from 6 months ago and completing it seems a lot easier now that I have the mutual recursion between the FromField/FromRow/TypeInfo/Simple modules. I can't say I'm extremely pleased with the current organization, but I suppose it will do for now.

Regarding the typeinfo overhaul, there are a number of new design options now that I'm allowing for IO inside the fromField and fromRow methods. And perhaps it wouldn't be all bad to go with a more "logical" design instead of mimicking the pg_type table. I think I'm going to make the static part of the table semi-internal; I'm not sure if I'll have backwards compatibility with the existing BuiltinTypes module.

@lpsmith
Owner

Oh, a couple small details:

  1. Need to hook the existing pg array parser up to the new typeinfo table, especially the array separator character.

  2. It would be nice to add way to turn the tableOid into a tableName, and adjust the ResultError type accordingly. (Should we cache the tableNames, or just re-query every time? I'm inclined to requery for now, until (and only if) it becomes clear that we should cache instead.)

  3. In my rush to allow for IO inside of FromField methods and rip out the typename pre-computation, I think I messed up two things: the array typechecking and the timestamp typename error reporting. Actually, I am sure of it.

  4. Update the changelog (though I suppose this could be done shortly after a release.)

@lpsmith lpsmith closed this Mar 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment