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

Support upsert with empty updates #301

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ test-ghcid-build:
--restart "stack.yaml" \
--restart "esqueleto.cabal"



init-pgsql:
sudo -u postgres -- createuser -s esqutest

Expand Down
10 changes: 9 additions & 1 deletion changelog.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
3.5.3.1
=======
- @parsonsmatt
- [#301](https://github.com/bitemyapp/esqueleto/pull/301)
- Fix `upsert` and `upsertBy` to produce `INSERT ... ON CONFLICT DO
NOTHING` code when given an empty list.


3.5.3.0
=======
- @m4dc4p
- [#291](https://github.com/bitemyapp/esqueleto/pull/291)
- Added `ToAlias` and `ToAliasReference` instaces to the `:&` type, mirroring
the tuple instances for the same classes. See [Issue #290](https://github.com/bitemyapp/esqueleto/issues/290)
for discussion.
for discussion.
- @NikitaRazmakhnin
- [#284](https://github.com/bitemyapp/esqueleto/pull/284)
- Add PostgreSQL-specific support of VALUES(..) literals
Expand Down
2 changes: 1 addition & 1 deletion esqueleto.cabal
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cabal-version: 1.12

name: esqueleto
version: 3.5.3.0
version: 3.5.3.1
synopsis: Type-safe EDSL for SQL queries on persistent backends.
description: @esqueleto@ is a bare bones, type-safe EDSL for SQL queries that works with unmodified @persistent@ SQL backends. Its language closely resembles SQL, so you don't have to learn new concepts, just new syntax, and it's fairly easy to predict the generated SQL and optimize it for your backend. Most kinds of errors committed when writing SQL are caught as compile-time errors---although it is possible to write type-checked @esqueleto@ queries that fail at runtime.
.
Expand Down
60 changes: 41 additions & 19 deletions src/Database/Esqueleto/PostgreSQL.hs
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,16 @@ import Data.Int (Int64)
import qualified Data.List.NonEmpty as NE
import Data.Maybe
import Data.Proxy (Proxy(..))
import qualified Data.Text as Text
import qualified Data.Text.Internal.Builder as TLB
import qualified Data.Text.Lazy as TL
import Data.Time.Clock (UTCTime)
import qualified Database.Esqueleto.Experimental as Ex
import qualified Database.Esqueleto.Experimental.From as Ex
import Database.Esqueleto.Internal.Internal hiding (random_)
import Database.Esqueleto.Internal.PersistentImport hiding (upsert, upsertBy)
import Database.Persist.Class (OnlyOneUniqueKey)
import Database.Persist (ConstraintNameDB(..), EntityNameDB(..))
import Database.Persist.Class (OnlyOneUniqueKey)
import Database.Persist.SqlBackend

-- | (@random()@) Split out into database specific modules
Expand Down Expand Up @@ -222,18 +223,40 @@ upsertBy uniqueKey record updates = do
Just upsertSql ->
handler sqlB upsertSql
where
addVals l = map toPersistValue (toPersistFields record) ++ l ++ persistUniqueToValues uniqueKey
entDef = entityDef (Just record)
updatesText conn = first builderToText $ renderUpdates conn updates
addVals l =
map toPersistValue (toPersistFields record) ++ l ++ case updates of
[] ->
[]
_ ->
persistUniqueToValues uniqueKey
entDef =
entityDef (Just record)
updatesText conn =
first builderToText $ renderUpdates conn updates
#if MIN_VERSION_persistent(2,11,0)
uniqueFields = persistUniqueToFieldNames uniqueKey
handler sqlB upsertSql = do
let (updateText, updateVals) =
updatesText sqlB
queryText =
queryTextUnmodified =
upsertSql entDef uniqueFields updateText
queryText =
case updates of
[] ->
let
(okay, _bad) =
Text.breakOn "DO UPDATE" queryTextUnmodified
good =
okay <> "DO NOTHING RETURNING ??"
in
good
_ ->
queryTextUnmodified

queryVals =
addVals updateVals
addVals $ case updates of
[] -> []
_ -> updateVals
xs <- rawSql queryText queryVals
pure (head xs)
#else
Expand All @@ -248,10 +271,7 @@ upsertBy uniqueKey record updates = do
-- Example of usage:
--
-- @
-- share [ mkPersist sqlSettings
-- , mkDeleteCascade sqlSettings
-- , mkMigrate "migrate"
-- ] [persistLowerCase|
-- 'mkPersist' 'sqlSettings' ['persistLowerCase'|
-- Bar
-- num Int
-- deriving Eq Show
Expand All @@ -261,17 +281,19 @@ upsertBy uniqueKey record updates = do
-- deriving Eq Show
-- |]
--
-- insertSelectWithConflict
-- UniqueFoo -- (UniqueFoo undefined) or (UniqueFoo anyNumber) would also work
-- (from $ \b ->
-- return $ Foo <# (b ^. BarNum)
-- )
-- (\current excluded ->
-- [FooNum =. (current ^. FooNum) +. (excluded ^. FooNum)]
-- )
-- action = do
-- 'insertSelectWithConflict'
-- UniqueFoo -- (UniqueFoo undefined) or (UniqueFoo anyNumber) would also work
-- (do
-- b <- from $ table \@Bar
-- return $ Foo <# (b ^. BarNum)
-- )
-- (\\current excluded ->
-- [FooNum =. (current ^. FooNum) +. (excluded ^. FooNum)]
-- )
-- @
--
-- Inserts to table Foo all Bar.num values and in case of conflict SomeFooUnique,
-- Inserts to table @Foo@ all @Bar.num@ values and in case of conflict @SomeFooUnique@,
-- the conflicting value is updated to the current plus the excluded.
--
-- @since 3.1.3
Expand Down
90 changes: 47 additions & 43 deletions test/PostgreSQL/Test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,34 @@ import Common.Test
import Common.Test.Import hiding (from, on)
import PostgreSQL.MigrateJSON

spec :: Spec
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just moved up to the top

spec = beforeAll mkConnectionPool $ do
tests

describe "PostgreSQL specific tests" $ do
testAscRandom random_
testRandomMath
testSelectDistinctOn
testPostgresModule
testPostgresqlOneAscOneDesc
testPostgresqlTwoAscFields
testPostgresqlSum
testPostgresqlRandom
testPostgresqlUpdate
testPostgresqlCoalesce
testPostgresqlTextFunctions
testInsertUniqueViolation
testUpsert
testInsertSelectWithConflict
testFilterWhere
testCommonTableExpressions
setDatabaseState insertJsonValues cleanJSON
$ describe "PostgreSQL JSON tests" $ do
testJSONInsertions
testJSONOperators
testLateralQuery
testValuesExpression

returningType :: forall a m . m a -> m a
returningType a = a

Expand Down Expand Up @@ -1038,18 +1066,27 @@ testInsertUniqueViolation =
sqlErrorHint = ""}

testUpsert :: SpecDb
testUpsert =
describe "Upsert test" $ do
testUpsert = focus $ describe "Upsert test" $ do
itDb "Upsert can insert like normal" $ do
u1e <- EP.upsert u1 [OneUniqueName =. val "fifth"]
liftIO $ entityVal u1e `shouldBe` u1
u1e <- EP.upsert u1 [OneUniqueName =. val "fifth"]
liftIO $ entityVal u1e `shouldBe` u1
itDb "Upsert performs update on collision" $ do
u1e <- EP.upsert u1 [OneUniqueName =. val "fifth"]
liftIO $ entityVal u1e `shouldBe` u1
u2e <- EP.upsert u2 [OneUniqueName =. val "fifth"]
liftIO $ entityVal u2e `shouldBe` u2
u3e <- EP.upsert u3 [OneUniqueName =. val "fifth"]
liftIO $ entityVal u3e `shouldBe` u1{oneUniqueName="fifth"}
u1e <- EP.upsert u1 [OneUniqueName =. val "fifth"]
liftIO $ entityVal u1e `shouldBe` u1
u2e <- EP.upsert u2 [OneUniqueName =. val "fifth"]
liftIO $ entityVal u2e `shouldBe` u2
u3e <- EP.upsert u3 [OneUniqueName =. val "fifth"]
liftIO $ entityVal u3e `shouldBe` u1{oneUniqueName="fifth"}
describe "With no updates" $ do
itDb "Works with no updates" $ do
_ <- EP.upsert u1 []
pure ()
Comment on lines +1081 to +1083
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this passes. Hooray.

itDb "Works with no updates, twice" $ do
Entity u1Key u1' <- EP.upsert u1 []
Entity u1Key_ u1'' <- EP.upsert u1 { oneUniqueName = "Something Else" } []
pure ()
Comment on lines +1084 to +1087
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this fails with ErrorCall "head: empty list".

The reason is that:

INSERT INTO "OneUnique" (name, value)
VALUES (?, ?)
ON CONFLICT DO NOTHING
RETURNING id, name, value

does not return anything if we don't perform an INSERT - so the return is [] which he head on and it goes boom.

DO UPDATE always does something. But we really don't want to do something silly like DO UPDATE SET blah = EXCLUDED.blah because this does unnecessary writes and triggers.

-- liftIO $ do
-- u1 `shouldBe` u1'

testInsertSelectWithConflict :: SpecDb
testInsertSelectWithConflict =
Expand Down Expand Up @@ -1378,39 +1415,6 @@ selectJSON f = select $ from $ \v -> do
f $ just (v ^. JsonValue)
return v

--------------- JSON --------------- JSON --------------- JSON ---------------
--------------- JSON --------------- JSON --------------- JSON ---------------
--------------- JSON --------------- JSON --------------- JSON ---------------



spec :: Spec
spec = beforeAll mkConnectionPool $ do
tests

describe "PostgreSQL specific tests" $ do
testAscRandom random_
testRandomMath
testSelectDistinctOn
testPostgresModule
testPostgresqlOneAscOneDesc
testPostgresqlTwoAscFields
testPostgresqlSum
testPostgresqlRandom
testPostgresqlUpdate
testPostgresqlCoalesce
testPostgresqlTextFunctions
testInsertUniqueViolation
testUpsert
testInsertSelectWithConflict
testFilterWhere
testCommonTableExpressions
setDatabaseState insertJsonValues cleanJSON
$ describe "PostgreSQL JSON tests" $ do
testJSONInsertions
testJSONOperators
testLateralQuery
testValuesExpression

insertJsonValues :: SqlPersistT IO ()
insertJsonValues = do
Expand Down
1 change: 0 additions & 1 deletion test/Spec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,3 @@ spec = do
sequential $ MySQL.spec
describe "Postgresql" $ do
sequential $ Postgres.spec