bitemyapp / esqueleto

New home of Esqueleto, please file issues so we can get things caught up!
BSD 3-Clause "New" or "Revised" License
370 stars 107 forks source link

Alias CTEs upon initialising #373

Open RikvanToor opened 10 months ago

RikvanToor commented 10 months ago

Hello. :)

This PR fixes #372 by adding a fresh alias whenever a common table expression is used within a query.

As described in the issue, previously this Haskell code

share [mkPersist sqlSettings] [persistLowerCase|
  A
    k Int
    v Int
    Primary k

  B
    k Int
    v Int
    Primary k
|]

q :: SqlQuery (SqlExpr (Value Int), SqlExpr (Value Int))
q = do
  bCte <- with $ do
    b <- from $ table @B
    return b

  a :& b1 :& b2 <- from $ table @A
    `innerJoin` bCte
    `on` (\(a :& b) -> a.k ==. b.k)
    `innerJoin` bCte
    `on` (\(a :& _ :& b2) -> a.k ==. b2.k)
  return (a.k, a.v +. b1.v +. b2.v)

would generated this SQL query

WITH `cte` AS (
  SELECT `b`.`k` AS `v_k`, `b`.`v` AS `v_v`
  FROM `b`
)

SELECT `a`.`k`,  ((`a`.`v` + `cte`.`v_v`) + `cte`.`v_v`)
FROM `a`
INNER JOIN `cte`
  ON `a`.`k` = `cte`.`v_k`
INNER JOIN `cte`
  ON `a`.`k` = `cte`.`v_k`;

which is invalid. After this PR, it instead generates this SQL query:

WITH `cte` AS (
  SELECT `b`.`k` AS `v_k`, `b`.`v` AS `v_v`
  FROM `b`
)

SELECT `a`.`k`,  ((`a`.`v` + `cte2`.`v_v`) + `cte3`.`v_v`)
FROM `a`
INNER JOIN `cte` AS `cte2`
  ON `a`.`k` = `cte2`.`v_k`
INNER JOIN `cte` AS `cte3`
  ON `a`.`k` = `cte3`.`v_k`

I'm not 100% satisfied with this, as the aliasing is only required when a single CTE is used multiple times in a single query, but my PR applies the aliasing at all times. I was not sure how to check for such a situation from within the context of the with function, so perhaps this PR can serve as a conversation about that. Thanks a lot!


After submitting your PR:

belevy commented 10 months ago

please add a test that would fail to work without this fix. As far as aliasing only on duplicates, how do we handle it with regular table joins?

jonathanmoregard commented 9 months ago

please add a test that would fail to work without this fix. As far as aliasing only on duplicates, how do we handle it with regular table joins?

See https://github.com/bitemyapp/esqueleto/issues/372.

I'm pretty sure I've run into the same issue in our production app :)