feat: randn expression support

docs/spark_expressions_support.md

@@ -351,6 +351,8 @@
  - [ ] input_file_name
  - [ ] monotonically_increasing_id
  - [ ] raise_error
+ - [x] rand
+ - [x] randn
  - [ ] spark_partition_id
  - [ ] typeof
  - [x] user

native/core/src/execution/planner.rs

@@ -103,7 +103,7 @@ use datafusion_comet_proto::{
 use datafusion_comet_spark_expr::{
     ArrayInsert, Avg, AvgDecimal, Cast, CheckOverflow, Correlation, Covariance, CreateNamedStruct,
     GetArrayStructFields, GetStructField, IfExpr, ListExtract, NormalizeNaNAndZero, RLike,
-    RandExpr, SparkCastOptions, Stddev, StringSpaceExpr, SubstringExpr, SumDecimal,
+    RandExpr, RandnExpr, SparkCastOptions, Stddev, StringSpaceExpr, SubstringExpr, SumDecimal,
     TimestampTruncExpr, ToJson, UnboundColumn, Variance,
 };
 use itertools::Itertools;
@@ -791,8 +791,12 @@ impl PhysicalPlanner {
                 )))
             }
             ExprStruct::Rand(expr) => {
-                let child = self.create_expr(expr.child.as_ref().unwrap(), input_schema)?;
-                Ok(Arc::new(RandExpr::new(child, self.partition)))
+                let seed = expr.seed.wrapping_add(self.partition.into());
+                Ok(Arc::new(RandExpr::new(seed)))
+            }
+            ExprStruct::Randn(expr) => {
+                let seed = expr.seed.wrapping_add(self.partition.into());
+                Ok(Arc::new(RandnExpr::new(seed)))
             }
             expr => Err(GeneralError(format!("Not implemented: {expr:?}"))),
         }

native/proto/src/proto/expr.proto

@@ -80,7 +80,8 @@ message Expr {
     ArrayInsert array_insert = 58;
     MathExpr integral_divide = 59;
     ToPrettyString to_pretty_string = 60;
-    UnaryExpr rand = 61;
+    Rand rand = 61;
+    Randn randn = 62;
   }
 }
 
@@ -415,6 +416,15 @@ message ArrayJoin {
   Expr null_replacement_expr = 3;
 }
 
+message Rand {
+  int64 seed = 1;
+}
+
+message Randn {
+  int64 seed = 1;
+}
+
+
 message DataType {
   enum DataTypeId {
     BOOL = 0;

native/spark-expr/src/nondetermenistic_funcs/internal/mod.rs

@@ -0,0 +1,21 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+mod rand_utils;
+
+pub use rand_utils::evaluate_batch_for_rand;
+pub use rand_utils::StatefulSeedValueGenerator;

native/spark-expr/src/nondetermenistic_funcs/internal/rand_utils.rs

@@ -0,0 +1,60 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use arrow::array::{Float64Array, Float64Builder};
+use datafusion::logical_expr::ColumnarValue;
+use std::ops::Deref;
+use std::sync::{Arc, Mutex};
+
+pub fn evaluate_batch_for_rand<R, S>(
+    state_holder: &Arc<Mutex<Option<S>>>,
+    seed: i64,
+    num_rows: usize,
+) -> datafusion::common::Result<ColumnarValue>
+where
+    R: StatefulSeedValueGenerator<S, f64>,
+    S: Copy,
+{
+    let seed_state = state_holder.lock().unwrap();
+    let mut rnd = R::from_state_ref(seed_state, seed);
+    let mut arr_builder = Float64Builder::with_capacity(num_rows);
+    std::iter::repeat_with(|| rnd.next_value())
+        .take(num_rows)
+        .for_each(|v| arr_builder.append_value(v));
+    let array_ref = Arc::new(Float64Array::from(arr_builder.finish()));
+    let mut seed_state = state_holder.lock().unwrap();
+    seed_state.replace(rnd.get_current_state());
+    Ok(ColumnarValue::Array(array_ref))
+}
+
+pub trait StatefulSeedValueGenerator<State: Copy, Value>: Sized {
+    fn from_init_seed(init_seed: i64) -> Self;
+
+    fn from_stored_state(stored_state: State) -> Self;
+
+    fn next_value(&mut self) -> Value;
+
+    fn get_current_state(&self) -> State;
+
+    fn from_state_ref(state: impl Deref<Target = Option<State>>, init_value: i64) -> Self {
+        if state.is_none() {
+            Self::from_init_seed(init_value)
+        } else {
+            Self::from_stored_state(state.unwrap())
+        }
+    }
+}

native/spark-expr/src/nondetermenistic_funcs/mod.rs

@@ -15,6 +15,9 @@
 // specific language governing permissions and limitations
 // under the License.
 
+pub mod internal;
 pub mod rand;
+pub mod randn;
 
 pub use rand::RandExpr;
+pub use randn::RandnExpr;

native/spark-expr/src/nondetermenistic_funcs/rand.rs

@@ -16,11 +16,11 @@
 // under the License.
 
 use crate::hash_funcs::murmur3::spark_compatible_murmur3_hash;
-use arrow::array::{Float64Array, Float64Builder, RecordBatch};
+
+use crate::internal::{evaluate_batch_for_rand, StatefulSeedValueGenerator};
+use arrow::array::RecordBatch;
 use arrow::datatypes::{DataType, Schema};
 use datafusion::common::Result;
-use datafusion::common::ScalarValue;
-use datafusion::error::DataFusionError;
 use datafusion::logical_expr::ColumnarValue;
 use datafusion::physical_expr::PhysicalExpr;
 use std::any::Any;
@@ -42,21 +42,11 @@ const DOUBLE_UNIT: f64 = 1.1102230246251565e-16;
 const SPARK_MURMUR_ARRAY_SEED: u32 = 0x3c074a61;
 
 #[derive(Debug, Clone)]
-struct XorShiftRandom {
-    seed: i64,
+pub(crate) struct XorShiftRandom {
+    pub(crate) seed: i64,
 }
 
 impl XorShiftRandom {
-    fn from_init_seed(init_seed: i64) -> Self {
-        XorShiftRandom {
-            seed: Self::init_seed(init_seed),
-        }
-    }
-
-    fn from_stored_seed(stored_seed: i64) -> Self {
-        XorShiftRandom { seed: stored_seed }
-    }
-
     fn next(&mut self, bits: u8) -> i32 {
         let mut next_seed = self.seed ^ (self.seed << 21);
         next_seed ^= ((next_seed as u64) >> 35) as i64;
@@ -70,60 +60,43 @@ impl XorShiftRandom {
         let b = self.next(27) as i64;
         ((a << 27) + b) as f64 * DOUBLE_UNIT
     }
+}
 
-    fn init_seed(init: i64) -> i64 {
-        let bytes_repr = init.to_be_bytes();
+impl StatefulSeedValueGenerator<i64, f64> for XorShiftRandom {
+    fn from_init_seed(init_seed: i64) -> Self {
+        let bytes_repr = init_seed.to_be_bytes();
         let low_bits = spark_compatible_murmur3_hash(bytes_repr, SPARK_MURMUR_ARRAY_SEED);
         let high_bits = spark_compatible_murmur3_hash(bytes_repr, low_bits);
-        ((high_bits as i64) << 32) | (low_bits as i64 & 0xFFFFFFFFi64)
+        let init_seed = ((high_bits as i64) << 32) | (low_bits as i64 & 0xFFFFFFFFi64);
+        XorShiftRandom { seed: init_seed }
+    }
+
+    fn from_stored_state(stored_state: i64) -> Self {
+        XorShiftRandom { seed: stored_state }
+    }
+
+    fn next_value(&mut self) -> f64 {
+        self.next_f64()
+    }
+
+    fn get_current_state(&self) -> i64 {
+        self.seed
     }
 }
 
 #[derive(Debug)]
 pub struct RandExpr {
-    seed: Arc<dyn PhysicalExpr>,
-    init_seed_shift: i32,
+    seed: i64,
     state_holder: Arc<Mutex<Option<i64>>>,
 }
 
 impl RandExpr {
-    pub fn new(seed: Arc<dyn PhysicalExpr>, init_seed_shift: i32) -> Self {
+    pub fn new(seed: i64) -> Self {
         Self {
             seed,
-            init_seed_shift,
             state_holder: Arc::new(Mutex::new(None::<i64>)),
         }
     }
-
-    fn extract_init_state(seed: ScalarValue) -> Result<i64> {
-        if let ScalarValue::Int64(seed_opt) = seed.cast_to(&DataType::Int64)? {
-            Ok(seed_opt.unwrap_or(0))
-        } else {
-            Err(DataFusionError::Internal(
-                "unexpected execution branch".to_string(),
-            ))
-        }
-    }
-    fn evaluate_batch(&self, seed: ScalarValue, num_rows: usize) -> Result<ColumnarValue> {
-        let mut seed_state = self.state_holder.lock().unwrap();
-        let mut rnd = if seed_state.is_none() {
-            let init_seed = RandExpr::extract_init_state(seed)?;
-            let init_seed = init_seed.wrapping_add(self.init_seed_shift as i64);
-            *seed_state = Some(init_seed);
-            XorShiftRandom::from_init_seed(init_seed)
-        } else {
-            let stored_seed = seed_state.unwrap();
-            XorShiftRandom::from_stored_seed(stored_seed)
-        };
-
-        let mut arr_builder = Float64Builder::with_capacity(num_rows);
-        std::iter::repeat_with(|| rnd.next_f64())
-            .take(num_rows)
-            .for_each(|v| arr_builder.append_value(v));
-        let array_ref = Arc::new(Float64Array::from(arr_builder.finish()));
-        *seed_state = Some(rnd.seed);
-        Ok(ColumnarValue::Array(array_ref))
-    }
 }
 
 impl Display for RandExpr {
@@ -134,7 +107,7 @@ impl Display for RandExpr {
 
 impl PartialEq for RandExpr {
     fn eq(&self, other: &Self) -> bool {
-        self.seed.eq(&other.seed) && self.init_seed_shift == other.init_seed_shift
+        self.seed.eq(&other.seed)
     }
 }
 
@@ -160,16 +133,15 @@ impl PhysicalExpr for RandExpr {
     }
 
     fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
-        match self.seed.evaluate(batch)? {
-            ColumnarValue::Scalar(seed) => self.evaluate_batch(seed, batch.num_rows()),
-            ColumnarValue::Array(_arr) => Err(DataFusionError::NotImplemented(format!(
-                "Only literal seeds are supported for {self}"
-            ))),
-        }
+        evaluate_batch_for_rand::<XorShiftRandom, i64>(
+            &self.state_holder,
+            self.seed,
+            batch.num_rows(),
+        )
     }
 
     fn children(&self) -> Vec<&Arc<dyn PhysicalExpr>> {
-        vec![&self.seed]
+        vec![]
     }
 
     fn fmt_sql(&self, _: &mut Formatter<'_>) -> std::fmt::Result {
@@ -178,26 +150,22 @@ impl PhysicalExpr for RandExpr {
 
     fn with_new_children(
         self: Arc<Self>,
-        children: Vec<Arc<dyn PhysicalExpr>>,
+        _children: Vec<Arc<dyn PhysicalExpr>>,
     ) -> Result<Arc<dyn PhysicalExpr>> {
-        Ok(Arc::new(RandExpr::new(
-            Arc::clone(&children[0]),
-            self.init_seed_shift,
-        )))
+        Ok(Arc::new(RandExpr::new(self.seed)))
     }
 }
 
-pub fn rand(seed: Arc<dyn PhysicalExpr>, init_seed_shift: i32) -> Result<Arc<dyn PhysicalExpr>> {
-    Ok(Arc::new(RandExpr::new(seed, init_seed_shift)))
+pub fn rand(seed: i64) -> Arc<dyn PhysicalExpr> {
+    Arc::new(RandExpr::new(seed))
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use arrow::array::{Array, BooleanArray, Int64Array};
+    use arrow::array::{Array, Float64Array, Int64Array};
     use arrow::{array::StringArray, compute::concat, datatypes::*};
     use datafusion::common::cast::as_float64_array;
-    use datafusion::physical_expr::expressions::lit;
 
     const SPARK_SEED_42_FIRST_5: [f64; 5] = [
         0.619189370225301,
@@ -212,7 +180,7 @@ mod tests {
         let schema = Schema::new(vec![Field::new("a", DataType::Utf8, true)]);
         let data = StringArray::from(vec![Some("foo"), None, None, Some("bar"), None]);
         let batch = RecordBatch::try_new(Arc::new(schema), vec![Arc::new(data)])?;
-        let rand_expr = rand(lit(42), 0)?;
+        let rand_expr = rand(42);
         let result = rand_expr.evaluate(&batch)?.into_array(batch.num_rows())?;
         let result = as_float64_array(&result)?;
         let expected = &Float64Array::from(Vec::from(SPARK_SEED_42_FIRST_5));
@@ -226,7 +194,7 @@ mod tests {
         let first_batch_data = Int64Array::from(vec![Some(42), None]);
         let second_batch_schema = first_batch_schema.clone();
         let second_batch_data = Int64Array::from(vec![None, Some(-42), None]);
-        let rand_expr = rand(lit(42), 0)?;
+        let rand_expr = rand(42);
         let first_batch = RecordBatch::try_new(
             Arc::new(first_batch_schema),
             vec![Arc::new(first_batch_data)],
@@ -251,23 +219,4 @@ mod tests {
         assert_eq!(final_result, expected);
         Ok(())
     }
-
-    #[test]
-    fn test_overflow_shift_seed() -> Result<()> {
-        let schema = Schema::new(vec![Field::new("a", DataType::Boolean, false)]);
-        let data = BooleanArray::from(vec![Some(true), Some(false)]);
-        let batch = RecordBatch::try_new(Arc::new(schema), vec![Arc::new(data)])?;
-        let max_seed_and_shift_expr = rand(lit(i64::MAX), 1)?;
-        let min_seed_no_shift_expr = rand(lit(i64::MIN), 0)?;
-        let first_expr_result = max_seed_and_shift_expr
-            .evaluate(&batch)?
-            .into_array(batch.num_rows())?;
-        let first_expr_result = as_float64_array(&first_expr_result)?;
-        let second_expr_result = min_seed_no_shift_expr
-            .evaluate(&batch)?
-            .into_array(batch.num_rows())?;
-        let second_expr_result = as_float64_array(&second_expr_result)?;
-        assert_eq!(first_expr_result, second_expr_result);
-        Ok(())
-    }
 }