Repository: cassandra-rs/cassandra-rs (language: rust)

Issue:
Make execute_batch not consume its argument
We're implementing some retry logic at the moment, and we've hit a challenge that I think is caused by a simple oversight in the binding. `execute_batch` is declared as follows (https://github.com/Metaswitch/cassandra-rs/blob/master/src/cassandra/session.rs#L129):
```rust
pub fn execute_batch(&self, batch: Batch) -> CassFuture<CassResult>
```
i.e., it consumes `Batch`. There is nothing in either its implementation or in the C++ docs at https://docs.datastax.com/en/developer/cpp-driver/2.16/api/struct.CassSession/#function-cass_session_execute_batch that suggests this should be the case, and `execute` doesn't consume its `Statement`. So I think this is just a mistake. Similarly for `execute_batch_with_payload`.

Does anyone have any objections to me making this change? It would be a breaking API change, but a very easy one to fix (just add `&` at the callsite).



Discussion / hints:
@jhgg keen to have your thoughts on this one?
I think that passing by reference might have triggered some odd lifetime woes across cffi bounds. I don't remember specifically, but might as well try it and see. 

Fix patch:
diff --git a/.travis.yml b/.travis.yml
index d32bf42..be3935b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -26,11 +26,6 @@ before_install:
   - sudo docker pull cassandra
   - sudo docker run -d --net=host --name=cassandra cassandra
 
-before_script:
-- |
-  pip install 'travis-cargo<0.2' --user &&
-  export PATH=$HOME/.local/bin:/usr/local/bin:$PATH
-
 # Enforce that all new commits are signed off according to the DCO,
 # per CONTRIBUTING.md. Prior commits are either pre-fork, are signed off,
 # or were made by Keith Wansbrough, who hereby certifies the DCO with regard
@@ -47,5 +42,3 @@ script:
   cargo build --all &&
   cargo test -- --test-threads 1
 
-after_success:
-- travis-cargo coveralls --no-sudo
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9ee5005..1c62a15 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,18 @@ version number is tracked in the file `VERSION`.
 ## [Unreleased]
 ### Added
 ### Changed
+- Changed `Session::execute_batch` and `Session::execute_batch_with_payloads` to take only 
+  a reference to `Batch` rather than consuming it.
+
+  This is a breaking change; to update your code, simply change `batch` to `&batch`
+  in your argument list. If this causes an error `future cannot be sent between threads safely`
+  because `&Batch` is `used across an await`, you need to introduce a `let` before the `await`
+  as follows:
+  ```
+  let fut = session.execute_batch(&batch);
+  let result = fut.await?
+  ```
+  
 ### Fixed
 
 ## [0.16.0] - 2021-03-10
diff --git a/src/cassandra/session.rs b/src/cassandra/session.rs
index 4618baa..a9b7723 100644
--- a/src/cassandra/session.rs
+++ b/src/cassandra/session.rs
@@ -126,7 +126,7 @@ impl Session {
     //    }
 
     /// Execute a batch statement.
-    pub fn execute_batch(&self, batch: Batch) -> CassFuture<CassResult> {
+    pub fn execute_batch(&self, batch: &Batch) -> CassFuture<CassResult> {
         <CassFuture<CassResult>>::build(unsafe {
             cass_session_execute_batch(self.0, batch.inner())
         })
@@ -135,7 +135,7 @@ impl Session {
     /// Execute a batch statement and get any custom payloads from the response.
     pub fn execute_batch_with_payloads(
         &self,
-        batch: Batch,
+        batch: &Batch,
     ) -> CassFuture<(CassResult, CustomPayloadResponse)> {
         <CassFuture<(CassResult, CustomPayloadResponse)>>::build(unsafe {
             cass_session_execute_batch(self.0, batch.inner())


Test patch:
diff --git a/tests/batch.rs b/tests/batch.rs
index a3b348d..4b4eb1d 100644
--- a/tests/batch.rs
+++ b/tests/batch.rs
@@ -26,7 +26,7 @@ fn insert_into_batch_with_prepared(
             Err(err) => panic!("{:?}", err),
         }
     }
-    session.execute_batch(batch).wait()?;
+    session.execute_batch(&batch).wait()?;
     Ok(prepared)
 }
 
diff --git a/tests/logging.rs b/tests/logging.rs
index c2ef626..e87f766 100644
--- a/tests/logging.rs
+++ b/tests/logging.rs
@@ -50,6 +50,7 @@ fn test_logging() {
     let log_output: String = drain.0.lock().unwrap().clone();
     assert!(
         log_output.contains("Unable to resolve address for absolute-gibberish.invalid"),
+        "{}",
         log_output
     );
 }
diff --git a/tests/paging.rs b/tests/paging.rs
index 85f384e..f3b24e3 100644
--- a/tests/paging.rs
+++ b/tests/paging.rs
@@ -52,7 +52,7 @@ fn select_from_paging(session: &Session) -> Result<Vec<(String, String)>> {
                     println!("key: '{:?}' value: '{:?}'", &key_str, &value);
                     res.push((key_str, value));
                 }
-                Err(err) => panic!(err),
+                Err(err) => panic!("{}", err),
             }
         }
         has_more_pages = result.has_more_pages();