ocaml-community / ocaml-mariadb

OCaml bindings to MariaDB, supporting the nonblocking API
55 stars 18 forks source link

`Res.affected_rows` not available where it's needed #14

Closed paurkedal closed 6 years ago

paurkedal commented 6 years ago

I think there is a design flaw in the way mysql_stmt_affected_rows is exposed. Stmt.execute returns None : Res.t option for INSERT or UPDATE statements, making Res.affected_rows unavailable. I can see the logic of placing this function there, though. I guess the options are to move the function to Stmt.t or synthesising a Res.t with num_rows = 0 and a fetch consistent with it?

andrenth commented 6 years ago

I think the idea was to not allow num_rows to be called after an INSERT or UPDATE because it doesn't make sense, and I probably added affected_rows later and didn't consider the None result.

I think I'll simply return a Res.t from Stmt.execute and get rid of the option type.

andrenth commented 6 years ago

Commit 59f30f6 does that. Please tell me what you think.

paurkedal commented 6 years ago

Yes, I think this was the nicest option. Will the release be 0.10.0 (so I get my version constraints right)?

andrenth commented 6 years ago

Yep, 0.10.0. I'll publish soon.

On Wed, Nov 29, 2017 at 4:38 PM, Petter Urkedal notifications@github.com wrote:

Yes, I think this was the nicest option. Will the release be 0.10.0 (so I get my version constraints right)?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/andrenth/ocaml-mariadb/issues/14#issuecomment-347954871, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB2BTxWLKYHu2mEy2JzS4-SWMHxeeLjks5s7aS6gaJpZM4QuA7I .

paurkedal commented 6 years ago

Great, thanks!

andrenth commented 6 years ago

Pull request submitted to opam-repository.