pacman82 / odbc-safe

A crate for writing odbc clients in safe Rust
MIT License
7 stars 4 forks source link

RFC: Bind RefCell instances instead of bare references #4

Closed adamreichold closed 3 years ago

adamreichold commented 6 years ago

Since binding parameters and columns basically means sharing variables with the ODBC driver in a push/pull model, this change tries to make that sharing explicit via the usage of RefCell so that bindings can be reused, e.g. I can repeatedly execute an INSERT statement that was prepared and bound once after just modifying the content of those RefCell instances.

Of course, this is not really a zero overhead abstraction, but it almost certainly is less overhead the repeatedly destroying and recreating the bindings. If necessary, one could of course keep both mechanisms, i.e. just tracking the lifetimes as it is now with the additional type parameters Params and Cols to track bound RefCell instances. (The type signatures will of course not get prettier due to this.)

One completely open issue is that Rust does not yet support generics over integral constants so that RefCell<[T; N]> cannot really be handled AFAIK, but its erasure as RefCell<[T]> is not well formed as the type parameter becomes unsized. (I could provide a macro that generates a CDataType implementation for [SQLCHAR; $N] given any $N, but that is not a very ergonomic API.)

Having these bindings present would also allow us to check parameter and row set bindings by directly comparing address ranges, hence making it possible to expose them via a safe API.

Note that this includes the changes of #3

adamreichold commented 6 years ago

As another approach which does not have the runtime costs of RefCell and does not suffer from the array-size issue would https://github.com/adamreichold/odbc-safe/tree/keep-bound-refs which keeps and hands out bare references, i.e. after binding let mut x: i32 = 0; using stmt.bind_input_parameter(1, DataType::Integer, &mut x, None), it can be modified via stmt.param(1) which yields &mut i32 by mutably borrowing the statement.

While this is looks lees ergonomic it does avoid any definite overhead as the compiler should theoretically be able to "optimize away" the internal references and it does not force consumer code to utilize RefCell. Of course the downside is that it requires #[feature(specialization)] AFAIU and is therefore restricted to nightly for now. (I though about using TypeId to compare the bound and borrow type in the implementation of the BorrowBInding trait but I guess this would imply runtime overhead as well as it is with RTTI and C++? But at least it would only apply to users of the look back methods...)

pacman82 commented 6 years ago

@adamreichold Hello. First: thanks for your contribution and sorry for not reacting for almost two days. I am very busy now, but I'll have a look at your PR on Saturday.

adamreichold commented 6 years ago

@pacman82 Gladly and please do not worry about reaction time and such. This probably being spare time work for the both of us, I do not see me having any entitlement on your time.

pacman82 commented 6 years ago

@adamreichold I added you as a collaborator. I have little time on my hands to review stuff at the moment. So just merge it if you think it is good enough.

pacman82 commented 3 years ago

Closing this issue, as it hasn't seen any change in over two years. Feel free to reopen.