You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/01/03 23:07:00 UTC
[PR] feat: Add Spark-compatible decimal division [datafusion]
andygrove opened a new pull request, #19628:
URL: https://github.com/apache/datafusion/pull/19628
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
- Part of https://github.com/apache/datafusion/issues/15914
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
This PR adds Spark-compatible decimal division functions to datafusion-spark. These functions implement Spark's specific decimal division semantics which differ from standard SQL:
- Legacy mode behavior: Division by zero returns 0 (not an error)
- Spark rounding: Round half away from zero
- BigInt fallback: When the required precision exceeds Decimal128 limits (scale > 38), the implementation falls back to BigInt arithmetic
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
- New file datafusion/spark/src/function/math/decimal_div.rs containing:
- spark_decimal_div() - Regular decimal division with Spark semantics
- spark_decimal_integral_div() - Integer division (truncates toward zero)
- SparkDecimalDiv and SparkDecimalIntegralDiv UDF structs for use by query planners
- Added num crate dependency to datafusion-spark
- 6 unit tests covering basic division, rounding, division by zero, integral division, negative values, and NULL handling
Note: These are internal functions intended for use by query planners when Spark-compatible decimal division semantics are needed. The precision and scale of the result type are determined at query planning time based on input types, similar to how Comet uses them.
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
New public APIs in datafusion-spark:
- SparkDecimalDiv::new(result_precision, result_scale)
- SparkDecimalIntegralDiv::new(result_precision, result_scale)
- spark_decimal_div() and spark_decimal_integral_div() functions
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #19628:
URL: https://github.com/apache/datafusion/pull/19628#issuecomment-4937823176
Thanks for the reviews and apologies for letting this go stale. Let's close this for now.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove closed pull request #19628: feat: Add Spark-compatible decimal division
URL: https://github.com/apache/datafusion/pull/19628
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #19628:
URL: https://github.com/apache/datafusion/pull/19628#issuecomment-4547748527
This PR has been open for a long time. It is approved and I am trying to clear out the approved PR queue.
Do we want to merge it?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19628:
URL: https://github.com/apache/datafusion/pull/19628#discussion_r2659502642
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Arguments
+/// * `args` - Two ColumnarValue arguments (dividend and divisor)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+/// * `is_integral_div` - If true, performs integer division (truncates result)
+fn spark_decimal_div_internal(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+ is_integral_div: bool,
+) -> Result<ColumnarValue> {
+ assert_eq_or_internal_err!(
+ args.len(),
+ 2,
+ "decimal division expects exactly two arguments"
+ );
+
+ let left = &args[0];
+ let right = &args[1];
+
+ let (left, right): (ArrayRef, ArrayRef) = match (left, right) {
+ (ColumnarValue::Array(l), ColumnarValue::Array(r)) => {
+ (Arc::clone(l), Arc::clone(r))
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Array(r)) => {
+ (l.to_array_of_size(r.len())?, Arc::clone(r))
+ }
+ (ColumnarValue::Array(l), ColumnarValue::Scalar(r)) => {
+ (Arc::clone(l), r.to_array_of_size(l.len())?)
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Scalar(r)) => {
+ (l.to_array()?, r.to_array()?)
+ }
+ };
+
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let l_exp = ((s2 + result_scale + 1) as u32).saturating_sub(s1 as u32);
+ let r_exp = (s1 as u32).saturating_sub((s2 + result_scale + 1) as u32);
+
+ let result: Decimal128Array = if p1 as u32 + l_exp > DECIMAL128_MAX_PRECISION as u32
+ || p2 as u32 + r_exp > DECIMAL128_MAX_PRECISION as u32
+ {
+ // Use BigInt for high precision calculations that would overflow i128
+ let ten = BigInt::from(10);
+ let l_mul = ten.pow(l_exp);
+ let r_mul = ten.pow(r_exp);
+ let five = BigInt::from(5);
+ let zero = BigInt::from(0);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = BigInt::from(l) * &l_mul;
+ let r = BigInt::from(r) * &r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r.eq(&zero) { zero.clone() } else { &l / &r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - &five
+ } else {
+ div + &five
+ } / &ten;
+ Ok(res.to_i128().unwrap_or(i128::MAX))
+ })?
+ } else {
+ // Standard i128 calculation when precision is within bounds
+ let l_mul = 10_i128.pow(l_exp);
+ let r_mul = 10_i128.pow(r_exp);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = l * l_mul;
+ let r = r * r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r == 0 { 0 } else { l / r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - 5
+ } else {
+ div + 5
+ } / 10;
+ Ok(res)
+ })?
+ };
+
+ let result =
+ result.with_data_type(DataType::Decimal128(result_precision, result_scale));
+ Ok(ColumnarValue::Array(Arc::new(result)))
+}
+
+/// Spark-compatible decimal division function.
+///
+/// Performs division with Spark's rounding behavior (round half away from zero).
+pub fn spark_decimal_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ spark_decimal_div_internal(args, result_precision, result_scale, false)
+}
+
+/// Spark-compatible integral decimal division function.
+///
+/// Performs integer division (truncates toward zero).
+pub fn spark_decimal_integral_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ spark_decimal_div_internal(args, result_precision, result_scale, true)
+}
+
+/// SparkDecimalDiv implements the Spark-compatible decimal division function.
+///
+/// This UDF takes the result precision and scale as part of its configuration,
+/// since Spark determines these at query planning time.
+#[derive(Debug)]
+pub struct SparkDecimalDiv {
+ signature: Signature,
+ result_precision: u8,
+ result_scale: i8,
+}
+
+impl PartialEq for SparkDecimalDiv {
Review Comment:
Could we remove these manual impls and use derive?
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
Review Comment:
Should we put a note here that these UDFs are only meant to be used at the physical level instead of the logical? Since it doesn't handle things like type coercion to supported types (decimal128).
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Arguments
+/// * `args` - Two ColumnarValue arguments (dividend and divisor)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+/// * `is_integral_div` - If true, performs integer division (truncates result)
+fn spark_decimal_div_internal(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+ is_integral_div: bool,
+) -> Result<ColumnarValue> {
Review Comment:
```suggestion
fn spark_decimal_div_internal<const IS_INTEGRAL_DIV: bool>(
left: ArrayRef,
right: ArrayRef,
result_precision: u8,
result_scale: i8,
) -> Result<ColumnarValue> {
```
So callers can call like so:
```rust
let arrays = ColumnarValue::values_to_arrays(args.args)?;
let (left, right) = take_function_args("fn_name", arrays)?;
spark_decimal_div_internal::<TRUE>(left, right, precision, scale)
```
So can remove code inside related to checking args length, conversion to arrays, etc.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19628:
URL: https://github.com/apache/datafusion/pull/19628#discussion_r3121307590
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,528 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! **Note:** These UDFs are intended for use at the physical plan level only.
+//! They do not handle type coercion to Decimal128 — callers must ensure
+//! inputs are already Decimal128.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::utils::take_function_args;
+use datafusion_common::{Result, internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Type Parameters
+/// * `IS_INTEGRAL_DIV` - If true, performs integer division (truncates result)
+///
+/// # Arguments
+/// * `left` - The dividend array (must be Decimal128)
+/// * `right` - The divisor array (must be Decimal128)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+fn spark_decimal_div_internal<const IS_INTEGRAL_DIV: bool>(
+ left: &ArrayRef,
+ right: &ArrayRef,
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let scale_sum = s2 as i32 + result_scale as i32 + 1;
+ let l_exp = (scale_sum - s1 as i32).max(0) as u32;
+ let r_exp = (s1 as i32 - scale_sum).max(0) as u32;
+
+ let result: Decimal128Array = if p1 as u32 + l_exp > DECIMAL128_MAX_PRECISION as u32
+ || p2 as u32 + r_exp > DECIMAL128_MAX_PRECISION as u32
+ {
+ // Use BigInt for high precision calculations that would overflow i128
+ let ten = BigInt::from(10);
+ let l_mul = ten.pow(l_exp);
+ let r_mul = ten.pow(r_exp);
+ let five = BigInt::from(5);
+ let zero = BigInt::from(0);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = BigInt::from(l) * &l_mul;
+ let r = BigInt::from(r) * &r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r.eq(&zero) { zero.clone() } else { &l / &r };
+ let res = if IS_INTEGRAL_DIV {
+ div
+ } else if div.is_negative() {
+ div - &five
+ } else {
+ div + &five
+ } / &ten;
+ Ok(res.to_i128().unwrap_or_else(|| {
+ if res.is_negative() {
+ i128::MIN
+ } else {
+ i128::MAX
+ }
+ }))
+ })?
+ } else {
+ // Standard i128 calculation when precision is within bounds
+ let l_mul = 10_i128.pow(l_exp);
+ let r_mul = 10_i128.pow(r_exp);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = l.checked_mul(l_mul).unwrap_or_else(|| {
+ if l.is_negative() {
+ i128::MIN
+ } else {
+ i128::MAX
+ }
+ });
+ let r = r.checked_mul(r_mul).unwrap_or_else(|| {
+ if r.is_negative() {
+ i128::MIN
+ } else {
+ i128::MAX
+ }
+ });
+ // Legacy mode: divide by zero returns 0
+ let div = if r == 0 { 0 } else { l / r };
+ let res = if IS_INTEGRAL_DIV {
+ div
+ } else if div.is_negative() {
+ div - 5
+ } else {
+ div + 5
+ } / 10;
+ Ok(res)
+ })?
+ };
+
+ let result =
+ result.with_data_type(DataType::Decimal128(result_precision, result_scale));
+ Ok(ColumnarValue::Array(Arc::new(result)))
+}
+
+/// Spark-compatible decimal division function.
+///
+/// Performs division with Spark's rounding behavior (round half away from zero).
+pub fn spark_decimal_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ let arrays = ColumnarValue::values_to_arrays(args)?;
+ let [left, right] = take_function_args("spark_decimal_div", arrays)?;
+ spark_decimal_div_internal::<false>(&left, &right, result_precision, result_scale)
+}
+
+/// Spark-compatible integral decimal division function.
+///
+/// Performs integer division (truncates toward zero).
+pub fn spark_decimal_integral_div(
Review Comment:
```suggestion
fn spark_decimal_integral_div(
```
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,528 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! **Note:** These UDFs are intended for use at the physical plan level only.
+//! They do not handle type coercion to Decimal128 — callers must ensure
+//! inputs are already Decimal128.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::utils::take_function_args;
+use datafusion_common::{Result, internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Type Parameters
+/// * `IS_INTEGRAL_DIV` - If true, performs integer division (truncates result)
+///
+/// # Arguments
+/// * `left` - The dividend array (must be Decimal128)
+/// * `right` - The divisor array (must be Decimal128)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+fn spark_decimal_div_internal<const IS_INTEGRAL_DIV: bool>(
+ left: &ArrayRef,
+ right: &ArrayRef,
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let scale_sum = s2 as i32 + result_scale as i32 + 1;
+ let l_exp = (scale_sum - s1 as i32).max(0) as u32;
+ let r_exp = (s1 as i32 - scale_sum).max(0) as u32;
+
+ let result: Decimal128Array = if p1 as u32 + l_exp > DECIMAL128_MAX_PRECISION as u32
+ || p2 as u32 + r_exp > DECIMAL128_MAX_PRECISION as u32
+ {
+ // Use BigInt for high precision calculations that would overflow i128
+ let ten = BigInt::from(10);
+ let l_mul = ten.pow(l_exp);
+ let r_mul = ten.pow(r_exp);
+ let five = BigInt::from(5);
+ let zero = BigInt::from(0);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = BigInt::from(l) * &l_mul;
+ let r = BigInt::from(r) * &r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r.eq(&zero) { zero.clone() } else { &l / &r };
+ let res = if IS_INTEGRAL_DIV {
+ div
+ } else if div.is_negative() {
+ div - &five
+ } else {
+ div + &five
+ } / &ten;
+ Ok(res.to_i128().unwrap_or_else(|| {
+ if res.is_negative() {
+ i128::MIN
+ } else {
+ i128::MAX
+ }
+ }))
+ })?
+ } else {
+ // Standard i128 calculation when precision is within bounds
+ let l_mul = 10_i128.pow(l_exp);
+ let r_mul = 10_i128.pow(r_exp);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = l.checked_mul(l_mul).unwrap_or_else(|| {
+ if l.is_negative() {
+ i128::MIN
+ } else {
+ i128::MAX
+ }
+ });
+ let r = r.checked_mul(r_mul).unwrap_or_else(|| {
+ if r.is_negative() {
+ i128::MIN
+ } else {
+ i128::MAX
+ }
+ });
+ // Legacy mode: divide by zero returns 0
+ let div = if r == 0 { 0 } else { l / r };
+ let res = if IS_INTEGRAL_DIV {
+ div
+ } else if div.is_negative() {
+ div - 5
+ } else {
+ div + 5
+ } / 10;
+ Ok(res)
+ })?
+ };
+
+ let result =
+ result.with_data_type(DataType::Decimal128(result_precision, result_scale));
+ Ok(ColumnarValue::Array(Arc::new(result)))
+}
+
+/// Spark-compatible decimal division function.
+///
+/// Performs division with Spark's rounding behavior (round half away from zero).
+pub fn spark_decimal_div(
Review Comment:
```suggestion
fn spark_decimal_div(
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #19628:
URL: https://github.com/apache/datafusion/pull/19628#discussion_r2668475358
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Arguments
+/// * `args` - Two ColumnarValue arguments (dividend and divisor)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+/// * `is_integral_div` - If true, performs integer division (truncates result)
+fn spark_decimal_div_internal(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+ is_integral_div: bool,
+) -> Result<ColumnarValue> {
+ assert_eq_or_internal_err!(
+ args.len(),
+ 2,
+ "decimal division expects exactly two arguments"
+ );
+
+ let left = &args[0];
+ let right = &args[1];
+
+ let (left, right): (ArrayRef, ArrayRef) = match (left, right) {
+ (ColumnarValue::Array(l), ColumnarValue::Array(r)) => {
+ (Arc::clone(l), Arc::clone(r))
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Array(r)) => {
+ (l.to_array_of_size(r.len())?, Arc::clone(r))
+ }
+ (ColumnarValue::Array(l), ColumnarValue::Scalar(r)) => {
+ (Arc::clone(l), r.to_array_of_size(l.len())?)
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Scalar(r)) => {
+ (l.to_array()?, r.to_array()?)
+ }
+ };
+
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let l_exp = ((s2 + result_scale + 1) as u32).saturating_sub(s1 as u32);
+ let r_exp = (s1 as u32).saturating_sub((s2 + result_scale + 1) as u32);
+
+ let result: Decimal128Array = if p1 as u32 + l_exp > DECIMAL128_MAX_PRECISION as u32
+ || p2 as u32 + r_exp > DECIMAL128_MAX_PRECISION as u32
+ {
+ // Use BigInt for high precision calculations that would overflow i128
+ let ten = BigInt::from(10);
+ let l_mul = ten.pow(l_exp);
+ let r_mul = ten.pow(r_exp);
+ let five = BigInt::from(5);
+ let zero = BigInt::from(0);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = BigInt::from(l) * &l_mul;
+ let r = BigInt::from(r) * &r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r.eq(&zero) { zero.clone() } else { &l / &r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - &five
+ } else {
+ div + &five
+ } / &ten;
+ Ok(res.to_i128().unwrap_or(i128::MAX))
+ })?
+ } else {
+ // Standard i128 calculation when precision is within bounds
+ let l_mul = 10_i128.pow(l_exp);
+ let r_mul = 10_i128.pow(r_exp);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = l * l_mul;
+ let r = r * r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r == 0 { 0 } else { l / r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - 5
+ } else {
+ div + 5
+ } / 10;
+ Ok(res)
+ })?
+ };
+
+ let result =
+ result.with_data_type(DataType::Decimal128(result_precision, result_scale));
+ Ok(ColumnarValue::Array(Arc::new(result)))
+}
+
+/// Spark-compatible decimal division function.
+///
+/// Performs division with Spark's rounding behavior (round half away from zero).
+pub fn spark_decimal_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ spark_decimal_div_internal(args, result_precision, result_scale, false)
+}
+
+/// Spark-compatible integral decimal division function.
+///
+/// Performs integer division (truncates toward zero).
+pub fn spark_decimal_integral_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ spark_decimal_div_internal(args, result_precision, result_scale, true)
+}
+
+/// SparkDecimalDiv implements the Spark-compatible decimal division function.
+///
+/// This UDF takes the result precision and scale as part of its configuration,
+/// since Spark determines these at query planning time.
+#[derive(Debug)]
+pub struct SparkDecimalDiv {
+ signature: Signature,
+ result_precision: u8,
+ result_scale: i8,
+}
+
+impl PartialEq for SparkDecimalDiv {
+ fn eq(&self, other: &Self) -> bool {
+ self.result_precision == other.result_precision
+ && self.result_scale == other.result_scale
+ }
+}
+
+impl Eq for SparkDecimalDiv {}
+
+impl std::hash::Hash for SparkDecimalDiv {
+ fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+ self.result_precision.hash(state);
+ self.result_scale.hash(state);
+ }
+}
+
+impl Default for SparkDecimalDiv {
+ fn default() -> Self {
+ Self::new(38, 18)
+ }
+}
+
+impl SparkDecimalDiv {
+ /// Create a new SparkDecimalDiv with the specified result precision and scale.
+ pub fn new(result_precision: u8, result_scale: i8) -> Self {
+ Self {
+ signature: Signature::new(TypeSignature::Any(2), Volatility::Immutable),
Review Comment:
The signature accepts two parameters of any type.
As far as I can see the arguments' types are "checked" at https://github.com/apache/datafusion/pull/19628/changes#diff-22862a268cd15854f04bd5248f78af375bf9e2d155a7807e6626fb7eeabe8054R87 where it tries to cast them to Decimal128.
Wouldn't it be better to use:
```suggestion
signature: Signature::new(TypeSignature::Exact(vec![DataType::Decimal128, DataType::Decimal128]), Volatility::Immutable),
```
and fail earlier ?
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Arguments
+/// * `args` - Two ColumnarValue arguments (dividend and divisor)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+/// * `is_integral_div` - If true, performs integer division (truncates result)
+fn spark_decimal_div_internal(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+ is_integral_div: bool,
+) -> Result<ColumnarValue> {
+ assert_eq_or_internal_err!(
+ args.len(),
+ 2,
+ "decimal division expects exactly two arguments"
+ );
+
+ let left = &args[0];
+ let right = &args[1];
+
+ let (left, right): (ArrayRef, ArrayRef) = match (left, right) {
+ (ColumnarValue::Array(l), ColumnarValue::Array(r)) => {
+ (Arc::clone(l), Arc::clone(r))
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Array(r)) => {
+ (l.to_array_of_size(r.len())?, Arc::clone(r))
+ }
+ (ColumnarValue::Array(l), ColumnarValue::Scalar(r)) => {
+ (Arc::clone(l), r.to_array_of_size(l.len())?)
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Scalar(r)) => {
+ (l.to_array()?, r.to_array()?)
+ }
+ };
+
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let l_exp = ((s2 + result_scale + 1) as u32).saturating_sub(s1 as u32);
+ let r_exp = (s1 as u32).saturating_sub((s2 + result_scale + 1) as u32);
+
+ let result: Decimal128Array = if p1 as u32 + l_exp > DECIMAL128_MAX_PRECISION as u32
+ || p2 as u32 + r_exp > DECIMAL128_MAX_PRECISION as u32
+ {
+ // Use BigInt for high precision calculations that would overflow i128
+ let ten = BigInt::from(10);
+ let l_mul = ten.pow(l_exp);
+ let r_mul = ten.pow(r_exp);
+ let five = BigInt::from(5);
+ let zero = BigInt::from(0);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = BigInt::from(l) * &l_mul;
+ let r = BigInt::from(r) * &r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r.eq(&zero) { zero.clone() } else { &l / &r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - &five
+ } else {
+ div + &five
+ } / &ten;
+ Ok(res.to_i128().unwrap_or(i128::MAX))
+ })?
+ } else {
+ // Standard i128 calculation when precision is within bounds
+ let l_mul = 10_i128.pow(l_exp);
+ let r_mul = 10_i128.pow(r_exp);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = l * l_mul;
Review Comment:
This could lead to multiplication overflow
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Arguments
+/// * `args` - Two ColumnarValue arguments (dividend and divisor)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+/// * `is_integral_div` - If true, performs integer division (truncates result)
+fn spark_decimal_div_internal(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+ is_integral_div: bool,
+) -> Result<ColumnarValue> {
+ assert_eq_or_internal_err!(
+ args.len(),
+ 2,
+ "decimal division expects exactly two arguments"
+ );
+
+ let left = &args[0];
+ let right = &args[1];
+
+ let (left, right): (ArrayRef, ArrayRef) = match (left, right) {
+ (ColumnarValue::Array(l), ColumnarValue::Array(r)) => {
+ (Arc::clone(l), Arc::clone(r))
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Array(r)) => {
+ (l.to_array_of_size(r.len())?, Arc::clone(r))
+ }
+ (ColumnarValue::Array(l), ColumnarValue::Scalar(r)) => {
+ (Arc::clone(l), r.to_array_of_size(l.len())?)
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Scalar(r)) => {
+ (l.to_array()?, r.to_array()?)
+ }
+ };
+
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let l_exp = ((s2 + result_scale + 1) as u32).saturating_sub(s1 as u32);
+ let r_exp = (s1 as u32).saturating_sub((s2 + result_scale + 1) as u32);
+
+ let result: Decimal128Array = if p1 as u32 + l_exp > DECIMAL128_MAX_PRECISION as u32
+ || p2 as u32 + r_exp > DECIMAL128_MAX_PRECISION as u32
+ {
+ // Use BigInt for high precision calculations that would overflow i128
+ let ten = BigInt::from(10);
+ let l_mul = ten.pow(l_exp);
+ let r_mul = ten.pow(r_exp);
+ let five = BigInt::from(5);
+ let zero = BigInt::from(0);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = BigInt::from(l) * &l_mul;
+ let r = BigInt::from(r) * &r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r.eq(&zero) { zero.clone() } else { &l / &r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - &five
+ } else {
+ div + &five
+ } / &ten;
+ Ok(res.to_i128().unwrap_or(i128::MAX))
+ })?
+ } else {
+ // Standard i128 calculation when precision is within bounds
+ let l_mul = 10_i128.pow(l_exp);
+ let r_mul = 10_i128.pow(r_exp);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = l * l_mul;
+ let r = r * r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r == 0 { 0 } else { l / r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - 5
+ } else {
+ div + 5
+ } / 10;
+ Ok(res)
+ })?
+ };
+
+ let result =
+ result.with_data_type(DataType::Decimal128(result_precision, result_scale));
+ Ok(ColumnarValue::Array(Arc::new(result)))
+}
+
+/// Spark-compatible decimal division function.
+///
+/// Performs division with Spark's rounding behavior (round half away from zero).
+pub fn spark_decimal_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ spark_decimal_div_internal(args, result_precision, result_scale, false)
+}
+
+/// Spark-compatible integral decimal division function.
+///
+/// Performs integer division (truncates toward zero).
+pub fn spark_decimal_integral_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ spark_decimal_div_internal(args, result_precision, result_scale, true)
+}
+
+/// SparkDecimalDiv implements the Spark-compatible decimal division function.
+///
+/// This UDF takes the result precision and scale as part of its configuration,
+/// since Spark determines these at query planning time.
+#[derive(Debug)]
+pub struct SparkDecimalDiv {
+ signature: Signature,
+ result_precision: u8,
+ result_scale: i8,
+}
+
+impl PartialEq for SparkDecimalDiv {
+ fn eq(&self, other: &Self) -> bool {
+ self.result_precision == other.result_precision
+ && self.result_scale == other.result_scale
+ }
+}
+
+impl Eq for SparkDecimalDiv {}
+
+impl std::hash::Hash for SparkDecimalDiv {
+ fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+ self.result_precision.hash(state);
+ self.result_scale.hash(state);
+ }
+}
+
+impl Default for SparkDecimalDiv {
+ fn default() -> Self {
+ Self::new(38, 18)
+ }
+}
+
+impl SparkDecimalDiv {
+ /// Create a new SparkDecimalDiv with the specified result precision and scale.
+ pub fn new(result_precision: u8, result_scale: i8) -> Self {
+ Self {
+ signature: Signature::new(TypeSignature::Any(2), Volatility::Immutable),
+ result_precision,
+ result_scale,
+ }
+ }
+}
+
+impl ScalarUDFImpl for SparkDecimalDiv {
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn name(&self) -> &str {
+ "spark_decimal_div"
+ }
+
+ fn signature(&self) -> &Signature {
+ &self.signature
+ }
+
+ fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
+ Ok(DataType::Decimal128(
+ self.result_precision,
+ self.result_scale,
+ ))
+ }
+
+ fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result<ColumnarValue> {
+ spark_decimal_div(&args.args, self.result_precision, self.result_scale)
+ }
+}
+
+/// SparkDecimalIntegralDiv implements Spark-compatible integral decimal division.
+///
+/// Returns the integer quotient of division (truncates toward zero).
+#[derive(Debug)]
+pub struct SparkDecimalIntegralDiv {
+ signature: Signature,
+ result_precision: u8,
+ result_scale: i8,
+}
+
+impl PartialEq for SparkDecimalIntegralDiv {
+ fn eq(&self, other: &Self) -> bool {
+ self.result_precision == other.result_precision
+ && self.result_scale == other.result_scale
+ }
+}
+
+impl Eq for SparkDecimalIntegralDiv {}
+
+impl std::hash::Hash for SparkDecimalIntegralDiv {
+ fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+ self.result_precision.hash(state);
+ self.result_scale.hash(state);
+ }
+}
+
+impl Default for SparkDecimalIntegralDiv {
+ fn default() -> Self {
+ Self::new(38, 0)
+ }
+}
+
+impl SparkDecimalIntegralDiv {
+ /// Create a new SparkDecimalIntegralDiv with the specified result precision and scale.
+ pub fn new(result_precision: u8, result_scale: i8) -> Self {
+ Self {
+ signature: Signature::new(TypeSignature::Any(2), Volatility::Immutable),
+ result_precision,
+ result_scale,
Review Comment:
Do we need validation here ?
E.g. for `result_precision > DECIMAL128_MAX_PRECISION` and `result_scale.abs() as u8 > result_precision` ?
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Arguments
+/// * `args` - Two ColumnarValue arguments (dividend and divisor)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+/// * `is_integral_div` - If true, performs integer division (truncates result)
+fn spark_decimal_div_internal(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+ is_integral_div: bool,
+) -> Result<ColumnarValue> {
+ assert_eq_or_internal_err!(
+ args.len(),
+ 2,
+ "decimal division expects exactly two arguments"
+ );
+
+ let left = &args[0];
+ let right = &args[1];
+
+ let (left, right): (ArrayRef, ArrayRef) = match (left, right) {
+ (ColumnarValue::Array(l), ColumnarValue::Array(r)) => {
+ (Arc::clone(l), Arc::clone(r))
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Array(r)) => {
+ (l.to_array_of_size(r.len())?, Arc::clone(r))
+ }
+ (ColumnarValue::Array(l), ColumnarValue::Scalar(r)) => {
+ (Arc::clone(l), r.to_array_of_size(l.len())?)
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Scalar(r)) => {
+ (l.to_array()?, r.to_array()?)
+ }
+ };
+
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let l_exp = ((s2 + result_scale + 1) as u32).saturating_sub(s1 as u32);
+ let r_exp = (s1 as u32).saturating_sub((s2 + result_scale + 1) as u32);
+
+ let result: Decimal128Array = if p1 as u32 + l_exp > DECIMAL128_MAX_PRECISION as u32
+ || p2 as u32 + r_exp > DECIMAL128_MAX_PRECISION as u32
+ {
+ // Use BigInt for high precision calculations that would overflow i128
+ let ten = BigInt::from(10);
+ let l_mul = ten.pow(l_exp);
+ let r_mul = ten.pow(r_exp);
+ let five = BigInt::from(5);
+ let zero = BigInt::from(0);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = BigInt::from(l) * &l_mul;
+ let r = BigInt::from(r) * &r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r.eq(&zero) { zero.clone() } else { &l / &r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - &five
+ } else {
+ div + &five
+ } / &ten;
+ Ok(res.to_i128().unwrap_or(i128::MAX))
+ })?
+ } else {
+ // Standard i128 calculation when precision is within bounds
+ let l_mul = 10_i128.pow(l_exp);
+ let r_mul = 10_i128.pow(r_exp);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = l * l_mul;
+ let r = r * r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r == 0 { 0 } else { l / r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - 5
+ } else {
+ div + 5
+ } / 10;
+ Ok(res)
+ })?
+ };
+
+ let result =
+ result.with_data_type(DataType::Decimal128(result_precision, result_scale));
+ Ok(ColumnarValue::Array(Arc::new(result)))
+}
+
+/// Spark-compatible decimal division function.
+///
+/// Performs division with Spark's rounding behavior (round half away from zero).
+pub fn spark_decimal_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ spark_decimal_div_internal(args, result_precision, result_scale, false)
+}
+
+/// Spark-compatible integral decimal division function.
+///
+/// Performs integer division (truncates toward zero).
+pub fn spark_decimal_integral_div(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+) -> Result<ColumnarValue> {
+ spark_decimal_div_internal(args, result_precision, result_scale, true)
+}
+
+/// SparkDecimalDiv implements the Spark-compatible decimal division function.
+///
+/// This UDF takes the result precision and scale as part of its configuration,
+/// since Spark determines these at query planning time.
+#[derive(Debug)]
+pub struct SparkDecimalDiv {
+ signature: Signature,
+ result_precision: u8,
+ result_scale: i8,
+}
+
+impl PartialEq for SparkDecimalDiv {
+ fn eq(&self, other: &Self) -> bool {
+ self.result_precision == other.result_precision
+ && self.result_scale == other.result_scale
+ }
+}
+
+impl Eq for SparkDecimalDiv {}
+
+impl std::hash::Hash for SparkDecimalDiv {
+ fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+ self.result_precision.hash(state);
+ self.result_scale.hash(state);
+ }
+}
+
+impl Default for SparkDecimalDiv {
+ fn default() -> Self {
+ Self::new(38, 18)
+ }
+}
+
+impl SparkDecimalDiv {
+ /// Create a new SparkDecimalDiv with the specified result precision and scale.
+ pub fn new(result_precision: u8, result_scale: i8) -> Self {
+ Self {
+ signature: Signature::new(TypeSignature::Any(2), Volatility::Immutable),
+ result_precision,
+ result_scale,
Review Comment:
Do we need validation here ?
E.g. for `result_precision > DECIMAL128_MAX_PRECISION` and `result_scale.abs() as u8 > result_precision` ?
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Arguments
+/// * `args` - Two ColumnarValue arguments (dividend and divisor)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+/// * `is_integral_div` - If true, performs integer division (truncates result)
+fn spark_decimal_div_internal(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+ is_integral_div: bool,
+) -> Result<ColumnarValue> {
+ assert_eq_or_internal_err!(
+ args.len(),
+ 2,
+ "decimal division expects exactly two arguments"
+ );
+
+ let left = &args[0];
+ let right = &args[1];
+
+ let (left, right): (ArrayRef, ArrayRef) = match (left, right) {
+ (ColumnarValue::Array(l), ColumnarValue::Array(r)) => {
+ (Arc::clone(l), Arc::clone(r))
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Array(r)) => {
+ (l.to_array_of_size(r.len())?, Arc::clone(r))
+ }
+ (ColumnarValue::Array(l), ColumnarValue::Scalar(r)) => {
+ (Arc::clone(l), r.to_array_of_size(l.len())?)
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Scalar(r)) => {
+ (l.to_array()?, r.to_array()?)
+ }
+ };
+
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let l_exp = ((s2 + result_scale + 1) as u32).saturating_sub(s1 as u32);
Review Comment:
With negative scales this sum may still be a negative i8. Casting it to u32 will lead to a big positive number.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #19628:
URL: https://github.com/apache/datafusion/pull/19628#issuecomment-3915633070
Thanks for the reviews @martin-g and @Jefffrey. I just pushed a commit to address feedback.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #19628:
URL: https://github.com/apache/datafusion/pull/19628#discussion_r2668571415
##########
datafusion/spark/src/function/math/decimal_div.rs:
##########
@@ -0,0 +1,434 @@
+// 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.
+
+//! Spark-compatible decimal division functions.
+//!
+//! This module implements Spark's decimal division semantics, which require
+//! special handling for precision and scale that differs from standard SQL.
+//!
+//! # Scale Expansion
+//!
+//! For Decimal(p1, s1) / Decimal(p2, s2) = Decimal(p3, s3):
+//! The dividend needs to be scaled to s2 + s3 + 1 to get correct precision.
+//! This can exceed Decimal128's maximum scale (38), requiring BigInt fallback.
+
+use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array};
+use arrow::datatypes::{DECIMAL128_MAX_PRECISION, DataType, Decimal128Type};
+use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_expr::{
+ ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
+ Volatility,
+};
+use num::{BigInt, Signed, ToPrimitive};
+use std::any::Any;
+use std::sync::Arc;
+
+/// Extract precision and scale from a Decimal128 DataType.
+fn get_precision_scale(data_type: &DataType) -> (u8, i8) {
+ match data_type {
+ DataType::Decimal128(p, s) => (*p, *s),
+ _ => unreachable!("Expected Decimal128 type"),
+ }
+}
+
+/// Internal implementation for both regular and integral decimal division.
+///
+/// # Arguments
+/// * `args` - Two ColumnarValue arguments (dividend and divisor)
+/// * `result_precision` - The precision of the result type
+/// * `result_scale` - The scale of the result type
+/// * `is_integral_div` - If true, performs integer division (truncates result)
+fn spark_decimal_div_internal(
+ args: &[ColumnarValue],
+ result_precision: u8,
+ result_scale: i8,
+ is_integral_div: bool,
+) -> Result<ColumnarValue> {
+ assert_eq_or_internal_err!(
+ args.len(),
+ 2,
+ "decimal division expects exactly two arguments"
+ );
+
+ let left = &args[0];
+ let right = &args[1];
+
+ let (left, right): (ArrayRef, ArrayRef) = match (left, right) {
+ (ColumnarValue::Array(l), ColumnarValue::Array(r)) => {
+ (Arc::clone(l), Arc::clone(r))
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Array(r)) => {
+ (l.to_array_of_size(r.len())?, Arc::clone(r))
+ }
+ (ColumnarValue::Array(l), ColumnarValue::Scalar(r)) => {
+ (Arc::clone(l), r.to_array_of_size(l.len())?)
+ }
+ (ColumnarValue::Scalar(l), ColumnarValue::Scalar(r)) => {
+ (l.to_array()?, r.to_array()?)
+ }
+ };
+
+ let left = left.as_primitive::<Decimal128Type>();
+ let right = right.as_primitive::<Decimal128Type>();
+ let (p1, s1) = get_precision_scale(left.data_type());
+ let (p2, s2) = get_precision_scale(right.data_type());
+
+ // Calculate the scale expansion needed
+ // To get Decimal(p3, s3) from p1/p2, we need to widen s1 to s2 + s3 + 1
+ let l_exp = ((s2 + result_scale + 1) as u32).saturating_sub(s1 as u32);
+ let r_exp = (s1 as u32).saturating_sub((s2 + result_scale + 1) as u32);
+
+ let result: Decimal128Array = if p1 as u32 + l_exp > DECIMAL128_MAX_PRECISION as u32
+ || p2 as u32 + r_exp > DECIMAL128_MAX_PRECISION as u32
+ {
+ // Use BigInt for high precision calculations that would overflow i128
+ let ten = BigInt::from(10);
+ let l_mul = ten.pow(l_exp);
+ let r_mul = ten.pow(r_exp);
+ let five = BigInt::from(5);
+ let zero = BigInt::from(0);
+
+ arrow::compute::kernels::arity::try_binary(left, right, |l, r| {
+ let l = BigInt::from(l) * &l_mul;
+ let r = BigInt::from(r) * &r_mul;
+ // Legacy mode: divide by zero returns 0
+ let div = if r.eq(&zero) { zero.clone() } else { &l / &r };
+ let res = if is_integral_div {
+ div
+ } else if div.is_negative() {
+ div - &five
+ } else {
+ div + &five
+ } / &ten;
+ Ok(res.to_i128().unwrap_or(i128::MAX))
Review Comment:
If `res` is a big negative number that is smaller than i128::MIN then unwrap_or() should return i128::MIN, not MAX
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: Add Spark-compatible decimal division [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #19628:
URL: https://github.com/apache/datafusion/pull/19628#issuecomment-4723580955
Converting to draft as it isn't ready to merge and I am not sure what the plan is and I am trying to clean up the approved / needs merging queue https://github.com/apache/datafusion/pulls?q=is%3Apr+is%3Aopen+review%3Aapproved+-is%3Adraft
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org