[0.16] improve api around binding collections to statements.

[jhgg]

#91 was a "hackfix", but I realized that we indeed do have a CassCollection trait, and we can most definitely use it here, insofar as we can revive set_collection(&mut self, index: usize, collection: impl CassCollection)

This issue tracks that betterment.

[jhgg]

I am wondering if instead we should remove all the bind_{type} functions, and instead just expose a single bind and bind_by_name function.

Minimal example:

trait Bindable {
    #[doc(hidden)]
    fn bind(self, index: usize, statement: &mut Statement) -> Result<()>;
}

impl Statement {
    fn bind(&mut self, index: usize, bindable: impl Bindable) -> Result<&mut self> {
        bindable.bind(index, &mut self)?;
        Ok(self)
    }
}

/// Implement all of these internally
impl Bindable for i8 {
   fn bind(self, index: usize, statement: &mut Statement) -> Result<()> { ... }
}

And now the client side API is:

let mut statement = session.statement("SELECT * FROM foo WHERE id = ?");
statement.bind(0, 5i8)?;
let result = statement.execute().await?;

Thoughts?

Here's a rust playground that shows the concept in action: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a2c4e6387d4fa8ebfe4a91769913c26e

[jhgg]

I'm silly, because we do infact have this via BindRustType 🤔

I wonder if we should just unify around that then... it's silly to have both APIs imo. We should offer a single concise way of doing things for the 1.0 api surface.

[kw217]

Sure, happy to unify around the type-driven approach. I don't think that currently works for collections though, so the original intention of this issue still stands.

[jhgg]

Okay, so I think that we can unify everything underneath a CassValue & CassIndex. The API will shrink, such that:

• CassValue would be a trait implemented for anything that can be be bounded, set, appended to collection, etc...
• CassIndex will be implemented for usize, T: AsRef<str>, to tell us whether to bind by name or index.
• We can add a special CassValue type for Null, (maybe CassNull?)
• Statement would have the following method:
  • fn bind(&mut self, index: impl CassIndex, value: impl CassValue) -> Result<&mut Self>
• UserType will have the following method:
  • fn set(&mut self, index: impl CassIndex, value: impl CassValue) -> Result<&mut Self>
• CassCollection would be implemented for Map, Set, and List, with the new and with_capacity methods.
• Set and List would gain an append (either independently or via some common trait.
  • fn append(&mut self, value: impl CassValue) -> Result<&mut Self>
• Map would not gain an append function, and instead would gain an append_pair function:
  • The rationale behind this is that it's only sane to append pairs to a map, so an individual append API does not make much sense.
  • fn append_pair(&mut self, key: impl CassValue, value: impl CassValue) -> Result<&mut Self>

Example code:

let mut list = List::new();
list.append(1f32)?;
list.append(5f32)?;

let mut map = Map::new();
map.append_pair("key", "value")?;

let mut statement = prepared_statement.bind();
statement.bind(0, list)?;
statement.bind("key_for_map", map)?;
statement.bind("another_value", 5000f32)?;
statement.bind("null_value", CassNull)?;

let result = statement.execute().await?;

I also want to implement std::iter::FromIterator for Set, List, and Map, so you can build collections with them, it'd be nice to be able to (i think outside of this issue though)

let vec = vec![1u32, 2, 3, 4];
let list: List = vec.into_iter().collect()?;

let hash_map = hashmap!{"foo" => 1u32};
let map: Map = hash_map.into_iter().collect()?;

[kw217]

I like this. Can you sketch out all the methods that would be needed on the two traits, just so we can see how it will work out? I'm always a little worried about coherence and intelligible error messages when an API starts to get clever with traits.

You're right of course wrt append_pair.

The design above is missing some type safety though - recall Cassandra collections are homogeneous, but as stated this allows set.append("foo"); set.append(42); set.append(3.14f32);. That might be acceptable but it's not very nice. If we want to prevent this, we will need Set, Map and List to have a type parameter, so append is more like

impl Set<T> where T: CassValue {
  fn append(&mut self, value: T) -> Result<&mut self> { ... }
}

(Tuples are different - they are allowed to be heterogeneous and so they don't need the extra type parameter.)

I totally agree wrt FromIterator - this is a glaring omission. Let's handle in a separate issue.