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

Support UPDATE/DELETE .. RETURNING #362

Open ulidtko opened 1 year ago

ulidtko commented 1 year ago

Howdy!

This implements #44 :tada: The aggregate issue #306 is also related.

beginner friendly indeed :ok_hand: was a bit scary at first, but I found a way! Pretty pleased with the result.

Kindly review & merge :pray:


PR template checklist:

After submitting your PR:

ulidtko commented 1 year ago

One more thing... as I read around, UPDATE .. RETURNING isn't unique to Postgres — MariaDB, Oracle, even MSSQL support it too. The implementation in this PR is pretty much DBMS-agnostic as well.

Not sure what to make of that; my usecase is Postgres, so... :shrug:

ulidtko commented 1 year ago

Gee, INSERT also supports RETURNING

ulidtko commented 1 year ago

Changelog rebased after v3.5.10.0 release. @parsonsmatt @ivanbakel please review

ulidtko commented 10 months ago

Nicely wrapping INSERT .. RETURNING as well in this DSL is above my head at this point. :confused:

I'd tried several times; the best I got is a failing test that generates the expected query:

[Debug#SQL] INSERT INTO "Person"("name","age","weight","favNum") VALUES(?,?,?,?) RETURNING "id"; [PersistText "John",PersistInt64 36,PersistNull,PersistInt64 1]

but then result readout fails with UnsupportedSqlInsertIntoType, apparently here:

https://github.com/bitemyapp/esqueleto/blob/e50fed1ddb2f3a6d5f148c5f9148a58517c9d4a8/src/Database/Esqueleto/Internal/Internal.hs#L3302

But then to use the other SqlSelect instance with Insertion a, I'd need to fix its sqlSelectProcessRow, and then somehow combine rawSelectSource and rawEsqueletto to both be passing input data "into" the query, and get the conduit for reading "out" of it.


Factoring in the complete radio silence from maintainers — I won't invest much further effort.

ulidtko commented 10 months ago

Hello @parsonsmatt @9999years would you be so kind to take a look. Review the PR?

Changelog rebased once again, and @since tags fixed, after releases v3.5.10.1 and v3.5.10.2.

ulidtko commented 2 months ago

Ping @parsonsmatt @9999years @arguri @halogenandtoast @isomorpheme @ttuegel

image

parsonsmatt commented 2 months ago

Hey, I'm sorry it's taken me a long time to look at this. I'll review it today.

ulidtko commented 2 months ago

Thanks for review @parsonsmatt :pray:

Easy nits done; I'll work on this some more. Your suggestion does make sense, I see its merits.

One question I got which could use your input: how to implement the INSERT … RETURNING … variant? I've got at the point of building the intended query, but then hit issues trying to combine the output conduit with a way to simultaneously input values into the query. Modulo the (?) placeholders, none of other query types allow to both input & output entire relations at the same time. So I struggled to come up with a way to do it. Any approach you can suggest?

parsonsmatt commented 2 months ago

insertReturning is a good question. I think the overall signature we want is something like:

insertSelectReturning 
    :: (SqlSelect entity x, SqlSelect returning r) =>
     => SqlQuery (SqlExpr (Insertion entity), returning)
    -> ReaderT backend m r

So folks would write:

f :: SqlPersistT m [FooId]
f = do
    insertSelectReturning do
        t <- from $ table @Foo
        pure 
          ( Bar <# (t ^. FooName) <&> (t ^. FooBar)
          , t ^. FooId
          )

I think you can do fmap fst :: SqlQuery (SqlExpr (Insertion a), returning) -> SqlQuery (SqlExpr (Insertion a)) and run that through to generate the main query. Then fmap snd :: SqlQuery (SqlExpr (Insertion a), returning) -> SqlQuery returning, which you'd want to run to get the returning, and then you can construct the returning clause for it.

It may also be better to define a data InsertReturning ins ret = InsertReturning { insertClause :: SqlExpr (Insertion ins)), returningClause :: ret } with it's own SqlSelect instance. Then you'd write pure InsertReturning { insertClause = ... etc } which may be clearer, and provide better documentation/behavior for the actual query generation.

belevy commented 1 month ago

So I just hit a reason to implement insertSelectWithConflictReturning and it led me to want something like SqlQuery specifically for inserts. My original type was SqlInsert insertType returningType, I ended up moving to SqlInsert backend insertType returningType though really it could probably just be monomorphic in SqlBackend.

So my idea for would be something like

data OnConflictDo ent 
  = DoNothing 
  | DoUpdate (SqlExpr (Entity ent) -> SqlExpr (Entity ent) -> [SqlExpr (Entity ent) ->SqlExpr Update])
runInsertReturning :: SqlSelect a r => SqlInsert backend insertType a -> ReaderT backend m [r]
onConflict :: EI.KnowResult uniq ~ Unique ent => uniq -> OnConflictDo ent -> SqlInsert backend ent a -> SqlInsert backend ent a

let doNothing = DoNothing
in insertFrom query 
  & onConflict UniqueField doNothing
  & runInsertReturning
belevy commented 1 month ago

Having worked with this a bit more, the backend can always just be SqlBackend inside of the SqlInsert type. Additionally runInsertReturning should take a projection function to determine the output rather than figuring out how to return an a directly.