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

odbc-safe layer #54

Closed
pacman82 opened this issue Aug 13, 2017 · 6 comments
Closed

odbc-safe layer #54

pacman82 opened this issue Aug 13, 2017 · 6 comments

Comments

@pacman82
Copy link
Contributor

I am open a GitHub issue since this is probably the best way to have a public design discussion. During the last month I have been busy revisiting some of the design decisions in odbc-rs and did some iterating in a separate repository without worrying to much about breaking interfaces and tests. I did this not with use cases in mind and convenience in mind, but wanted to stay very truthful to the original ODBC C interface. You can have a look at the result of this at odbc-safe. Referring our design discussion in issue #14 [odbc-safe] is currently a merge of layers 2 and 3.

My suggestion is to rewrite odbc-rs in Terms of [odbc-safe]. This should allow us:

  • To write odbc-rs entirely in safe Rust. Making contributions less dangerous and easier to review.
  • Provide users with uses cases which are not supported yet (i.e. How to get blob field data? #48 ), or are add odds with the design decisions made in odbc-rs a layer to fall back on, which is still safe Rust.

However, some breaking changes are likely to occur down this road. I've down little thinking on how exactly an idiomatic odbc library build on top of odbc-safe would look like, but I thought it is about time to discuss this with you and hear your general thoughts about the subject.

@Koka
Copy link
Owner

Koka commented Aug 14, 2017

@pacman82 Thanks, you're doing great things!

The more I think about ODBC in Rust and database access in general, I begin to understand that odbc-rs crate is really need to be something like your odbc-safe crate - it should allow to use ODBC more or less safely and mimic original API very close to not scare ODBC users from other platforms, and nothing more.

All this ergonomic and idiomatic API thing is actually role for some general rust database access layer crate which could use inside various easy switchable db access implementations, like odbc-rs or postgres rust native driver, whatever. For now it looks like Diesel is candidate, but as for me it's too high level and I'd prefer a design like in a popular java jOOQ library - with excessive codegen and ergonomic SQL builder which supports many switchable database backends.

So one way to go now is just to migrate everything from odbc-safe to odbc-rs with major version switch and stop worrying about high level API design, or as you suggest build new generic high level API over odbc-safe but then it tends to expand scope of this library to infinity and beyond, and I'm not sure if we can really do this actually.

What do you think?

@pacman82
Copy link
Contributor Author

All this ergonomic and idiomatic API thing is actually role for some general rust database access layer crate [...]

I think you hit the nail on the head with this one. Binding to ODBC and designing a high level DB API is a bit to ambitious in one take.

So one way to go now is just to migrate everything from odbc-safe to odbc-rs with major version switch and stop worrying about high level API design, or [...]

I think a little incubation phase would be nice to soften the blow for our users. I did create a branch in my fork of the repository and added you as a collaborator. Yet, maybe it is even better to create the branch in your repository. Regarding the major version: I think it is a little to early release a 1.0.0, since I think we will continue to break our interface quite a lot in the near future, before we have all major use cases covered. (We need at the very least one extra lifetime on statements for the bound columns). Although I hope this to be the most dramatic change.

Here is a list of things I suggest we do before we can releasing this branch:

  • Run at least one Appveyor test to the branch (preferable with MSSQL)
  • Call SQLDisconnect for dropped Connections (preferable a solution, which works without unsafe code)
  • Some means of propagating errors for applications which do not want to recover (this will likely need some design. Maybe some helper functions could be placed in an extra crate like odbc-log)

As soon as we feel confident we might start suggesting to our user to try out the new branch.

@pacman82
Copy link
Contributor Author

Or:

Did some thinking yesterday. Maybe a sensible, somewhat higher layer could be achieved. Still not super high level, but not quite as bare bones as odbc-safe. By giving up unusual use cases like:

  • Choosing ODBC Version at runtime without reallocating Environment
  • Reusing previously allocated connection handles to connect to different DataSources
  • Using the same statement handle for prepared and unprepared Queries

A lot of different state transitions could be eliminated, the API Could be made a lot simpler. Some pseudo code:

// Direct transition from Unversioned to Versioned state
let odbc = create_odbc_3_env()?; 
let conn = odbc.connect("Datasource", "User", "Password");
if let Some(result_set) = conn.direct_execution("SELECT * FROM Table"){
   // use result set...
}

Something like this is may be entirely doable.

There are two major caveat though:

  • UCS2 vs Single byte encoding. We probably need both to support Linux as well as windows users. How can we abstract from raw odbc function calls, without keeping the user from this decision? How could we make the decision for the user?
  • While there are great mechanisms in place for propagating errors, warnings have to be dealt with in place. As a first idea we simply could write them to the environment logger, like we are doing now. However this solution relies somewhat on a solution to the first point, since we need to decode the message to log it away.

Other than that, I honestly think that a convenient to use layer on top of odbc-safe is entirely within reach.

@Koka
Copy link
Owner

Koka commented Aug 16, 2017

@pacman82 Looks good to me. We can provide automatic support of non utf8 encodings using rust-encoding crate as AFAIK client encoding can be obtained from connection attributes and rust encoding is utf8, so we can do the conversion - I think most of the time users will prefer to work with Rust strings than vec raw buffers, even for the price of decoding speed. We can support raw buffers too for the rare cases where this performance hit is unacceptable.

And warning messages could be just transcoded to Rust strings, I don't think this could be performance issue for anyone.

@pacman82
Copy link
Contributor Author

@Koka We could also consider using the UCS2 functions ending with W on windows and sticking with utf8 on linux. Maybe we should move the encoding discussion to issue #52?

@Koka
Copy link
Owner

Koka commented Feb 16, 2018

Closing this because odbc-safe is implemented and this discussion lacks activity

@Koka Koka closed this as completed Feb 16, 2018
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