Jefffrey commented on code in PR #18032:
URL: https://github.com/apache/datafusion/pull/18032#discussion_r2490361171
##########
datafusion/functions/src/math/power.rs:
##########
@@ -91,58 +194,209 @@ impl ScalarUDFImpl for PowerFunc {
}
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
- match arg_types[0] {
- DataType::Int64 => Ok(DataType::Int64),
- _ => Ok(DataType::Float64),
+ // Ok(arg_types[0].clone())
+ let [data_type, _] = take_function_args(self.name(), arg_types)?;
+ match data_type {
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(data_type) => Ok(d.clone()),
+ // DataType::Decimal32(p, s) => Ok(DataType::Decimal32(*p, *s)),
+ // DataType::Decimal64(p, s) => Ok(DataType::Decimal64(*p, 0)),
+ // DataType::Decimal128(p, s) => Ok(DataType::Decimal128(*p, 0)),
+ // DataType::Decimal256(p, s) => Ok(DataType::Decimal256(*p, 0)),
+ other => exec_err!(
+ "Unsupported data type {other:?} for {} function",
+ self.name()
+ ),
}
}
fn aliases(&self) -> &[String] {
&self.aliases
}
+ fn coerce_types(&self, arg_types: &[DataType]) -> Result<Vec<DataType>> {
+ let [arg1, arg2] = take_function_args(self.name(), arg_types)?;
+
+ fn coerced_type_base(name: &str, data_type: &DataType) ->
Result<DataType> {
+ match data_type {
+ DataType::Null => Ok(DataType::Int64),
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(d) => Ok(d.clone()),
+ other => {
+ exec_err!("Unsupported data type {other:?} for {}
function", name)
+ }
+ }
+ }
+
+ fn coerced_type_exp(name: &str, data_type: &DataType) ->
Result<DataType> {
+ match data_type {
+ DataType::Null => Ok(DataType::Int64),
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(d) => Ok(DataType::Float64),
+ other => {
+ exec_err!("Unsupported data type {other:?} for {}
function", name)
+ }
+ }
+ }
+
+ Ok(vec![
+ coerced_type_base(self.name(), arg1)?,
+ coerced_type_exp(self.name(), arg2)?,
+ ])
+ }
+
fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
- let args = ColumnarValue::values_to_arrays(&args.args)?;
+ let base = &args.args[0].to_array(args.number_rows)?;
+ let exponent = &args.args[1];
- let arr: ArrayRef = match args[0].data_type() {
+ let arr: ArrayRef = match base.data_type() {
+ DataType::Float32 => {
Review Comment:
The arms for float32 & int32 are not reachable based on `coerce_types`,
which coerces them to either float64 or int64
##########
datafusion/functions/src/math/power.rs:
##########
@@ -91,58 +194,209 @@ impl ScalarUDFImpl for PowerFunc {
}
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
- match arg_types[0] {
- DataType::Int64 => Ok(DataType::Int64),
- _ => Ok(DataType::Float64),
+ // Ok(arg_types[0].clone())
+ let [data_type, _] = take_function_args(self.name(), arg_types)?;
+ match data_type {
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(data_type) => Ok(d.clone()),
+ // DataType::Decimal32(p, s) => Ok(DataType::Decimal32(*p, *s)),
+ // DataType::Decimal64(p, s) => Ok(DataType::Decimal64(*p, 0)),
+ // DataType::Decimal128(p, s) => Ok(DataType::Decimal128(*p, 0)),
+ // DataType::Decimal256(p, s) => Ok(DataType::Decimal256(*p, 0)),
+ other => exec_err!(
+ "Unsupported data type {other:?} for {} function",
+ self.name()
+ ),
}
}
fn aliases(&self) -> &[String] {
&self.aliases
}
+ fn coerce_types(&self, arg_types: &[DataType]) -> Result<Vec<DataType>> {
+ let [arg1, arg2] = take_function_args(self.name(), arg_types)?;
+
+ fn coerced_type_base(name: &str, data_type: &DataType) ->
Result<DataType> {
+ match data_type {
+ DataType::Null => Ok(DataType::Int64),
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(d) => Ok(d.clone()),
+ other => {
+ exec_err!("Unsupported data type {other:?} for {}
function", name)
+ }
+ }
+ }
+
+ fn coerced_type_exp(name: &str, data_type: &DataType) ->
Result<DataType> {
+ match data_type {
+ DataType::Null => Ok(DataType::Int64),
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(d) => Ok(DataType::Float64),
+ other => {
+ exec_err!("Unsupported data type {other:?} for {}
function", name)
+ }
+ }
+ }
+
+ Ok(vec![
+ coerced_type_base(self.name(), arg1)?,
+ coerced_type_exp(self.name(), arg2)?,
+ ])
+ }
+
fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
- let args = ColumnarValue::values_to_arrays(&args.args)?;
+ let base = &args.args[0].to_array(args.number_rows)?;
+ let exponent = &args.args[1];
- let arr: ArrayRef = match args[0].data_type() {
+ let arr: ArrayRef = match base.data_type() {
+ DataType::Float32 => {
+ calculate_binary_math::<Float32Type, Float32Type, Float32Type,
_>(
+ base,
+ exponent,
+ |b, e| Ok(f32::powf(b, e)),
+ )?
+ }
DataType::Float64 => {
- let bases = args[0].as_primitive::<Float64Type>();
- let exponents = args[1].as_primitive::<Float64Type>();
- let result = arrow::compute::binary::<_, _, _, Float64Type>(
- bases,
- exponents,
- f64::powf,
- )?;
- Arc::new(result) as _
+ calculate_binary_math::<Float64Type, Float64Type, Float64Type,
_>(
+ &base,
+ exponent,
+ |b, e| Ok(f64::powf(b, e)),
+ )?
+ }
+ DataType::Int32 => {
+ calculate_binary_math::<Int32Type, Int32Type, Int32Type, _>(
+ &base,
+ exponent,
+ |b, e| match e.try_into() {
+ Ok(exp_u32) => b.pow_checked(exp_u32),
+ Err(_) => Err(ArrowError::ArithmeticOverflow(format!(
+ "Exponent {e} in integer computation is out of
bounds."
+ ))),
+ },
+ )?
}
DataType::Int64 => {
- let bases = downcast_named_arg!(&args[0], "base", Int64Array);
- let exponents = downcast_named_arg!(&args[1], "exponent",
Int64Array);
- bases
- .iter()
- .zip(exponents.iter())
- .map(|(base, exp)| match (base, exp) {
- (Some(base), Some(exp)) => Ok(Some(base.pow_checked(
- exp.try_into().map_err(|_| {
- exec_datafusion_err!(
- "Can't use negative exponents: {exp} in
integer computation, please use Float."
- )
- })?,
- ).map_err(|e| arrow_datafusion_err!(e))?)),
- _ => Ok(None),
- })
- .collect::<Result<Int64Array>>()
- .map(Arc::new)? as _
+ calculate_binary_math::<Int64Type, Int64Type, Int64Type, _>(
+ &base,
+ exponent,
+ |b, e| match e.try_into() {
+ Ok(exp_u32) => b.pow_checked(exp_u32),
+ Err(_) => Err(ArrowError::ArithmeticOverflow(format!(
+ "Exponent {e} in integer computation is out of
bounds."
+ ))),
+ },
+ )?
}
+ DataType::Decimal32(precision, scale) => match
exponent.data_type() {
+ DataType::Int64 => rescale_decimal(
+ calculate_binary_math::<Decimal32Type, Int64Type,
Decimal32Type, _>(
+ &base,
+ exponent,
+ |b, e| pow_decimal32_int(b, *scale, e),
+ )?,
+ *precision,
+ *scale,
+ )?,
+ DataType::Float64 => rescale_decimal(
+ calculate_binary_math::<Decimal32Type, Float64Type,
Decimal32Type, _>(
+ &base,
+ exponent,
+ |b, e| pow_decimal32_float(b, *scale, e),
+ )?,
+ *precision,
+ *scale,
+ )?,
+ other => {
+ return exec_err!("Unsupported data type {other:?} for
exponent")
+ }
+ },
+ DataType::Decimal64(precision, scale) => match
exponent.data_type() {
+ DataType::Int64 => rescale_decimal(
+ calculate_binary_math::<Decimal64Type, Int64Type,
Decimal64Type, _>(
+ &base,
+ exponent,
+ |b, e| pow_decimal64_int(b, *scale, e),
+ )?,
+ *precision,
+ *scale,
+ )?,
+ DataType::Float64 => rescale_decimal(
+ calculate_binary_math::<Decimal64Type, Float64Type,
Decimal64Type, _>(
+ &base,
+ exponent,
+ |b, e| pow_decimal64_float(b, *scale, e),
+ )?,
+ *precision,
+ *scale,
+ )?,
+ other => {
+ return exec_err!("Unsupported data type {other:?} for
exponent")
+ }
+ },
+ DataType::Decimal128(precision, scale) => match
exponent.data_type() {
+ DataType::Int64 => rescale_decimal(
+ calculate_binary_math::<Decimal128Type, Int64Type,
Decimal128Type, _>(
+ &base,
+ exponent,
+ |b, e| pow_decimal128_int(b, *scale, e),
+ )?,
+ *precision,
+ *scale,
+ )?,
+ DataType::Float64 => rescale_decimal(
+ calculate_binary_math::<
+ Decimal128Type,
+ Float64Type,
+ Decimal128Type,
+ _,
+ >(&base, exponent, |b, e| {
+ pow_decimal128_float(b, *scale, e)
+ })?,
+ *precision,
+ *scale,
+ )?,
+ other => {
+ return exec_err!("Unsupported data type {other:?} for
exponent")
+ }
+ },
+ DataType::Decimal256(precision, scale) => match
exponent.data_type() {
+ DataType::Int64 => rescale_decimal(
+ calculate_binary_math::<Decimal256Type, Int64Type,
Decimal256Type, _>(
+ &base,
+ exponent,
+ |b, e| pow_decimal256_int(b, *scale, e),
+ )?,
+ *precision,
+ *scale,
+ )?,
+ DataType::Float64 => rescale_decimal(
+ calculate_binary_math::<
Review Comment:
This pattern of rescaling decimals I assume is because the output type of
`calculate_binary_math` can lose the origin precision & scale? We should
encompass this in another utility function since it's repeated multiple times
here
##########
datafusion/functions/src/math/power.rs:
##########
@@ -64,20 +69,118 @@ impl Default for PowerFunc {
impl PowerFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::one_of(
- vec![
- TypeSignature::Exact(vec![Int64, Int64]),
- TypeSignature::Exact(vec![Float64, Float64]),
- ],
- Volatility::Immutable,
- ),
+ signature: Signature::user_defined(Volatility::Immutable),
aliases: vec![String::from("pow")],
}
}
}
+macro_rules! make_pow_fn {
+ ($t:ty, $name_int:ident, $name_float:ident) => {
+ /// Binary function to calculate a math power to integer exponent
+ /// for scaled integer types.
+ ///
+ /// Formula
+ /// The power for a scaled integer `b` is
+ ///
+ /// ```text
+ /// (b*s) ^ e
+ /// ```
+ /// However, the result should be scaled back from scale 0 to scale
`s`,
+ /// which is done by multiplying by `10^s`.
+ /// At the end, the formula is:
+ ///
+ /// ```text
+ /// b^e * 10^(-s * e) * 10^s = b^e / 10^(s * (e-1))
+ /// ```
+ /// Example of 2.5 ^ 4 = 39:
+ /// 2.5 is represented as 25 with scale 1
+ /// The unscaled result is 25^4 = 390625
+ /// Scale it back to 1: 390625 / 10^4 = 39
+ ///
+ /// Returns error if base is invalid
+ fn $name_int(base: $t, scale: i8, exp: i64) -> Result<$t, ArrowError> {
+ let scale: u32 = scale.try_into().map_err(|_| {
+ ArrowError::ArithmeticOverflow(format!(
+ "Unsupported scale value: {scale}"
+ ))
+ })?;
+ if exp == 0 {
+ // Edge case to provide 1 as result (10^s with scale)
+ let result: $t = <$t>::from(10).checked_pow(scale).ok_or(
+ ArrowError::ArithmeticOverflow(format!(
+ "Cannot make unscale factor for {scale} and {exp}"
+ )),
+ )?;
+ return Ok(result);
+ }
+ let exp: u32 = exp.try_into().map_err(|_| {
+ ArrowError::ArithmeticOverflow(format!("Unsupported exp value:
{exp}"))
+ })?;
+ let powered: $t = base.pow_checked(exp).map_err(|_| {
+ ArrowError::ArithmeticOverflow(format!(
+ "Cannot raise base {base} to exp {exp}"
+ ))
+ })?;
+ let unscale_factor: $t = <$t>::from(10)
+ .checked_pow(scale * (exp - 1))
+ .ok_or(ArrowError::ArithmeticOverflow(format!(
+ "Cannot make unscale factor for {scale} and {exp}"
+ )))?;
+
+ powered
+ .checked_div(unscale_factor)
+ .ok_or(ArrowError::ArithmeticOverflow(format!(
+ "Cannot divide in power"
+ )))
+ }
+
+ /// Binary function to calculate a math power to float exponent
+ /// for scaled integer types.
+ /// Returns error if base is negative or non-integer
+ fn $name_float(base: $t, scale: i8, exp: f64) -> Result<$t,
ArrowError> {
+ if !exp.is_finite() || exp.trunc() != exp {
+ return Err(ArrowError::ComputeError(format!(
+ "Cannot use non-integer exp: {exp}"
+ )));
+ }
+ if exp < 0f64 || exp >= u32::MAX as f64 {
+ return Err(ArrowError::ArithmeticOverflow(format!(
+ "Unsupported exp value: {exp}"
+ )));
+ }
+ $name_int(base, scale, exp as i64)
+ }
+ };
+}
+
+// Generate functions for numeric types
+make_pow_fn!(i32, pow_decimal32_int, pow_decimal32_float);
+make_pow_fn!(i64, pow_decimal64_int, pow_decimal64_float);
+make_pow_fn!(i128, pow_decimal128_int, pow_decimal128_float);
+make_pow_fn!(i256, pow_decimal256_int, pow_decimal256_float);
+
+/// Helper function to set precision and scale on a decimal array
+fn rescale_decimal<T>(
+ array: Arc<PrimitiveArray<T>>,
+ precision: u8,
+ scale: i8,
+) -> Result<Arc<PrimitiveArray<T>>>
+where
+ T: DecimalType,
+{
+ if scale < 0 {
+ return exec_err!("Negative scale is not supported for power for
decimal types");
Review Comment:
Is there a specific reason we don't accept negative scale?
##########
datafusion/functions/src/math/power.rs:
##########
@@ -91,58 +194,209 @@ impl ScalarUDFImpl for PowerFunc {
}
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
- match arg_types[0] {
- DataType::Int64 => Ok(DataType::Int64),
- _ => Ok(DataType::Float64),
+ // Ok(arg_types[0].clone())
+ let [data_type, _] = take_function_args(self.name(), arg_types)?;
+ match data_type {
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(data_type) => Ok(d.clone()),
+ // DataType::Decimal32(p, s) => Ok(DataType::Decimal32(*p, *s)),
+ // DataType::Decimal64(p, s) => Ok(DataType::Decimal64(*p, 0)),
+ // DataType::Decimal128(p, s) => Ok(DataType::Decimal128(*p, 0)),
+ // DataType::Decimal256(p, s) => Ok(DataType::Decimal256(*p, 0)),
+ other => exec_err!(
+ "Unsupported data type {other:?} for {} function",
+ self.name()
+ ),
}
}
fn aliases(&self) -> &[String] {
&self.aliases
}
+ fn coerce_types(&self, arg_types: &[DataType]) -> Result<Vec<DataType>> {
+ let [arg1, arg2] = take_function_args(self.name(), arg_types)?;
+
+ fn coerced_type_base(name: &str, data_type: &DataType) ->
Result<DataType> {
+ match data_type {
+ DataType::Null => Ok(DataType::Int64),
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(d) => Ok(d.clone()),
+ other => {
+ exec_err!("Unsupported data type {other:?} for {}
function", name)
+ }
+ }
+ }
+
+ fn coerced_type_exp(name: &str, data_type: &DataType) ->
Result<DataType> {
+ match data_type {
+ DataType::Null => Ok(DataType::Int64),
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(d) => Ok(DataType::Float64),
+ other => {
+ exec_err!("Unsupported data type {other:?} for {}
function", name)
+ }
+ }
+ }
+
+ Ok(vec![
+ coerced_type_base(self.name(), arg1)?,
+ coerced_type_exp(self.name(), arg2)?,
+ ])
+ }
+
fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
- let args = ColumnarValue::values_to_arrays(&args.args)?;
+ let base = &args.args[0].to_array(args.number_rows)?;
+ let exponent = &args.args[1];
- let arr: ArrayRef = match args[0].data_type() {
+ let arr: ArrayRef = match base.data_type() {
+ DataType::Float32 => {
+ calculate_binary_math::<Float32Type, Float32Type, Float32Type,
_>(
+ base,
+ exponent,
+ |b, e| Ok(f32::powf(b, e)),
+ )?
+ }
DataType::Float64 => {
- let bases = args[0].as_primitive::<Float64Type>();
- let exponents = args[1].as_primitive::<Float64Type>();
- let result = arrow::compute::binary::<_, _, _, Float64Type>(
- bases,
- exponents,
- f64::powf,
- )?;
- Arc::new(result) as _
+ calculate_binary_math::<Float64Type, Float64Type, Float64Type,
_>(
+ &base,
+ exponent,
+ |b, e| Ok(f64::powf(b, e)),
+ )?
+ }
+ DataType::Int32 => {
+ calculate_binary_math::<Int32Type, Int32Type, Int32Type, _>(
+ &base,
+ exponent,
+ |b, e| match e.try_into() {
+ Ok(exp_u32) => b.pow_checked(exp_u32),
+ Err(_) => Err(ArrowError::ArithmeticOverflow(format!(
+ "Exponent {e} in integer computation is out of
bounds."
+ ))),
+ },
+ )?
}
DataType::Int64 => {
- let bases = downcast_named_arg!(&args[0], "base", Int64Array);
- let exponents = downcast_named_arg!(&args[1], "exponent",
Int64Array);
- bases
- .iter()
- .zip(exponents.iter())
- .map(|(base, exp)| match (base, exp) {
- (Some(base), Some(exp)) => Ok(Some(base.pow_checked(
- exp.try_into().map_err(|_| {
- exec_datafusion_err!(
- "Can't use negative exponents: {exp} in
integer computation, please use Float."
- )
- })?,
- ).map_err(|e| arrow_datafusion_err!(e))?)),
- _ => Ok(None),
- })
- .collect::<Result<Int64Array>>()
- .map(Arc::new)? as _
+ calculate_binary_math::<Int64Type, Int64Type, Int64Type, _>(
+ &base,
+ exponent,
+ |b, e| match e.try_into() {
+ Ok(exp_u32) => b.pow_checked(exp_u32),
+ Err(_) => Err(ArrowError::ArithmeticOverflow(format!(
+ "Exponent {e} in integer computation is out of
bounds."
+ ))),
+ },
+ )?
}
+ DataType::Decimal32(precision, scale) => match
exponent.data_type() {
+ DataType::Int64 => rescale_decimal(
+ calculate_binary_math::<Decimal32Type, Int64Type,
Decimal32Type, _>(
+ &base,
+ exponent,
+ |b, e| pow_decimal32_int(b, *scale, e),
+ )?,
+ *precision,
+ *scale,
+ )?,
+ DataType::Float64 => rescale_decimal(
+ calculate_binary_math::<Decimal32Type, Float64Type,
Decimal32Type, _>(
+ &base,
+ exponent,
+ |b, e| pow_decimal32_float(b, *scale, e),
+ )?,
+ *precision,
+ *scale,
+ )?,
+ other => {
+ return exec_err!("Unsupported data type {other:?} for
exponent")
+ }
+ },
+ DataType::Decimal64(precision, scale) => match
exponent.data_type() {
Review Comment:
It might be better to match on both instead of having these nested matches,
e.g.
```rust
match (base.data_type(), exponent.data_type()) {
(DataType::Int64, _) => { todo!() },
(DataType::Decimal(p, s), DataType::Int64) => { todo!() },
}
```
##########
datafusion/functions/src/math/power.rs:
##########
@@ -92,8 +195,26 @@ impl ScalarUDFImpl for PowerFunc {
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
Review Comment:
I think this simplification is still valid
##########
datafusion/functions/src/math/power.rs:
##########
@@ -91,58 +194,209 @@ impl ScalarUDFImpl for PowerFunc {
}
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
- match arg_types[0] {
- DataType::Int64 => Ok(DataType::Int64),
- _ => Ok(DataType::Float64),
+ // Ok(arg_types[0].clone())
+ let [data_type, _] = take_function_args(self.name(), arg_types)?;
+ match data_type {
+ d if d.is_floating() => Ok(DataType::Float64),
+ d if d.is_integer() => Ok(DataType::Int64),
+ d if is_decimal(data_type) => Ok(d.clone()),
+ // DataType::Decimal32(p, s) => Ok(DataType::Decimal32(*p, *s)),
+ // DataType::Decimal64(p, s) => Ok(DataType::Decimal64(*p, 0)),
+ // DataType::Decimal128(p, s) => Ok(DataType::Decimal128(*p, 0)),
+ // DataType::Decimal256(p, s) => Ok(DataType::Decimal256(*p, 0)),
+ other => exec_err!(
+ "Unsupported data type {other:?} for {} function",
+ self.name()
+ ),
}
}
fn aliases(&self) -> &[String] {
&self.aliases
}
+ fn coerce_types(&self, arg_types: &[DataType]) -> Result<Vec<DataType>> {
Review Comment:
Something to be concerned about here is does it allow having a base of int64
but exponent of float64 now? I think this was prevented before as it would
either be both int64s or both float64s, but now it seems this coercion is done
independently. I wonder if we have any test cases to catch this however 🤔
##########
datafusion/functions/src/math/power.rs:
##########
@@ -64,20 +69,118 @@ impl Default for PowerFunc {
impl PowerFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::one_of(
- vec![
- TypeSignature::Exact(vec![Int64, Int64]),
- TypeSignature::Exact(vec![Float64, Float64]),
- ],
- Volatility::Immutable,
- ),
+ signature: Signature::user_defined(Volatility::Immutable),
aliases: vec![String::from("pow")],
}
}
}
+macro_rules! make_pow_fn {
Review Comment:
It would be nice if we could do these via generics instead of macros
##########
datafusion/functions/src/math/power.rs:
##########
@@ -64,20 +69,118 @@ impl Default for PowerFunc {
impl PowerFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::one_of(
- vec![
- TypeSignature::Exact(vec![Int64, Int64]),
- TypeSignature::Exact(vec![Float64, Float64]),
- ],
- Volatility::Immutable,
- ),
+ signature: Signature::user_defined(Volatility::Immutable),
aliases: vec![String::from("pow")],
}
}
}
+macro_rules! make_pow_fn {
+ ($t:ty, $name_int:ident, $name_float:ident) => {
+ /// Binary function to calculate a math power to integer exponent
+ /// for scaled integer types.
+ ///
+ /// Formula
+ /// The power for a scaled integer `b` is
+ ///
+ /// ```text
+ /// (b*s) ^ e
+ /// ```
+ /// However, the result should be scaled back from scale 0 to scale
`s`,
+ /// which is done by multiplying by `10^s`.
+ /// At the end, the formula is:
Review Comment:
I'm finding this explanation a little confusing, specifically around `(b*s)
^ e` which implies base is multiplied by scale directly instead of via a power
of 10
##########
datafusion/functions/src/math/power.rs:
##########
@@ -279,4 +522,364 @@ mod tests {
}
}
}
+
+ #[test]
Review Comment:
There's still quite a lot of test code here.
We can use `arrow_cast(column, 'Decimal32(1, 2)')` to get decimal32/64 in
SLTs
##########
datafusion/functions/src/math/power.rs:
##########
@@ -64,20 +69,118 @@ impl Default for PowerFunc {
impl PowerFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::one_of(
- vec![
- TypeSignature::Exact(vec![Int64, Int64]),
- TypeSignature::Exact(vec![Float64, Float64]),
- ],
- Volatility::Immutable,
- ),
+ signature: Signature::user_defined(Volatility::Immutable),
aliases: vec![String::from("pow")],
}
}
}
+macro_rules! make_pow_fn {
+ ($t:ty, $name_int:ident, $name_float:ident) => {
+ /// Binary function to calculate a math power to integer exponent
+ /// for scaled integer types.
+ ///
+ /// Formula
+ /// The power for a scaled integer `b` is
+ ///
+ /// ```text
+ /// (b*s) ^ e
+ /// ```
+ /// However, the result should be scaled back from scale 0 to scale
`s`,
+ /// which is done by multiplying by `10^s`.
+ /// At the end, the formula is:
+ ///
+ /// ```text
+ /// b^e * 10^(-s * e) * 10^s = b^e / 10^(s * (e-1))
+ /// ```
+ /// Example of 2.5 ^ 4 = 39:
+ /// 2.5 is represented as 25 with scale 1
+ /// The unscaled result is 25^4 = 390625
+ /// Scale it back to 1: 390625 / 10^4 = 39
+ ///
+ /// Returns error if base is invalid
+ fn $name_int(base: $t, scale: i8, exp: i64) -> Result<$t, ArrowError> {
+ let scale: u32 = scale.try_into().map_err(|_| {
+ ArrowError::ArithmeticOverflow(format!(
+ "Unsupported scale value: {scale}"
Review Comment:
This is also another check against negative scales?
##########
datafusion/functions/src/math/power.rs:
##########
@@ -64,20 +69,118 @@ impl Default for PowerFunc {
impl PowerFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::one_of(
- vec![
- TypeSignature::Exact(vec![Int64, Int64]),
- TypeSignature::Exact(vec![Float64, Float64]),
- ],
- Volatility::Immutable,
- ),
+ signature: Signature::user_defined(Volatility::Immutable),
aliases: vec![String::from("pow")],
}
}
}
+macro_rules! make_pow_fn {
+ ($t:ty, $name_int:ident, $name_float:ident) => {
+ /// Binary function to calculate a math power to integer exponent
+ /// for scaled integer types.
+ ///
+ /// Formula
+ /// The power for a scaled integer `b` is
+ ///
+ /// ```text
+ /// (b*s) ^ e
+ /// ```
+ /// However, the result should be scaled back from scale 0 to scale
`s`,
+ /// which is done by multiplying by `10^s`.
+ /// At the end, the formula is:
+ ///
+ /// ```text
+ /// b^e * 10^(-s * e) * 10^s = b^e / 10^(s * (e-1))
+ /// ```
+ /// Example of 2.5 ^ 4 = 39:
+ /// 2.5 is represented as 25 with scale 1
+ /// The unscaled result is 25^4 = 390625
+ /// Scale it back to 1: 390625 / 10^4 = 39
+ ///
+ /// Returns error if base is invalid
+ fn $name_int(base: $t, scale: i8, exp: i64) -> Result<$t, ArrowError> {
+ let scale: u32 = scale.try_into().map_err(|_| {
+ ArrowError::ArithmeticOverflow(format!(
+ "Unsupported scale value: {scale}"
+ ))
+ })?;
+ if exp == 0 {
+ // Edge case to provide 1 as result (10^s with scale)
+ let result: $t = <$t>::from(10).checked_pow(scale).ok_or(
+ ArrowError::ArithmeticOverflow(format!(
+ "Cannot make unscale factor for {scale} and {exp}"
+ )),
+ )?;
+ return Ok(result);
+ }
+ let exp: u32 = exp.try_into().map_err(|_| {
+ ArrowError::ArithmeticOverflow(format!("Unsupported exp value:
{exp}"))
+ })?;
+ let powered: $t = base.pow_checked(exp).map_err(|_| {
+ ArrowError::ArithmeticOverflow(format!(
+ "Cannot raise base {base} to exp {exp}"
+ ))
+ })?;
+ let unscale_factor: $t = <$t>::from(10)
+ .checked_pow(scale * (exp - 1))
+ .ok_or(ArrowError::ArithmeticOverflow(format!(
+ "Cannot make unscale factor for {scale} and {exp}"
+ )))?;
+
+ powered
+ .checked_div(unscale_factor)
+ .ok_or(ArrowError::ArithmeticOverflow(format!(
+ "Cannot divide in power"
+ )))
+ }
+
+ /// Binary function to calculate a math power to float exponent
+ /// for scaled integer types.
+ /// Returns error if base is negative or non-integer
Review Comment:
```suggestion
/// Returns error if exponent is negative or non-integer
```
nit
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
