scylladb / cpp-rust-driver

API-compatible rewrite of https://github.com/scylladb/cpp-driver as a wrapper for Rust driver.
GNU Lesser General Public License v2.1
13 stars 11 forks source link

Add extensive information about the driver in README #81

Closed Gor027 closed 1 year ago

Gor027 commented 1 year ago

Pre-review checklist

avelanarius commented 1 year ago

Please also add a section about limitations. Additionally, we should clean up the GitHub Issues that were opened - closing the ones that are now implemented.

Gor027 commented 1 year ago

Sure, I am working on it. I have updated the issues marking functions that are implemented and functions that are still in progress. I will base the limitations section on those issues unless you have another opinion.

Gor027 commented 1 year ago

V2:

Added a separate section about the limitations of some features. More information will be added to this section. For now, it covers the main limitations that the current implementation of the driver's features has.

Lorak-mmk commented 1 year ago

I'd like for the limitations section to include a note that the section itself may be incomplete - unless you are sure that the list is fairly complete?

One thing I'm not sure about: In bindings.rs, the function that checks types is unimplemented: https://github.com/scylladb/cpp-rust-driver/blob/5c009e88c6e3d0021d4318855053b0ef514da42c/scylla-rust-wrapper/src/binding.rs#L53-L56 This I think is used for all binding - and you only noted that we are not checking types for cass_collection_append_* - is that correct?

Gor027 commented 1 year ago

I'd like for the limitations section to include a note that the section itself may be incomplete - unless you are sure that the list is fairly complete?

Sure, I will add a note that the list is incomplete. For now, I have included the main functions which are not implemented or are implemented with some limitations. Functions that are relatively easy to implement (that do not require some implementation in the Rust driver, or maybe even in ScyllaDB) are just noted as unimplemented.

This I think is used for all binding - and you only noted that we are not checking types for cass_collection_append_* - is that correct?

What do you mean by all binding? The compatibility of types is being checked while appending or setting values in collections, tuples and UDTs. For binding a value to a prepared statement, the type is not being checked and in case of binding with incompatible type, the execution of the prepared statement should not be successful.

Lorak-mmk commented 1 year ago

Are you sure that prepared statements are not checked? I did a quick experiment: https://gist.github.com/Lorak-mmk/dbce7e0484b69000f8523df4ad963531

And for me it prints:

===== Using optimized driver!!! =====
bind 1 id: 0
bind 1 somestring: 0
execute 1: 0
bind 2 id: 0
bind 2 somestring: 16777229
execute 2: 0
DONE!

Which would mean that they are checked. After reading abstract_data.hh it also seems that the types should be checked for prepared statement.

Gor027 commented 1 year ago

Which would mean that they are checked. After reading abstract_data.hh it also seems that the types should be checked for prepared statement.

Yeah, I guess you are right. The UserTypeValue and Statement classes extend from AbstractData class, so setting value by index or name should also check the type compatibility. Will fix the description.

Edit: The Rust driver, too, does not validate the types passed to queries here, however, we can add the validation of types independently from the Rust driver.

Lorak-mmk commented 1 year ago

There is already a placeholder function for that, it's just not implemented yet - always returns true. See bindings.rs:53-56