Enable Tuple with duplicate keys (#212)

am357 · alancai98 · web-flow · commit 5d2f3b171728 · 2022-11-09T17:19:10.000-08:00
This PR includes the required changes to allow duplicate keys in `Tuple` to conform
with PartiQL definition of `Tuple` type as oppose to `Map` with unique keys.

Co-authored-by: Alan Cai &lt;caialan@amazon.com&gt;
diff --git a/partiql-eval/src/env.rs b/partiql-eval/src/env.rs
@@ -8,10 +8,10 @@ pub trait Bindings<T> {
 impl Bindings<Value> for Tuple {
     fn get(&self, name: &BindingsName) -> Option<&Value> {
         match name {
-            BindingsName::CaseSensitive(s) => self.0.get(s),
+            BindingsName::CaseSensitive(s) => self.get(s),
             BindingsName::CaseInsensitive(s) => {
                 //TODO
-                self.0.get(s)
+                self.get(s)
             }
         }
     }
diff --git a/partiql-eval/src/eval.rs b/partiql-eval/src/eval.rs
@@ -79,25 +79,25 @@ impl EvalScan {
 impl Evaluable for EvalScan {
     fn evaluate(&mut self, ctx: &dyn EvalContext) -> Option<Value> {
         let mut value = partiql_bag![];
-        let v = self.expr.evaluate(&Tuple(HashMap::new()), ctx);
+        let v = self.expr.evaluate(&Tuple::new(), ctx);
         let ordered = &v.is_ordered();
         let mut at_index_counter: i64 = 0;
         if let Some(at_key) = &self.at_key {
             for t in v.into_iter() {
-                let mut out = HashMap::from([(self.as_key.clone(), t)]);
+                let mut out = Tuple::from([(self.as_key.as_str(), t)]);
                 let at_id = if *ordered {
                     at_index_counter.into()
                 } else {
                     Missing
                 };
-                out.insert(at_key.clone(), at_id);
-                value.push(Value::Tuple(Box::new(Tuple(out))));
+                out.insert(&at_key, at_id);
+                value.push(Value::Tuple(Box::new(out)));
                 at_index_counter += 1;
             }
         } else {
             for t in v.into_iter() {
-                let out = HashMap::from([(self.as_key.clone(), t)]);
-                value.push(Value::Tuple(Box::new(Tuple(out))));
+                let out = Tuple::from([(self.as_key.as_str(), t)]);
+                value.push(Value::Tuple(Box::new(out)));
             }
         }
         self.output = Some(Value::Bag(Box::new(value)));
@@ -130,15 +130,15 @@ impl EvalUnpivot {
 
 impl Evaluable for EvalUnpivot {
     fn evaluate(&mut self, ctx: &dyn EvalContext) -> Option<Value> {
-        let result = self.expr.evaluate(&Tuple(HashMap::new()), ctx);
+        let result = self.expr.evaluate(&Tuple::new(), ctx);
         let mut out = vec![];
 
         let tuple = match result {
             Value::Tuple(tuple) => *tuple,
             other => other.coerce_to_tuple(),
         };
 
-        let unpivoted = tuple.0.into_iter().map(|(k, v)| {
+        let unpivoted = tuple.into_iter().map(|(k, v)| {
             Tuple::from([(self.as_key.as_str(), v), (self.at_key.as_str(), k.into())])
         });
 
@@ -233,14 +233,13 @@ impl Evaluable for EvalProject {
             .clone();
         let mut value = partiql_bag![];
         for v in input_value.into_iter() {
-            let out = v.coerce_to_tuple();
-
-            let proj: HashMap<String, Value> = self
-                .exprs
-                .iter()
-                .map(|(alias, expr)| (alias.to_string(), expr.evaluate(&out, ctx)))
-                .collect();
-            value.push(Value::Tuple(Box::new(Tuple(proj))));
+            let v_as_tuple = v.coerce_to_tuple();
+            let mut t = Tuple::new();
+
+            self.exprs.iter().for_each(|(alias, expr)| {
+                t.insert(alias.as_str(), expr.evaluate(&v_as_tuple, ctx));
+            });
+            value.push(Value::Tuple(Box::new(t)));
         }
 
         self.output = Some(Value::Bag(Box::new(value)));
@@ -269,7 +268,7 @@ impl EvalExpr for EvalPath {
         fn path_into(value: Value, path: &EvalPathComponent) -> Value {
             match path {
                 EvalPathComponent::Key(s) => match value {
-                    Value::Tuple(mut tuple) => tuple.0.remove(s).unwrap_or(Missing),
+                    Value::Tuple(mut tuple) => tuple.remove(s).unwrap_or(Missing),
                     _ => Missing,
                 },
                 EvalPathComponent::Index(idx) => match value {
@@ -344,9 +343,7 @@ pub struct EvalVarRef {
 
 impl EvalExpr for EvalVarRef {
     fn evaluate(&self, bindings: &Tuple, ctx: &dyn EvalContext) -> Value {
-        let value = bindings
-            .get(&self.name)
-            .or_else(|| ctx.bindings().get(&self.name));
+        let value = Bindings::get(bindings, &self.name).or_else(|| ctx.bindings().get(&self.name));
         value.map_or(Null, |v| v.clone())
     }
 }
@@ -421,9 +418,9 @@ impl EvalExpr for EvalBinOpExpr {
         #[inline]
         fn short_circuit(op: &EvalBinOp, value: &Value) -> Option<Value> {
             match (op, value) {
-                (EvalBinOp::And, Value::Boolean(false)) => Some(false.into()),
-                (EvalBinOp::Or, Value::Boolean(true)) => Some(true.into()),
-                (_, Value::Missing) => Some(Value::Missing),
+                (EvalBinOp::And, Boolean(false)) => Some(false.into()),
+                (EvalBinOp::Or, Boolean(true)) => Some(true.into()),
+                (_, Missing) => Some(Missing),
                 _ => None,
             }
         }
diff --git a/partiql-eval/src/lib.rs b/partiql-eval/src/lib.rs
@@ -34,13 +34,8 @@ mod tests {
     }
 
     fn data_customer() -> MapBindings<Value> {
-        fn customer_tuple(id: i64, first_name: &str, balance: i64) -> value::Value {
-            Tuple(HashMap::from([
-                ("id".into(), id.into()),
-                ("firstName".into(), first_name.into()),
-                ("balance".into(), balance.into()),
-            ]))
-            .into()
+        fn customer_tuple(id: i64, first_name: &str, balance: i64) -> Value {
+            partiql_tuple![("id", id), ("firstName", first_name), ("balance", balance),].into()
         }
 
         let customer_val = partiql_bag![
@@ -57,10 +52,10 @@ mod tests {
     }
 
     fn data_3_tuple() -> MapBindings<Value> {
-        fn a_tuple(n: i64) -> value::Value {
-            Tuple(HashMap::from([("a".into(), n.into())])).into()
+        fn a_tuple(n: i64) -> Value {
+            partiql_tuple![("a", n)].into()
         }
-        //let data = List(vec![a_tuple(1), a_tuple(2), a_tuple(3)]);
+
         let data = partiql_list![a_tuple(1), a_tuple(2), a_tuple(3)];
 
         let mut bindings = MapBindings::default();
@@ -81,7 +76,7 @@ mod tests {
             at_key: None,
         }));
 
-        let project = plan.add_operator(BindingsExpr::Project(logical::Project {
+        let project = plan.add_operator(Project(logical::Project {
             exprs: HashMap::from([(
                 "result".to_string(),
                 ValueExpr::BinaryExpr(
@@ -103,15 +98,12 @@ mod tests {
         let mut bindings = MapBindings::default();
         bindings.insert(
             "data",
-            partiql_list![Tuple(HashMap::from([("lhs".into(), lhs)]))].into(),
+            partiql_list![Tuple::from([("lhs".into(), lhs)])].into(),
         );
 
         let result = evaluate(plan, bindings).coerce_to_bag();
         assert!(!&result.is_empty());
-        let expected_result = partiql_bag!(Tuple(HashMap::from([(
-            "result".into(),
-            expected_first_elem
-        )])));
+        let expected_result = partiql_bag!(Tuple::from([("result".into(), expected_first_elem)]));
         assert_eq!(expected_result, result);
     }
 
@@ -448,7 +440,7 @@ mod tests {
             at_key: None,
         }));
 
-        let project = lg.add_operator(BindingsExpr::Project(logical::Project {
+        let project = lg.add_operator(Project(logical::Project {
             exprs: HashMap::from([(
                 "b".to_string(),
                 ValueExpr::Path(
@@ -491,7 +483,7 @@ mod tests {
 
         let filter = logical.add_operator(BindingsExpr::Filter(logical::Filter {
             expr: ValueExpr::BinaryExpr(
-                logical::BinaryOp::Gt,
+                BinaryOp::Gt,
                 Box::new(ValueExpr::Path(
                     Box::new(ValueExpr::VarRef(BindingsName::CaseInsensitive(
                         "customer".into(),
@@ -516,7 +508,7 @@ mod tests {
                 (
                     "doubleName".to_string(),
                     ValueExpr::BinaryExpr(
-                        logical::BinaryOp::Concat,
+                        BinaryOp::Concat,
                         Box::new(ValueExpr::Path(
                             Box::new(ValueExpr::VarRef(BindingsName::CaseInsensitive(
                                 "customer".into(),
@@ -544,8 +536,12 @@ mod tests {
             (distinct, sink),
         ]);
 
-        if let Value::Bag(b) = evaluate(logical, data_customer()) {
-            assert_eq!(b.len(), 2);
+        if let Value::Bag(out) = evaluate(logical, data_customer()) {
+            let expected = partiql_bag![
+                partiql_tuple![("firstName", "jason"), ("doubleName", "jasonjason")],
+                partiql_tuple![("firstName", "miriam"), ("doubleName", "miriammiriam")],
+            ];
+            assert_eq!(expected, *out);
         } else {
             panic!("Wrong output")
         }
diff --git a/partiql-value/src/lib.rs b/partiql-value/src/lib.rs

Original file line number	Diff line number	Diff line change
`@@ -8,10 +8,10 @@ pub trait Bindings<T> {`
`8`	`8`	`impl Bindings<Value> for Tuple {`
`9`	`9`	`fn get(&self, name: &BindingsName) -> Option<&Value> {`
`10`	`10`	`match name {`
`11`		`- BindingsName::CaseSensitive(s) => self.0.get(s),`
	`11`	`+ BindingsName::CaseSensitive(s) => self.get(s),`
`12`	`12`	`BindingsName::CaseInsensitive(s) => {`
`13`	`13`	`//TODO`
`14`		`- self.0.get(s)`
	`14`	`+ self.get(s)`
`15`	`15`	`}`
`16`	`16`	`}`
`17`	`17`	`}`