lpsmith / postgresql-simple

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

Does PGArray satisfy roundtrip property tests? #274

Closed rashadg1030 closed 5 years ago

rashadg1030 commented 5 years ago

I'm using postgresql-simple on a project and I'm wondering if the type PGArray satisfies roundtrip property tests and if not what I can do to fix that. In the project I have an Issue type which is a record:

-- | Data type representing a GitHub issue.
data Issue = Issue
    { issueId        :: Id Issue
    , issueNumber    :: Int
    , issueTitle     :: Text
    , issueBody      :: Text
    , issueRepoOwner :: RepoOwner
    , issueRepoName  :: RepoName
    , issueLabels    :: SqlArray Text
    } deriving stock (Generic, Show, Eq)
      deriving anyclass (ToJSON, FromRow)

instance ToRow Issue where
    toRow Issue{..} = toRow (issueNumber, issueTitle, issueBody, issueRepoOwner, issueRepoName, issueLabels)

SqlArray is a wrapper around the PGArray type defined as:

newtype SqlArray a = SqlArray { unSqlArray :: [a] }
    deriving stock   (Generic, Show)
    deriving newtype (Eq, ToJSON)
    deriving (ToField, FromField) via PGArray a

Our SQL schema for the issues table looks like this:

CREATE TABLE IF NOT EXISTS issues
( id         SERIAL    PRIMARY KEY 
, number     INT       NOT NULL
, title      TEXT      NOT NULL
, body       TEXT      NOT NULL
, repo_owner TEXT      NOT NULL
, repo_name  TEXT      NOT NULL
, labels     TEXT      ARRAY NOT NULL
, created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL
, updated_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL
);

I'm using hedgehog to test that queries to the database satisfy the roundtrip property. The test looks like this:

issueRoundtripProp :: AppEnv -> Property
issueRoundtripProp env = property $ do
    generatedIssue <- forAll genIssue
    parsedIssue <- liftIO 
                $ runAppLogIO env  
                $ asSingleRow
                $ query [sql| SELECT ?, ?, ?, ?, ?, ?, ? |] (Only (issueId generatedIssue) :. generatedIssue)
    parsedIssue === Right generatedIssue

I want the parsedIssue to be equal to the generatedIssue and it seems to work except for when the PGArray is empty. The test only fails when the PGArray is empty. It fails with this error:

✗ fromRow . toRow ≡ id failed after 83 tests.

       ┏━━ test/Test/Core/Issue.hs ━━━
    20 ┃ issueRoundtripProp :: AppEnv -> Property
    21 ┃ issueRoundtripProp env = property $ do
    22 ┃     generatedIssue <- forAll genIssue
    23 ┃     parsedIssue <- liftIO 
    24 ┃                 $ runAppLogIO env  
    25 ┃                 $ asSingleRow
    26 ┃                 $ query [sql| SELECT ?, ?, ?, ?, ?, ?, ? |] (Only (issueId generate
dIssue) :. generatedIssue)
    27 ┃     parsedIssue === Right generatedIssue
       ┃     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       ┃     │ ━━━ Exception (ResultError) ━━━
       ┃     │ Incompatible {errSQLType = "text", errSQLTableOid = Nothing, errSQLField = "?
column?", errHaskellType = "PGArray Text", errMessage = ""}

    This failure can be reproduced by running:
    > recheck (Size 82) (Seed 6549426097029610971 4625749683479689985) fromRow . toRow ≡ id

  ✗ 1 failed.

There is an Incompatible error and I'm trying to figure out if this because of the way PGArray is implemented or something else. I will continue to do testing and update this issue. Thank you for reading.

sopvop commented 5 years ago

errSQLType = "text" indicates that either libpq reports column to have text type, or pgs misinterprets libpq somehow. Have you tried adding type annotation to array in sql code? Maybe postgres can't inference type of empty array and thinks it's text.

phadej commented 5 years ago

@rashadg1030 @sopvop note that this is not the issue tracker of postgresql-simple anymore. I'd recommend to avoid commenting it.

I cannot close issues in this tracker, e.g.

See https://github.com/phadej/postgresql-simple/issues/21#issuecomment-508013439

rashadg1030 commented 5 years ago

errSQLType = "text" indicates that either libpq reports column to have text type, or pgs misinterprets libpq somehow. Have you tried adding type annotation to array in sql code? Maybe postgres can't inference type of empty array and thinks it's text.

@sopvop This is exactly what I needed to do. Thank you!