alamb commented on a change in pull request #9596:
URL: https://github.com/apache/arrow/pull/9596#discussion_r584917648



##########
File path: rust/datafusion/src/physical_plan/expressions/binary.rs
##########
@@ -700,9 +702,9 @@ mod tests {
             DataType::UInt32,
             vec![1u32, 2u32],
             Operator::Plus,
-            Int32Array,

Review comment:
       I reviewed the test changes in this file and they make sense to me. 👍 

##########
File path: rust/datafusion/src/physical_plan/expressions/coercion.rs
##########
@@ -44,6 +46,68 @@ pub fn is_numeric(dt: &DataType) -> bool {
         }
 }
 
+/// Get next byte size of the integer number
+fn next_size(size: usize) -> usize {
+    if size < 8_usize {
+        return size * 2;
+    }
+    size
+}
+
+/// Determine if a DataType is float or not
+pub fn is_floating(dt: &DataType) -> bool {
+    matches!(
+        dt,
+        DataType::Float16 | DataType::Float32 | DataType::Float64
+    )
+}
+
+pub fn is_integer(dt: &DataType) -> bool {
+    is_numeric(dt) && !is_floating(dt)
+}
+
+pub fn numeric_byte_size(dt: &DataType) -> Option<usize> {
+    match dt {
+        DataType::Int8 | DataType::UInt8 => Some(1),
+        DataType::Int16 | DataType::UInt16 | DataType::Float16 => Some(2),
+        DataType::Int32 | DataType::UInt32 | DataType::Float32 => Some(4),
+        DataType::Int64 | DataType::UInt64 | DataType::Float64 => Some(8),
+        _ => None,
+    }
+}
+
+pub fn construct_numeric_type(
+    is_signed: bool,
+    is_floating: bool,
+    byte_size: usize,
+) -> Option<DataType> {
+    match (is_signed, is_floating, byte_size) {
+        (false, false, 1) => Some(DataType::UInt8),
+        (false, false, 2) => Some(DataType::UInt16),
+        (false, false, 4) => Some(DataType::UInt32),
+        (false, false, 8) => Some(DataType::UInt64),
+        (false, true, 1) => Some(DataType::Float16),
+        (false, true, 2) => Some(DataType::Float16),
+        (false, true, 4) => Some(DataType::Float32),
+        (false, true, 8) => Some(DataType::Float64),
+        (true, false, 1) => Some(DataType::Int8),
+        (true, false, 2) => Some(DataType::Int16),
+        (true, false, 4) => Some(DataType::Int32),
+        (true, false, 8) => Some(DataType::Int64),
+        (true, true, 1) => Some(DataType::Float32),
+        (true, true, 2) => Some(DataType::Float32),
+        (true, true, 4) => Some(DataType::Float32),
+        (true, true, 8) => Some(DataType::Float64),
+
+        // TODO support bigint and decimal types, now we just let's overflow

Review comment:
       I think this is reasonable 

##########
File path: rust/datafusion/src/physical_plan/expressions/coercion.rs
##########
@@ -44,6 +46,68 @@ pub fn is_numeric(dt: &DataType) -> bool {
         }
 }
 
+/// Get next byte size of the integer number
+fn next_size(size: usize) -> usize {
+    if size < 8_usize {
+        return size * 2;
+    }
+    size
+}
+
+/// Determine if a DataType is float or not
+pub fn is_floating(dt: &DataType) -> bool {

Review comment:
       I think it might be worth putting these functions on  `DataType` itself 
-- `DataType::is_floating(&self)`, and `DataType::numeric_byte_size(&self)` but 
we can always do that in a subsequent PR -- it is not needed for this one




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

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


Reply via email to