postgresql-simple icon indicating copy to clipboard operation
postgresql-simple copied to clipboard

Added HStoreV2 & related tests.

Open saurabhnanda opened this issue 8 years ago • 7 comments

Fixes #204

HStore has been copy-pasted and modified to create HStoreV2. There is a lot of scope for V2 to reuse functions & types defined in V1.

saurabhnanda avatar Jun 10 '17 08:06 saurabhnanda

@lpsmith This is not ready to be merged right now (too much code duplication), but we would appreciate if you could skim through the code and point out any glaring errors.

/cc @sras

saurabhnanda avatar Jun 10 '17 08:06 saurabhnanda

@lpsmith also, is there any way to allow the users of pg-simple to pick between two versions of HStoreList and HStoreMap -- one for (Text, Text) and the other for (Text, Maybe Text) without needing to have two separate version of HStore itself?

saurabhnanda avatar Jun 10 '17 08:06 saurabhnanda

@lpsmith is the following a good idea preserve backward compatibility AND to give the users of the library a choice to pick between Text or Maybe Text as the type of the hstore values:

newtype HStoreBase a = HStoreList {fromHStoreList :: [(Text, a)]} deriving (Show)
type HStoreList = HStoreBase Text
type HStoreNullable = HStoreBase (Maybe Text) 

saurabhnanda avatar Jun 10 '17 11:06 saurabhnanda

@lpsmith also, is there any way to allow the users of pg-simple to pick between two versions of HStoreList and HStoreMap -- one for (Text, Text) and the other for (Text, Maybe Text) without needing to have two separate version of HStore itself?

I dunno, I haven't given that idea much thought myself. The idea with HStoreBase is plausible, though I don't have any immediate reaction as to whether or not it would be a great idea going forward. (Although, it might make sense to do some additional conversions, e.g. to integers or whatever, though this would be a terribly inefficient way to store a map of text to integers in postgres.)

Though the one thing that strikes me about your statements is that you either misspoke, or you have the relationship between the two modules wrong. It would seem to me that there is a lot of opportunity for the HStore module to re-use code in the HStoreV2 module, the relationship in reverse (as you literally wrote) seems a little backwards to me.

lpsmith avatar Jun 10 '17 18:06 lpsmith

@lpsmith did you get a change to decide on this?

saurabhnanda avatar Nov 06 '17 13:11 saurabhnanda

@lpsmith any thoughts on this or #215 ? A PR in Opaleye's repo is waiting for this upstream merge.

saurabhnanda avatar Nov 09 '17 12:11 saurabhnanda

Sorry, life's been pretty crazy for me the last year, and I've not been paying enough attention to my open source efforts. So I do apologize for ignoring this issue for so long; you aren't the only one.

lpsmith avatar Nov 09 '17 17:11 lpsmith