Newlines between query chunks (#376)

* Newlines between query chunks * Fix whitespace newline issues * changelog, cabal, new ghcs * ok * lmao * uhhh
bitemyapp · Aug 29, 2023 · e50fed1 · e50fed1
1 parent b782593
commit e50fed1
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 10 deletions.
diff --git a/.github/workflows/haskell.yml b/.github/workflows/haskell.yml
@@ -32,13 +32,13 @@ jobs:
           --health-retries=3
     strategy:
       matrix:
-        cabal: ["3.6"]
-        ghc: ["8.6.5", "8.8.4", "8.10.4", "9.0.2", "9.2.2"]
+        cabal: ["3.8.1.0"]
+        ghc: ["8.6.5", "8.8.4", "8.10.4", "9.0.2", "9.2.2", "9.4.5", "9.6.2"]
     env:
       CONFIG: "--enable-tests --enable-benchmarks "
     steps:
       - uses: actions/checkout@v2
-      - uses: haskell/actions/setup@v1
+      - uses: haskell/actions/setup@v2
         id: setup-haskell-cabal
         with:
           ghc-version: ${{ matrix.ghc }}

diff --git a/changelog.md b/changelog.md
@@ -1,3 +1,16 @@
+3.5.10.2
+========
+- @parsonsmatt
+    - [#376](https://github.com/bitemyapp/esqueleto/pull/376)
+        - When using Postgres 15, `LIMIT`, and the `locking` functions, you
+          could accidentally construct SQL code like:
+
+          > ... LIMIT 1FOR UPDATE ...
+
+          This parsed on Postgres <15, but the new Postgres parser is more
+          strict, and fails to parse. This PR introduces newlines between each
+          query chunk, which fixes the issue.
+
 3.5.10.1
 ========
 - @9999years

diff --git a/esqueleto.cabal b/esqueleto.cabal
@@ -2,7 +2,7 @@ cabal-version: 1.12
 
 name:           esqueleto
 
-version:        3.5.10.1
+version:        3.5.10.2
 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/Internal/Internal.hs b/src/Database/Esqueleto/Internal/Internal.hs
@@ -2963,6 +2963,15 @@ toRawSql mode (conn, firstIdentState) query =
             flip S.runState firstIdentState $
             W.runWriterT $
             unQ query
+        deleteRepeatedNewlines txt =
+            let
+                (preNewlines, rest) = TL.break (== '\n') txt
+                (_, rest') = TL.break (/= '\n') rest
+             in
+                if TL.null rest'
+                    then preNewlines <> "\n"
+                    else preNewlines <> "\n" <> deleteRepeatedNewlines rest'
+
         SideData distinctClause
                  fromClauses
                  setClauses
@@ -2978,7 +2987,7 @@ toRawSql mode (conn, firstIdentState) query =
         -- that no name clashes will occur on subqueries that may
         -- appear on the expressions below.
         info = (projectBackend conn, finalIdentState)
-    in mconcat
+    in (\(x, t) -> (TLB.fromLazyText $ deleteRepeatedNewlines $ TL.strip $ TLB.toLazyText x, t)) $ mconcat $ intersperse ("\n", [])
         [ makeCte        info cteClause
         , makeInsertInto info mode ret
         , makeSelect     info mode distinctClause ret
@@ -2992,6 +3001,7 @@ toRawSql mode (conn, firstIdentState) query =
         , makeLocking    info lockingClause
         ]
 
+
 -- | Renders a 'SqlQuery' into a 'Text' value along with the list of
 -- 'PersistValue's that would be supplied to the database for @?@ placeholders.
 --
@@ -3213,11 +3223,11 @@ makeOrderBy :: IdentInfo -> [OrderByClause] -> (TLB.Builder, [PersistValue])
 makeOrderBy _ [] = mempty
 makeOrderBy info is =
     let (tlb, vals) = makeOrderByNoNewline info is
-    in ("\n" <> tlb, vals)
+    in (tlb, vals)
 
 makeLimit :: IdentInfo -> LimitClause -> (TLB.Builder, [PersistValue])
 makeLimit (conn, _) (Limit ml mo) =
-    let limitRaw = getConnLimitOffset (v ml, v mo) "\n" conn
+    let limitRaw = getConnLimitOffset (v ml, v mo) "" conn
         v :: Maybe Int64 -> Int
         v = maybe 0 fromIntegral
     in (TLB.fromText limitRaw, mempty)

diff --git a/test/Common/Test.hs b/test/Common/Test.hs
@@ -1652,9 +1652,11 @@ testLocking = do
           [complex, with1, with2, with3] <-
             return $
               map (toText conn) [complexQuery, queryWithClause1, queryWithClause2, queryWithClause3]
-          let expected = complex <> "\n" <> syntax
-          asserting $
-              (with1, with2, with3) `shouldBe` (expected, expected, expected)
+          let expected = complex <> syntax <> "\n"
+          asserting $ do
+            with1 `shouldBe` expected
+            with2 `shouldBe` expected
+            with3 `shouldBe` expected
     itDb "looks sane for ForUpdate"           $ sanityCheck ForUpdate           "FOR UPDATE"
     itDb "looks sane for ForUpdateSkipLocked" $ sanityCheck ForUpdateSkipLocked "FOR UPDATE SKIP LOCKED"
     itDb "looks sane for ForShare"            $ sanityCheck ForShare            "FOR SHARE"