matthewmturner commented on a change in pull request #1115:
URL: https://github.com/apache/arrow-rs/pull/1115#discussion_r777169804



##########
File path: arrow/src/compute/kernels/comparison.rs
##########
@@ -888,6 +890,375 @@ pub fn gt_eq_utf8_scalar<OffsetSize: 
StringOffsetSizeTrait>(
     compare_op_scalar!(left, right, |a, b| a >= b)
 }
 
+macro_rules! dyn_compare_scalar {
+    // Applies `LEFT OP RIGHT` when `LEFT` is a `DictionaryArray`
+    ($LEFT: expr, $RIGHT: expr, $OP: ident) => {{
+        let right: i128 = $RIGHT.try_into().map_err(|_| {
+            ArrowError::ComputeError(String::from("Can not convert scalar to 
i128"))
+        })?;
+        match $LEFT.data_type() {
+            DataType::Int8 => {

Review comment:
       its a good question and i had thought about it recently as well but 
expected to address after these were merged.
   
   below is my rough understanding but ill leave it to @alamb to confirm / 
expand on this.
   
   we are leveraging existing kernels, such as `eq_scalar` which take 
`PrimitiveArray` and `ArrowNumericType::Native` which i believe is a 
`ArrowNativeType` 
(https://docs.rs/arrow/latest/arrow/datatypes/trait.ArrowNativeType.html).  
Both `f32` and `f64` impl this trait and of course there can be `Float32` and 
`Float64` `PrimitiveArray`s so i think technically we could (however i dont see 
any tests for using the kernels we are leveraging with decimal values).   right 
now were kind of using a hack though by casting to a `i128` as a catch all and 
then back to the appropriate rust / arrow type to use the attendant kernel.  my 
guess is if we were comfortable casting to `f64` as the catch all instead of 
`i128` then we could simply add support for those data types but i admit to 
being new to the tradeoffs, such as precision, that may arise from using `f64` 
over `i128` so will defer to @alamb.




-- 
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