alamb commented on code in PR #8384:
URL: https://github.com/apache/arrow-datafusion/pull/8384#discussion_r1413071678


##########
datafusion/core/src/dataframe/mod.rs:
##########
@@ -1215,7 +1215,7 @@ impl DataFrame {
     ///   .with_param_values(vec![
     ///      // value at index 0 --> $1
     ///      ScalarValue::from(2i64)
-    ///    ].into())?
+    ///    ])?
     ///   .collect()
     ///   .await?;
     /// assert_batches_eq!(

Review Comment:
   Could you please also add a demonstration of how to use the hashmap version 
too so that it is easier to discover by a casual user? 
   
   Perhaps add this to the example:
   
   ```rust
       /// // Note you can also provide named parameters
       /// let results = ctx
       ///   .sql("SELECT a FROM example WHERE b = $my_param")
       ///   .await?
       ///    // replace $my_param with value 2
       ///    // Note you can also use a HashMap as well
       ///   .with_param_values(vec![
       ///      "my_param",
       ///      ScalarValue::from(2i64)
       ///    ])?
       ///   .collect()
       ///   .await?;
       /// assert_batches_eq!(



##########
datafusion/common/src/param_value.rs:
##########
@@ -126,8 +126,24 @@ impl From<Vec<ScalarValue>> for ParamValues {
     }
 }
 
-impl From<HashMap<String, ScalarValue>> for ParamValues {
-    fn from(value: HashMap<String, ScalarValue>) -> Self {
+impl<K> From<Vec<(K, ScalarValue)>> for ParamValues

Review Comment:
   👌  very nice



##########
datafusion/common/src/param_value.rs:
##########
@@ -126,8 +126,24 @@ impl From<Vec<ScalarValue>> for ParamValues {
     }
 }
 
-impl From<HashMap<String, ScalarValue>> for ParamValues {
-    fn from(value: HashMap<String, ScalarValue>) -> Self {
+impl<K> From<Vec<(K, ScalarValue)>> for ParamValues

Review Comment:
   👌  very nice



##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -981,14 +981,18 @@ impl LogicalPlan {
     /// // Fill in the parameter $1 with a literal 3
     /// let plan = plan.with_param_values(vec![
     ///   ScalarValue::from(3i32) // value at index 0 --> $1
-    /// ].into()).unwrap();
+    /// ]).unwrap();
     ///
     /// assert_eq!("Filter: t1.id = Int32(3)\
     ///             \n  TableScan: t1",
     ///             plan.display_indent().to_string()
     ///  );
     /// ```

Review Comment:
   As above I think it would be great to extend the example to show how to 
handle named parameters



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to