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

Return from MysqlShim methods instead of passing writers? #30

Open
Kinrany opened this issue Dec 29, 2021 · 1 comment
Open

Return from MysqlShim methods instead of passing writers? #30

Kinrany opened this issue Dec 29, 2021 · 1 comment

Comments

@Kinrany
Copy link

Kinrany commented Dec 29, 2021

Currently MysqlShim methods on_prepare, on_execute and on_query return Result<(), Self::Error> and use a writer value to write data.

This requires the methods and the trait itself to depend on the generic W: io::Write parameter. However none of the writer method signatures depend on W. So it should be possible to perform the operations that depend on W outside of the trait's implementations.

The first thing that comes to mind is returning values from the methods instead of passing them to writers. This would work well for on_prepare and on_execute since the writers are supposed to be only used once there as far as I can tell.

on_query is harder since it feeds values one by one. I imagine this must work for big queries where it doesn't make sense to collect a single value in memory before returning.

One solution is to return an async stream, but that makes the rest of the crate depend on an async runtime and probably means replacing all of the std::io stuff.

Another is to provide a non-generic channel-like writer that merely accepts values. Not sure if there are any caveats there.

@jonhoo
Copy link
Owner

jonhoo commented Dec 31, 2021

Hey!

As you're onto, the reason for the W is about allocations/buffering — if we required the trait methods to return (owned) values, then implementations would have no choice but to allocate and probably copy data so that ownership can be relinquished. With the W, the implementation can instead write directly to the output buffers, avoiding the extra allocation + copy, which is definitely worth it for larger responses.

That said, we may be able to improve upon the situation by storing an Box<dyn Write> rather than be generic over W. It's not ideal, because we'd lose monomorphization and the ability to, say, only expose certain methods if W also implements some other trait, but it would get rid of the generic. Whether that's worth it, I'm not sure. Are you running into any particular problem with W being a generic?

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

No branches or pull requests

2 participants