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

HStore support #309

Open
saurabhnanda opened this issue Jun 16, 2017 · 16 comments
Open

HStore support #309

saurabhnanda opened this issue Jun 16, 2017 · 16 comments

Comments

@saurabhnanda
Copy link
Contributor

Planning to raise a PR for this, once one of lpsmith/postgresql-simple#214 or lpsmith/postgresql-simple#215 is merged into PG-Simple.

However, in my development branch, I'm facing a very strange issue. Here's how I've defined the PGHStore data type:

{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE MultiParamTypeClasses #-}

module Opaleye.PGHStore where

import Opaleye
import Data.Profunctor.Product.Default
import VacationLabs.Database.PostgreSQL.Simple.HStore
import Opaleye.Internal.RunQuery ()

data PGHStore

instance IsSqlType PGHStore where
  showPGType _ = "hstore"

pgHStore :: HStoreMap -> Column PGHStore
pgHStore h = unsafeCoerceColumn $ pgLazyByteString $ toLazyByteString (toHStore h)

instance QueryRunnerColumnDefault PGHStore HStoreMap where
  queryRunnerColumnDefault = fieldQueryRunnerColumn

instance Default Constant HStoreMap (Column PGHStore) where
  def = Constant pgHStore

A simple test fails:

testHStoreDB :: Connection -> Assertion
testHStoreDB conn = void $ withTransaction conn $ do
  _ <- execute_ conn "create table if not exists hstore_test(hstore_col hstore)"
  -- h <- generate (arbitrary :: Gen HStoreMap)
  let h = HStoreMap $ fromList $ [("a", "b")]
  (h_:_) <- runInsertManyReturning conn hstoreTestTable [constant h] Prelude.id
  -- _ <- execute conn "insert into hstore_test(hstore_col) values(?)" (Only h)
  assertEqual "not equal" h h_
  execute_ conn "drop table if exists hstore_test"

Generated SQL query:

INSERT INTO "hstore_test" ("hstore_col")
	VALUES (E'\\x2261223d3e226222')
	RETURNING "hstore_col"

Expected SQL query:

INSERT INTO "hstore_test" ("hstore_col") VALUES ('"a"=>"b"') RETURNING "hstore_col"

I've already validated that the pg-simple machinery is working almost correctly. So, I've done something really stupid one while defining PGHStore above.

@tomjaguarpaw
Copy link
Owner

tomjaguarpaw commented Jun 16, 2017

I guess toHStore ("a", "b") = "\\x2261223d3e226222". Can you check? Where is toHStore defined?

@saurabhnanda
Copy link
Contributor Author

toHStore is defined in pg-simple

instance ToHStore HStoreMap where
    toHStore (HStoreMap xs) = Map.foldrWithKey f mempty xs
      where f k v ys = hstore k v `mappend` ys

class ToHStore a where
   toHStore :: a -> HStoreBuilder

toLazyByteString :: HStoreBuilder -> BL.ByteString
toLazyByteString x = case x of
                       Empty -> BL.empty
                       Comma y -> BU.toLazyByteString y

@saurabhnanda
Copy link
Contributor Author

The core conversion to a lazy bytestring is correct. I checked:

� toLazyByteString $ H.toHStore $ HStoreMap $ fromList [("a", "b")]
"\"a\"=>\"b\""
� ```

@tomjaguarpaw
Copy link
Owner

It's a bit confusing. Firstly, it seems like E'\\x2261223d3e226222' is actually a valid encoding of the Postgres string '"a"=>"b"'. Can you check? I don't have a server to hand.

Secondly, I looked at what pgLazyByteString does, and it converts to a bytea, not a text. I guess ideally you want the latter. Perhaps try converting the ByteString to an ASCII String first and going via pgString?

@tomjaguarpaw
Copy link
Owner

@saurabhnanda
Copy link
Contributor Author

Ok -- got it. Here's the correct code:

pgHStore :: HStoreMap -> Column PGHStore
pgHStore h = IPT.castToType "hstore" $ HSD.quote $ IPT.lazyDecodeUtf8 $ toLazyByteString (toHStore h)

@tomjaguarpaw
Copy link
Owner

Cool. What's HSD.quote?

@saurabhnanda
Copy link
Contributor Author

import qualified Opaleye.Internal.HaskellDB.Sql.Default as HSD

@tomjaguarpaw
Copy link
Owner

OK, this seems pretty reasonable.

@saurabhnanda
Copy link
Contributor Author

saurabhnanda commented Jun 16, 2017

so, can we keep this issue open, while I chase down the PR on pg-simple?

@saurabhnanda
Copy link
Contributor Author

I guess it'll be a good idea to include all the hstore functions in the PGHStore PR as well, right?

@tomjaguarpaw
Copy link
Owner

  • Feel free to keep the issue open if you like

  • Including all the hstore functions would be great

@tomjaguarpaw
Copy link
Owner

(and tests)

@saurabhnanda
Copy link
Contributor Author

@tomjaguarpaw do you have any thoughts on how to deal with haskellari/qc-instances#9 (comment) Any tests in Opaleye based on QC would also be affected by this issue, right?

@tomjaguarpaw
Copy link
Owner

The Opaleye QuickCheck tests only do round-tripping of Int and Bool at the moment so I have never come across this issue.

@tomjaguarpaw
Copy link
Owner

tomjaguarpaw commented Jun 21, 2017

I think patching quickcheck-text as you have done is the most sensible solution.

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

No branches or pull requests

2 participants