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

feat: add prepare statement #57

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sunng87
Copy link

@sunng87 sunng87 commented Feb 19, 2023

This patch brings next.jdbc/prepare to babshka-sql-pods

@sunng87 sunng87 marked this pull request as ready for review February 19, 2023 13:03
@borkdude
Copy link
Collaborator

Thanks, I'll check if I can fix CI prior to merging this PR

@borkdude
Copy link
Collaborator

Can you merge with master?

@sunng87 sunng87 force-pushed the feat/prepare branch 6 times, most recently from 9f19c37 to c477fc0 Compare February 20, 2023 08:35
@borkdude
Copy link
Collaborator

@sunng87 I think prepared statements should be saved in the pod side similar to connections.

@sunng87
Copy link
Author

sunng87 commented Feb 21, 2023

Sorry for disturb I thought bb's sql api is same with next.jdbc but it's slightly different. I will find an environment to test my implementation locally later.

@borkdude
Copy link
Collaborator

@sunng87 You can test it locally with a JVM version using this:

POD_DB_TYPE=...  script/test

@sunng87
Copy link
Author

sunng87 commented Feb 26, 2023

Thank you @borkdude, after some experiments I realized this is a little more complex than my thought. Yes as you said we need to cache prepared statement because it cannot be serialized via transit. I will need some time to think about how to manage its lifecycle and provide unstandable api.

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

Successfully merging this pull request may close these issues.

2 participants