From 2c5234699939dafc325f6fa382444a622928c4b6 Mon Sep 17 00:00:00 2001
From: Richard Newman <rnewman@twinql.com>
Date: Wed, 7 Jun 2017 11:55:05 -0700
Subject: [PATCH] Review comment: generalize from Uuid SQL arguments to byte
 arrays.

---
 sql/src/lib.rs | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/sql/src/lib.rs b/sql/src/lib.rs
index 270ce0e0..b7b7ab0c 100644
--- a/sql/src/lib.rs
+++ b/sql/src/lib.rs
@@ -24,7 +24,6 @@ use ordered_float::OrderedFloat;
 use mentat_core::{
     ToMicros,
     TypedValue,
-    Uuid,
 };
 
 pub use rusqlite::types::Value;
@@ -101,9 +100,9 @@ pub struct SQLiteQueryBuilder {
 
     // We can't just use an InternSet on the rusqlite::types::Value instances, because that
     // includes f64, so it's not Hash or Eq.
-    // Instead we track UUID and String arguments separately, mapping them to their argument name,
+    // Instead we track byte and String arguments separately, mapping them to their argument name,
     // in order to dedupe. We'll add these to the regular argument vector later.
-    uuid_args: HashMap<Rc<Uuid>, String>,            // From value to argument name.
+    byte_args: HashMap<Vec<u8>, String>,             // From value to argument name.
     string_args: HashMap<Rc<String>, String>,        // From value to argument name.
     args: Vec<(String, Rc<rusqlite::types::Value>)>, // (arg, value).
 }
@@ -119,7 +118,7 @@ impl SQLiteQueryBuilder {
             arg_prefix: prefix,
             arg_counter: 0,
 
-            uuid_args: HashMap::default(),
+            byte_args: HashMap::default(),
             string_args: HashMap::default(),
             args: vec![],
         }
@@ -166,12 +165,13 @@ impl QueryBuilder for SQLiteQueryBuilder {
                 self.push_sql(format!("{}", dt.to_micros()).as_str());      // TODO: argument instead?
             },
             &Uuid(ref u) => {
-                if let Some(arg) = self.uuid_args.get(u).cloned() {        // Why, borrow checker, why?!
+                let bytes = u.as_bytes();
+                if let Some(arg) = self.byte_args.get(bytes.as_ref()).cloned() {        // Why, borrow checker, why?!
                     self.push_named_arg(arg.as_str());
                 } else {
                     let arg = self.next_argument_name();
                     self.push_named_arg(arg.as_str());
-                    self.uuid_args.insert(Rc::new(u.clone()), arg);
+                    self.byte_args.insert(bytes.clone().to_vec(), arg);
                 }
             },
             // These are both `Rc`. Unfortunately, we can't use that fact when
@@ -219,20 +219,18 @@ impl QueryBuilder for SQLiteQueryBuilder {
     }
 
     fn finish(self) -> SQLQuery {
-        // We collected string and UUID arguments into separate maps so that we could
+        // We collected string and byte arguments into separate maps so that we could
         // dedupe them. Now we need to turn them into rusqlite Values.
         let mut args = self.args;
         let string_args = self.string_args.into_iter().map(|(val, arg)| {
              (arg, Rc::new(rusqlite::types::Value::Text(val.as_ref().clone())))
         });
-        let uuid_args = self.uuid_args.into_iter().map(|(val, arg)| {
-            // Get a byte array.
-            let bytes = val.as_bytes().clone();
-            (arg, Rc::new(rusqlite::types::Value::Blob(bytes.to_vec())))
+        let byte_args = self.byte_args.into_iter().map(|(val, arg)| {
+            (arg, Rc::new(rusqlite::types::Value::Blob(val)))
         });
 
         args.extend(string_args);
-        args.extend(uuid_args);
+        args.extend(byte_args);
 
         // Get the args in the right order -- $v0, $v1…
         args.sort_by(|&(ref k1, _), &(ref k2, _)| k1.cmp(k2));