Revert change to ToAlias and refix union bug (#336)

* Revert change to ToAlias that caused issues with subqueries selecting references. Change union behavior to reuse value aliases within the union itself. * Update test/PostgreSQL/Test.hs * Added cabal and changelog. Unfocus tests Co-authored-by: Matt Parsons <[email protected]>
bitemyapp · Aug 31, 2022 · 754b0b5 · 754b0b5
1 parent 84feb9b
commit 754b0b5
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 16 deletions.
diff --git a/changelog.md b/changelog.md
@@ -1,3 +1,10 @@
+3.5.8.1
+=======
+- @belevy
+    - [#336](https://github.com/bitemyapp/esqueleto/pull/336)
+        - Fix bug with multiple nested subqueries introduced in 3.5.7.1
+        - Set operations will now only reuse variable names within the context of the set operation.
+            a subquery that references the set operation will correctly pick up where the subquery left off
 3.5.8.0
 =======
 - @ivanbakel

diff --git a/esqueleto.cabal b/esqueleto.cabal
@@ -2,7 +2,7 @@ cabal-version: 1.12
 
 name:           esqueleto
 
-version:        3.5.8.0
+version:        3.5.8.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/Experimental/From/SqlSetOperation.hs b/src/Database/Esqueleto/Experimental/From/SqlSetOperation.hs
@@ -50,9 +50,7 @@ instance (SqlSelect a r, ToAlias a, ToAliasReference a) => ToSqlSetOperation (Sq
     toSqlSetOperation subquery =
         SqlSetOperation $ \p -> do
             (ret, sideData) <- Q $ W.censor (\_ -> mempty) $ W.listen $ unQ subquery
-            state <- Q $ lift S.get
             aliasedValue <- toAlias ret
-            Q $ lift $ S.put state
             let aliasedQuery = Q $ W.WriterT $ pure (aliasedValue, sideData)
             let p' =
                   case p of
@@ -70,7 +68,9 @@ instance (SqlSelect a r, ToAlias a, ToAliasReference a) => ToSqlSetOperation (Sq
 mkSetOperation :: (ToSqlSetOperation a a', ToSqlSetOperation b a')
                => TLB.Builder -> a -> b -> SqlSetOperation a'
 mkSetOperation operation lhs rhs = SqlSetOperation $ \p -> do
+    state <- Q $ lift S.get
     (leftValue, leftClause) <- unSqlSetOperation (toSqlSetOperation lhs) p
+    Q $ lift $ S.put state
     (_, rightClause) <- unSqlSetOperation (toSqlSetOperation rhs) p
     pure (leftValue, \info -> leftClause info <> (operation, mempty) <> rightClause info)
 

diff --git a/src/Database/Esqueleto/Experimental/ToAlias.hs b/src/Database/Esqueleto/Experimental/ToAlias.hs
@@ -16,20 +16,26 @@ class ToAlias a where
     toAlias :: a -> SqlQuery a
 
 instance ToAlias (SqlExpr (Value a)) where
-    toAlias (ERaw m f) = do
-        ident <- newIdentFor (DBName "v")
-        pure $ ERaw m{sqlExprMetaIsReference = False, sqlExprMetaAlias = Just ident} f
+    toAlias e@(ERaw m f)
+      | Just _ <- sqlExprMetaAlias m = pure e
+      | otherwise = do
+            ident <- newIdentFor (DBName "v")
+            pure $ ERaw noMeta{sqlExprMetaAlias = Just ident} f
 
 instance ToAlias (SqlExpr (Entity a)) where
-    toAlias (ERaw m f) = do
-       ident <- newIdentFor (DBName "v")
-       pure $ ERaw m{sqlExprMetaIsReference = False, sqlExprMetaAlias = Just ident} f
+    toAlias e@(ERaw m f)
+      | Just _ <- sqlExprMetaAlias m = pure e
+      | otherwise = do
+           ident <- newIdentFor (DBName "v")
+           pure $ ERaw m{sqlExprMetaIsReference = False, sqlExprMetaAlias = Just ident} f
 
 instance ToAlias (SqlExpr (Maybe (Entity a))) where
     -- FIXME: Code duplication because the compiler doesnt like half final encoding
-    toAlias (ERaw m f) = do
-       ident <- newIdentFor (DBName "v")
-       pure $ ERaw m{sqlExprMetaIsReference = False, sqlExprMetaAlias = Just ident} f
+    toAlias e@(ERaw m f)
+      | Just _ <- sqlExprMetaAlias m = pure e
+      | otherwise = do
+           ident <- newIdentFor (DBName "v")
+           pure $ ERaw m{sqlExprMetaIsReference = False, sqlExprMetaAlias = Just ident} f
 
 instance (ToAlias a, ToAlias b) => ToAlias (a,b) where
     toAlias (a,b) = (,) <$> toAlias a <*> toAlias b

diff --git a/test/PostgreSQL/Test.hs b/test/PostgreSQL/Test.hs
@@ -1318,9 +1318,14 @@ testValuesExpression = do
                                        , (Value 2, Value "str2", Value $ Just 2.5)
                                        , (Value 3, Value "str3", Value Nothing) ]
 
-testSubselectUnionError :: SpecDb
-testSubselectUnionError = do
-    describe "Subselect union error" $ do
+testSubselectAliasingBehavior :: SpecDb
+testSubselectAliasingBehavior = do
+    describe "Aliasing behavior" $ do
+        itDb "correctly realiases entities accross multiple subselects" $ do
+            _ <- select $ do
+                    Experimental.from $ Experimental.from $ Experimental.from $ table @Lord
+            asserting noExceptions
+
         itDb "doesnt erroneously repeat variable names when using subselect + union" $ do
             let lordQuery = do
                     l <- Experimental.from $ table @Lord
@@ -1428,7 +1433,7 @@ spec = beforeAll mkConnectionPool $ do
                 testJSONOperators
         testLateralQuery
         testValuesExpression
-        testSubselectUnionError
+        testSubselectAliasingBehavior
 
 insertJsonValues :: SqlPersistT IO ()
 insertJsonValues = do