Mattp/incorporate cte aliasing

changelog.md

@@ -3,6 +3,9 @@
 - @blujupiter32
     - [#379](https://github.com/bitemyapp/esqueleto/pull/379)
         - Fix a bug where `not_ (a &&. b)` would be interpeted as `(not_ a) &&. b`
+- @RikvanToor
+    - [#373](https://github.com/bitemyapp/esqueleto/pull/373), [#410](https://github.com/bitemyapp/esqueleto/pull/410)
+        - Fix name clashes when using CTEs multiple times
 - @TeofilC
     - [#394](https://github.com/bitemyapp/esqueleto/pull/394)
         - Use TH quotes to eliminate some CPP.

esqueleto.cabal

@@ -87,6 +87,7 @@ test-suite specs
     main-is: Spec.hs
     other-modules:
         Common.Test
+        Common.Test.CTE
         Common.Test.Models
         Common.Test.Import
         Common.Test.Select

src/Database/Esqueleto/Experimental/From/CommonTableExpression.hs

@@ -53,7 +53,11 @@ with query = do
     let clause = CommonTableExpressionClause NormalCommonTableExpression ident (\info -> toRawSql SELECT info aliasedQuery)
     Q $ W.tell mempty{sdCteClause = [clause]}
     ref <- toAliasReference ident aliasedValue
-    pure $ From $ pure (ref, (\_ info -> (useIdent info ident, mempty)))
+    pure $ From $ do
+        newIdent <- newIdentFor (DBName "cte")
+        localRef <- toAliasReference newIdent ref
+        let makeLH info = useIdent info ident <> " AS " <> useIdent info newIdent
+        pure (localRef, (\_ info -> (makeLH info, mempty)))
 
 -- | @WITH@ @RECURSIVE@ allows one to make a recursive subquery, which can
 -- reference itself. Like @WITH@, this is supported in most modern SQL engines.

test/Common/Test.hs

@@ -86,11 +86,12 @@
 import qualified Database.Esqueleto.Internal.ExprParser as P
 import qualified Database.Esqueleto.Internal.Internal as EI
 import Database.Esqueleto.PostgreSQL as EP
 import Database.Persist.Class.PersistEntity
 import qualified UnliftIO.Resource as R
 
 import Common.Record (testDeriveEsqueletoRecord)
 import Common.Test.Select
+import qualified Common.Test.CTE as CTESpec
 
 -- Test schema
 -- | this could be achieved with S.fromList, but not all lists
@@ -2426,6 +2427,7 @@
         testLocking
         testOverloadedRecordDot
         testDeriveEsqueletoRecord
+        CTESpec.testCTE
 
 insert' :: ( Functor m
            , BaseBackend backend ~ PersistEntityBackend val

test/Common/Test/CTE.hs

@@ -0,0 +1,35 @@
+{-# language TypeApplications #-}
+
+module Common.Test.CTE where
+
+import Common.Test.Models
+import Common.Test.Import
+import Database.Persist.TH
+
+testCTE :: SpecDb
+testCTE = describe "CTE" $ do
+    itDb "can refer to the same CTE twice" $ do
+        let q :: SqlQuery (SqlExpr (Value Int), SqlExpr (Value Int))
+            q = do
+                bCte <- with $ do
+                    b <- from $ table @B
+                    pure b
+
+                a :& b1 :& b2 <- from $
+                    table @A
+                        `innerJoin` bCte
+                            `on` do
+                                \(a :& b) ->
+                                    a ^. AK ==. b ^. BK
+                        `innerJoin` bCte
+                            `on` do
+                                \(a :& _ :& b2) ->
+                                    a ^. AK ==. b2 ^. BK
+                pure (a ^. AK, a ^. AV +. b1 ^. BV +. b2 ^. BV)
+        insert_ $ A { aK = 1, aV = 2 }
+        insert_ $ B { bK = 1, bV = 3 }
+        ret <- select q
+        asserting $ do
+            ret `shouldMatchList`
+                [ (Value 1, Value (2 + 3 + 3))
+                ]

test/Common/Test/Models.hs

@@ -24,10 +24,10 @@
 
 import Data.Time
 import Database.Esqueleto.Experimental
 import Database.Persist.Sql
 import Database.Persist.TH
 
 share [mkPersist sqlSettings, mkMigrate "migrateAll"] [persistUpperCase|
   Foo
     name Int
     Primary name
@@ -182,6 +182,16 @@
     address String
     deriving Show
     deriving Eq
+
+  A
+      k Int
+      v Int
+      Primary k
+
+  B
+      k Int
+      v Int
+      Primary k
 |]
 
 -- Unique Test schema

test/PostgreSQL/Test.hs

@@ -1347,7 +1347,6 @@ testPostgresqlLocking = do
                                             EP.forUpdateOf p EP.skipLocked
                                             return p
 
-                                liftIO $ print nonLockedRowsSpecifiedTable
                                 pure $ length nonLockedRowsSpecifiedTable `shouldBe` 2
 
                     withAsync sideThread $ \sideThreadAsync -> do
@@ -1371,7 +1370,6 @@ testPostgresqlLocking = do
                                                     EP.forUpdateOf p EP.skipLocked
                                                     return p
 
-                            liftIO $ print nonLockedRowsAfterUpdate
                             asserting sideThreadAsserts
                             asserting $ length nonLockedRowsAfterUpdate `shouldBe` 3