Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slick/JDBC: Support PreparedStatement use in Java DSL #2318

Merged
merged 1 commit into from
Jun 22, 2020

Conversation

ihostage
Copy link
Contributor

Extend Java DSL for using java.sql.PreparedStatement

References #2247

@ennru
Copy link
Member

ennru commented May 26, 2020

Thank you for suggesting this, it's a great improvement for the Java DSL.

I wonder if there is a way to create the prepared statement only once and update it for every element.

Maybe @renatocaval can have a look?

@ihostage
Copy link
Contributor Author

I wonder if there is a way to create the prepared statement only once and update it for every element.

Yep, I thought about that too. But PreparedStatement bound to Connection and It's a really problem for us 😞
https://stackoverflow.com/questions/4647476/reusing-of-a-preparedstatement-between-methods

Reuse PreparedStatement very easy, when we have a loop and one connection 😄 But it's not our case 🤷‍♂️
Besides, the user can use an external Map<Connection, PreparedStatement> for storing statements and reuse them if it really wants to do that.

Copy link
Member

@octonato octonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good.

I dropped some comments, will continue after lunch.

docs/src/main/paradox/slick.md Outdated Show resolved Hide resolved
private def toDBIO[T](javaDml: JBiFunction[T, Connection, PreparedStatement]): T => DBIO[Int] = { t =>
SimpleJdbcAction { ctx =>
val statement = javaDml(t, ctx.connection)
if (statement != null) statement.executeUpdate() else 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This becomes a hidden filter feature, but makes sense as a safe-guard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I thought about NPE-safety and I haven’t come up with anything better like this 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it becomes a dangerous mix with the failure handling you suggested in the example...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this check, rewritten examples in docs, and added test with recover function.
Please, review.

@ihostage ihostage force-pushed the issues/2247 branch 2 times, most recently from dc67c48 to 20fc83a Compare June 18, 2020 18:46
Extend Java DSL for using `java.sql.PreparedStatement`
Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great to finally make this usable from Java!

@ennru ennru added this to the 2.0.2 milestone Jun 22, 2020
@ennru ennru changed the title #2247 Relational database connectivity. (partially) Slick/JDBC: Support PreparedStatement use in Java DSL Jun 22, 2020
@ennru ennru merged commit 269276e into akka:master Jun 22, 2020
@ennru
Copy link
Member

ennru commented Jun 22, 2020

Thank you @ihostage!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants