From e9c87dbf704122065c324d0614ce09112f058dc7 Mon Sep 17 00:00:00 2001 From: parsonsmatt Date: Sat, 5 Mar 2022 13:44:18 -0700 Subject: [PATCH 1/5] Support upsert with empty updates --- Makefile | 2 - changelog.md | 10 +++- esqueleto.cabal | 2 +- src/Database/Esqueleto/PostgreSQL.hs | 62 +++++++++++++++------ test/PostgreSQL/Test.hs | 83 ++++++++++++++-------------- test/Spec.hs | 1 - 6 files changed, 94 insertions(+), 66 deletions(-) diff --git a/Makefile b/Makefile index fd3b9378f..9f07be915 100644 --- a/Makefile +++ b/Makefile @@ -33,8 +33,6 @@ test-ghcid-build: --restart "stack.yaml" \ --restart "esqueleto.cabal" - - init-pgsql: sudo -u postgres -- createuser -s esqutest diff --git a/changelog.md b/changelog.md index c7c440be3..c4660f462 100644 --- a/changelog.md +++ b/changelog.md @@ -1,10 +1,18 @@ +3.5.3.1 +======= +- @parsonsmatt + - [#](https://github.com/bitemyapp/esqueleto/pull/) + - 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 diff --git a/esqueleto.cabal b/esqueleto.cabal index 691d5796d..a10e9b827 100644 --- a/esqueleto.cabal +++ b/esqueleto.cabal @@ -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. . diff --git a/src/Database/Esqueleto/PostgreSQL.hs b/src/Database/Esqueleto/PostgreSQL.hs index 072d2737e..9aac73829 100644 --- a/src/Database/Esqueleto/PostgreSQL.hs +++ b/src/Database/Esqueleto/PostgreSQL.hs @@ -47,6 +47,7 @@ import Data.Maybe import Data.Proxy (Proxy(..)) import qualified Data.Text.Internal.Builder as TLB import qualified Data.Text.Lazy as TL +import qualified Data.Text as Text import Data.Time.Clock (UTCTime) import qualified Database.Esqueleto.Experimental as Ex import qualified Database.Esqueleto.Experimental.From as Ex @@ -222,18 +223,44 @@ 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 = + case updates of + [] -> + ("", []) + _ -> + 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 @@ -248,10 +275,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 @@ -261,17 +285,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 diff --git a/test/PostgreSQL/Test.hs b/test/PostgreSQL/Test.hs index b6dd09803..c9c99433a 100644 --- a/test/PostgreSQL/Test.hs +++ b/test/PostgreSQL/Test.hs @@ -51,6 +51,34 @@ import Common.Test import Common.Test.Import hiding (from, on) import PostgreSQL.MigrateJSON +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 + returningType :: forall a m . m a -> m a returningType a = a @@ -1038,18 +1066,20 @@ testInsertUniqueViolation = sqlErrorHint = ""} testUpsert :: SpecDb -testUpsert = - describe "Upsert test" $ do +testUpsert = 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"} + itDb "Works with no updates" $ do + _ <- EP.upsert u1 [] + pure () testInsertSelectWithConflict :: SpecDb testInsertSelectWithConflict = @@ -1378,39 +1408,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 diff --git a/test/Spec.hs b/test/Spec.hs index f6201f104..8d0bcd9f7 100644 --- a/test/Spec.hs +++ b/test/Spec.hs @@ -19,4 +19,3 @@ spec = do sequential $ MySQL.spec describe "Postgresql" $ do sequential $ Postgres.spec - From 8ef6846931c888061f3ae89eac6d4dbb22d47725 Mon Sep 17 00:00:00 2001 From: parsonsmatt Date: Sat, 5 Mar 2022 13:59:46 -0700 Subject: [PATCH 2/5] stylish, changelog link --- changelog.md | 2 +- src/Database/Esqueleto/PostgreSQL.hs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/changelog.md b/changelog.md index c4660f462..efd61a270 100644 --- a/changelog.md +++ b/changelog.md @@ -1,7 +1,7 @@ 3.5.3.1 ======= - @parsonsmatt - - [#](https://github.com/bitemyapp/esqueleto/pull/) + - [#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. diff --git a/src/Database/Esqueleto/PostgreSQL.hs b/src/Database/Esqueleto/PostgreSQL.hs index 9aac73829..e6fe38410 100644 --- a/src/Database/Esqueleto/PostgreSQL.hs +++ b/src/Database/Esqueleto/PostgreSQL.hs @@ -45,16 +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 qualified Data.Text as Text 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 From 84ff8de7ce3ad4bfca0072055e09cf31433d60fd Mon Sep 17 00:00:00 2001 From: parsonsmatt Date: Sat, 5 Mar 2022 14:04:31 -0700 Subject: [PATCH 3/5] clean --- src/Database/Esqueleto/PostgreSQL.hs | 6 +----- test/PostgreSQL/Test.hs | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Database/Esqueleto/PostgreSQL.hs b/src/Database/Esqueleto/PostgreSQL.hs index e6fe38410..2953908c3 100644 --- a/src/Database/Esqueleto/PostgreSQL.hs +++ b/src/Database/Esqueleto/PostgreSQL.hs @@ -232,11 +232,7 @@ upsertBy uniqueKey record updates = do entDef = entityDef (Just record) updatesText conn = - case updates of - [] -> - ("", []) - _ -> - first builderToText $ renderUpdates conn updates + first builderToText $ renderUpdates conn updates #if MIN_VERSION_persistent(2,11,0) uniqueFields = persistUniqueToFieldNames uniqueKey handler sqlB upsertSql = do diff --git a/test/PostgreSQL/Test.hs b/test/PostgreSQL/Test.hs index c9c99433a..437e47f0f 100644 --- a/test/PostgreSQL/Test.hs +++ b/test/PostgreSQL/Test.hs @@ -52,7 +52,7 @@ import Common.Test.Import hiding (from, on) import PostgreSQL.MigrateJSON spec :: Spec -spec = beforeAll mkConnectionPool $ do +spec = focus $ beforeAll mkConnectionPool $ do tests describe "PostgreSQL specific tests" $ do From c5c7650da961bb0e778d58fb7162ab311d960621 Mon Sep 17 00:00:00 2001 From: parsonsmatt Date: Sat, 5 Mar 2022 14:23:31 -0700 Subject: [PATCH 4/5] remove focus --- test/PostgreSQL/Test.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/PostgreSQL/Test.hs b/test/PostgreSQL/Test.hs index 437e47f0f..c9c99433a 100644 --- a/test/PostgreSQL/Test.hs +++ b/test/PostgreSQL/Test.hs @@ -52,7 +52,7 @@ import Common.Test.Import hiding (from, on) import PostgreSQL.MigrateJSON spec :: Spec -spec = focus $ beforeAll mkConnectionPool $ do +spec = beforeAll mkConnectionPool $ do tests describe "PostgreSQL specific tests" $ do From 6b22afbab29675971e388f98a3d749de242803db Mon Sep 17 00:00:00 2001 From: parsonsmatt Date: Sat, 5 Mar 2022 15:08:09 -0700 Subject: [PATCH 5/5] oh no --- test/PostgreSQL/Test.hs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/test/PostgreSQL/Test.hs b/test/PostgreSQL/Test.hs index c9c99433a..2069f9608 100644 --- a/test/PostgreSQL/Test.hs +++ b/test/PostgreSQL/Test.hs @@ -1066,7 +1066,7 @@ 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 @@ -1077,9 +1077,16 @@ testUpsert = describe "Upsert test" $ do liftIO $ entityVal u2e `shouldBe` u2 u3e <- EP.upsert u3 [OneUniqueName =. val "fifth"] liftIO $ entityVal u3e `shouldBe` u1{oneUniqueName="fifth"} - itDb "Works with no updates" $ do - _ <- EP.upsert u1 [] - pure () + describe "With no updates" $ do + itDb "Works with no updates" $ do + _ <- EP.upsert u1 [] + pure () + itDb "Works with no updates, twice" $ do + Entity u1Key u1' <- EP.upsert u1 [] + Entity u1Key_ u1'' <- EP.upsert u1 { oneUniqueName = "Something Else" } [] + pure () + -- liftIO $ do + -- u1 `shouldBe` u1' testInsertSelectWithConflict :: SpecDb testInsertSelectWithConflict =