Skip to content

Add/enhance MySQL support #864

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

Merged
merged 2 commits into from
Dec 12, 2022
Merged

Add/enhance MySQL support #864

merged 2 commits into from
Dec 12, 2022

Conversation

etehtsea
Copy link
Contributor

@etehtsea etehtsea commented Oct 27, 2022

Rebased #777 against current master, fixed a few conflicts/incompatibilities.
@itowlson, will be happy to rebase against your branch if you prefer to rebase/sign it yourself.

  • Extract outbound-mysql into separate crate
  • Add integration test
  • Add connections reusing during the request
  • Improve MySQL types conversion

@etehtsea etehtsea force-pushed the db-mysql branch 4 times, most recently from 573a12a to 0cf234b Compare November 1, 2022 16:52
@etehtsea etehtsea marked this pull request as ready for review November 1, 2022 16:53
@etehtsea
Copy link
Contributor Author

@itowlson this one is also ready for review!

Copy link
Collaborator

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

Thank you! This looks great and I'm 100% happy with it.

But...

I want to hold off a bit on this because I think we need to have a couple of broader discussions:

  1. Do we want to invest more in this API for databases, or do we want to switch to a more resource-oriented approach which better models providers, connections, transactions, etc., and could have a unified-ish interface instead of multiple parallel ones?

  2. How do we manage new host components in a way that doesn't require big-bang updates to client, SDK and cloud? That's not technically a blocker for this because it's purely additive, but it makes me wary of throwing in new interfaces (and yeah we want to do key-value stores soon so maybe I should just suck it up for another few months).

I hope this makes sense. We can always kick the can down the road: there's nothing about this PR that makes it a specific sticking point. So if you or anyone is looking to use this, we can go ahead. But otherwise I'd like to broaden the discussion a bit first...

@itowlson
Copy link
Collaborator

itowlson commented Dec 8, 2022

@etehtsea I tagged you in on the kick-off draft of wasi-sql. At the moment, what they have there is close enough to what you have here that we could consider pushing ahead on this basis. Or we could try to align more closely, though I'm not sure if the resource stuff is properly available yet, and the draft will certainly change. What do you reckon?

@etehtsea
Copy link
Contributor Author

etehtsea commented Dec 8, 2022

@itowlson thanks! I left a comment there. As you said earlier, if there is somebody interested in MySQL support, it should be good to go (for now).

I was curious about the differences between pg and MySQL implementations. And my conclusion is if the goal is to provide more or less full support for specific DB, it's hard to do this using a shared wit interface between them (though it's not covered in this PR).

P.S. Also, I've checked wasi preview2 today and it seems that sockets support is somewhere near. So, if there be an ability to use the "native" library from the guest module, it probably be the solution for those who needs full DB support.

itowlson and others added 2 commits December 8, 2022 23:41
Co-authored-by: Konstantin Shabanov <[email protected]>
Signed-off-by: Konstantin Shabanov <[email protected]>
- Extract outbound-mysql into separate crate
- Add integration test
- Add connections reusing during the request
- Improve MySQL types conversion

Signed-off-by: Konstantin Shabanov <[email protected]>
@itowlson
Copy link
Collaborator

@radu-matei (and anyone else interested in this!) Despite earlier reservations (expressed upthread), I'd like to merge this. The interface is well aligned with the current wasi-sql proposal, and I think comparative feedback from users of Postgres and MySQL would help inform the discussions there. It does, of course, mean another interface to maintain and, eventually, deprecate as wasi-sql evolves (and as the goals for it become clearer).

Let me know if you object to going ahead with this.

@itowlson itowlson mentioned this pull request Dec 12, 2022
@itowlson itowlson merged commit 8dfc4d6 into spinframework:main Dec 12, 2022
@etehtsea etehtsea deleted the db-mysql branch February 8, 2023 07:11
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