You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "pchintar (via GitHub)" <gi...@apache.org> on 2026/06/01 12:25:00 UTC
[PR] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
pchintar opened a new pull request, #22697:
URL: https://github.com/apache/datafusion/pull/22697
## Which issue does this PR close?
- Closes #22696 .
## Rationale for this change
`round()` should not change integer values when the scale is non-negative, since no fractional digits need to be rounded.
Currently, core `round()` coerces large `Int64` values through `Float64`, causing precision loss:
```sql
SELECT round(arrow_cast(9007199254740993, 'Int64'));
````
Before/Current Buggy Output:
```text
9007199254740992.0
```
Expected:
```text
9007199254740993
```
The Spark-compatible `round()` also fails for `UInt64` values above `i64::MAX` even when the scale is non-negative:
```sql
SELECT round(arrow_cast(18446744073709551615, 'UInt64'));
```
Before/Current Buggy Output:
```text
round: UInt64 value 18446744073709551615 exceeds i64::MAX and cannot be rounded
```
## What changes are included in this PR?
* Preserve integer inputs in core `round()` for non-negative scales instead of routing them through `Float64`.
* Preserve `UInt64` values in Spark-compatible `round()` when the scale is non-negative, avoiding unnecessary `UInt64 -> i64` conversion.
* Add SQLLogicTest coverage for:
* `Int64` values above `2^53` in core `round()`.
* `UInt64::MAX` in Spark-compatible `round()`.
* Both one-argument and two-argument forms.
## Are these changes tested?
Yes.
```bash
cargo fmt --all
git diff --check
cargo test -p datafusion-sqllogictest --test sqllogictests -- spark/math/round.slt
cargo test -p datafusion-functions round
cargo test -p datafusion-spark round
```
I also verified the core regression queries manually:
```sql
SELECT arrow_typeof(round(arrow_cast(9007199254740993, 'Int64'))),
round(arrow_cast(9007199254740993, 'Int64'));
```
After:
```text
Int64 9007199254740993
```
```sql
SELECT arrow_typeof(round(arrow_cast(9007199254740993, 'Int64'), 2)),
round(arrow_cast(9007199254740993, 'Int64'), 2);
```
After:
```text
Int64 9007199254740993
```
## Are there any user-facing changes?
No.
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4763167995
Hi @Jefffrey I'm v.sorry for this late reply as I was on vacation last week actually.
So you had provided me 5 suggestions earlier, I've addressed the 3 cleanup suggestions out of those 5:
* replaced the custom integer-type helper with `DataType::is_integer()`
* restored the original `UInt64` path in the Spark implementation, since Spark doesn't expose a `UInt64` type
* simplified `round_integer_scalar()` to match on `(value, return_type)` directly, removing the separate input/output matches while keeping the same checked conversions and overflow behavior
For the remaining two points though:
* I kept the shared `i128` intermediate intentionally. It allows all integer types to use the same negative-scale rounding logic and then perform a checked conversion back to the original type, so overflow is detected consistently without duplicating the implementation for every integer width.
* I also kept `ArrowError` in the rounding helpers. The array path goes through `calculate_binary_math`, whose callback is defined in terms of `ArrowError`, so keeping the same error type there avoids adding a separate conversion layer. The scalar path follows the same pattern for consistency.
I hope this reply helps clarify, thnx again!
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4646948462
Hi @kumarUjjawal , I've added tests for the overflow case, `UInt64` values, and rounding to a place larger than the value itself, in scalar.slt
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4826146037
thank you @Jefffrey
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on code in PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#discussion_r3347672444
##########
datafusion/functions/src/math/round.rs:
##########
@@ -468,6 +487,11 @@ fn round_columnar(
let decimal_places_is_array = matches!(decimal_places, ColumnarValue::Array(_));
let arr: ArrayRef = match (value_array.data_type(), return_type) {
Review Comment:
Hi @kumarUjjawal I've restored the fast path for non-negative scalar `decimal_places` values while still using `round_integer_array` for negative and column-valued `decimal_places`.
Before:
```rust
(input_type, return_type)
if input_type == return_type && is_integer_data_type(return_type) =>
{
round_integer_array(value_array.as_ref(), decimal_places, return_type)?
}
```
Now:
```rust
(input_type, return_type)
if input_type == return_type && is_integer_data_type(return_type) =>
{
match decimal_places {
ColumnarValue::Scalar(ScalarValue::Int32(Some(dp))) if *dp >= 0 => {
value_array
}
_ => round_integer_array(
value_array.as_ref(),
decimal_places,
return_type,
)?,
}
}
```
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4729499848
Hi @Jefffrey just a kind reminder, could you pls approve this PR if everything looks fine, if not pls lmk, thanks again.
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4834522920
Thanks @Jefffrey and @pchintar
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4600980156
@kumarUjjawal Thnx for the review!
I've updated the implementation so that integer inputs now stay as integer types instead of being converted through `Float64`. This preserves large `Int64` values that cannot be represented exactly as `Float64`.
I also added tests for the cases you mentioned:
* negative `decimal_places` values (for example, `round(125, -1)`)
* column-valued `decimal_places`
* large `Int64` values above the exact `Float64` range
Additionally, I updated the affected sqllogictest expectations since the plan output changed after removing the unnecessary cast to `Float64`.
I've also rerun the relevant tests and manually verified all these cases.
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on code in PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#discussion_r3347682694
##########
datafusion/functions/src/math/round.rs:
##########
@@ -468,6 +487,11 @@ fn round_columnar(
let decimal_places_is_array = matches!(decimal_places, ColumnarValue::Array(_));
let arr: ArrayRef = match (value_array.data_type(), return_type) {
Review Comment:
@kumarUjjawal could you kindly pls re-run the checks now? thnx
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4657077318
Hi @Jefffrey I replaced the custom integer-type helper with `DataType::is_integer()` and restored the original `UInt64` handling in the Spark implementation, since Spark doesn't expose a `UInt64` type. I also reran the existing tests and manually verified the `Int64`/`UInt64` rounding cases to make sure the behavior is unchanged.
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4815849512
I feel the i128 conversion could be avoided with some generic trickery 🤔
For reference, I updated the [benchmarks](https://github.com/apache/datafusion/blob/main/datafusion/functions/benches/round.rs) to have an i64 bench with scale 2 and with scale -2, and ran on this branch:
```
round size=1024/round_f64_array
time: [813.88 ns 819.69 ns 823.61 ns]
change: [−2.9895% −2.2717% −1.7347%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 10 measurements (20.00%)
1 (10.00%) low severe
1 (10.00%) high mild
round size=1024/round_i64_array
time: [104.98 ns 105.65 ns 106.55 ns]
change: [−2.5935% −1.3028% −0.0545%] (p = 0.07 > 0.05)
No change in performance detected.
Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) high severe
round size=1024/round_i64_array_neg_scale
time: [4.4552 µs 4.4583 µs 4.4623 µs]
Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) high severe
round size=4096/round_f64_array
time: [2.7175 µs 2.7288 µs 2.7357 µs]
Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) low severe
round size=4096/round_i64_array
time: [105.02 ns 105.41 ns 105.90 ns]
Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) high mild
round size=4096/round_i64_array_neg_scale
time: [17.140 µs 17.144 µs 17.149 µs]
round size=8192/round_f64_array
time: [5.6102 µs 5.6347 µs 5.6583 µs]
round size=8192/round_i64_array
time: [104.78 ns 105.08 ns 105.37 ns]
round size=8192/round_i64_array_neg_scale
time: [34.582 µs 34.604 µs 34.627 µs]
```
- round_i64_array = scale 2, expected pass through fast since no work needed
- round_i64_array_neg_scale = scale -2, needs to go through i128 path
And to test, I modified the code to avoid the i128 conversion and just operate directly on i64:
```
round size=1024/round_f64_array
time: [810.04 ns 814.87 ns 817.61 ns]
change: [−1.4862% −0.5891% +0.2634%] (p = 0.21 > 0.05)
No change in performance detected.
Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) low severe
round size=1024/round_i64_array
time: [105.65 ns 105.78 ns 105.92 ns]
change: [−0.7381% +0.1219% +0.7794%] (p = 0.79 > 0.05)
No change in performance detected.
round size=1024/round_i64_array_neg_scale
time: [2.3741 µs 2.3780 µs 2.3824 µs]
change: [−46.758% −46.661% −46.557%] (p = 0.00 < 0.05)
Performance has improved.
round size=4096/round_f64_array
time: [2.6846 µs 2.6904 µs 2.6962 µs]
change: [−1.7566% −1.4088% −0.9277%] (p = 0.00 < 0.05)
Change within noise threshold.
round size=4096/round_i64_array
time: [106.03 ns 106.31 ns 106.67 ns]
change: [+0.3251% +0.8590% +1.4019%] (p = 0.01 < 0.05)
Change within noise threshold.
Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) high mild
round size=4096/round_i64_array_neg_scale
time: [8.9223 µs 8.9313 µs 8.9400 µs]
change: [−47.965% −47.905% −47.852%] (p = 0.00 < 0.05)
Performance has improved.
round size=8192/round_f64_array
time: [5.5491 µs 5.5718 µs 5.5949 µs]
change: [−1.6684% −1.1164% −0.5111%] (p = 0.00 < 0.05)
Change within noise threshold.
round size=8192/round_i64_array
time: [109.97 ns 110.49 ns 111.02 ns]
change: [+4.5883% +5.1438% +5.7191%] (p = 0.00 < 0.05)
Performance has regressed.
round size=8192/round_i64_array_neg_scale
time: [17.872 µs 17.946 µs 18.034 µs]
change: [−48.360% −48.138% −47.920%] (p = 0.00 < 0.05)
Performance has improved.
```
So the i128 conversion does seem to have a noticeable performance impact.
We can always try to optimize in a followup if that is preference, just want to highlight this
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #22697:
URL: https://github.com/apache/datafusion/pull/22697
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4601706498
@kumarUjjawal This failure seems to be unrelated to my code.
It failed before running DataFusion tests because GitHub Actions could not pull the Docker image:
```text
docker pull amd64/rust
Get "https://registry-1.docker.io/v2/": context deadline exceeded
```
So this is a CI infrastructure/network failure, not a test failure perhaps?
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on code in PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#discussion_r3347124062
##########
datafusion/functions/src/math/round.rs:
##########
@@ -468,6 +487,11 @@ fn round_columnar(
let decimal_places_is_array = matches!(decimal_places, ColumnarValue::Array(_));
let arr: ArrayRef = match (value_array.data_type(), return_type) {
Review Comment:
Please restore a fast path, when decimal_places is a non-negative scalar literal and `input_type == return_type`, return `value_array directly`, only fall into `round_integer_array `for negative/array `decimal_places`.
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4729663180
Some of my comments haven't been addressed, could you please check them?
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4601716303
> @kumarUjjawal This failure seems to be unrelated to my code.
>
> It failed before running DataFusion tests because GitHub Actions could not pull the Docker image:
>
> ```
> docker pull amd64/rust
> Get "https://registry-1.docker.io/v2/": context deadline exceeded
> ```
>
> So this is a CI infrastructure/network failure, not a test failure perhaps?
Yeah ignore this, it's Github issue
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4601744407
> > Yeah ignore this, it's Github issue
>
> So wdyt of my PR? is it good enough to be approved? or is it too early to tell?
Will review tomorrow!
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4812064998
Hi @Jefffrey could you kindly provide any update on this?
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4646387683
@pchintar let's add some tests for:
- overflow-on-negative-scale case
- unsigned 64-bit values
- Rounding to a place larger than the number itself
-
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4637354195
Hi @Jefffrey could you pls check this PR and lmk if possible? thnx
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on code in PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#discussion_r3339113024
##########
datafusion/functions/src/math/round.rs:
##########
@@ -467,7 +506,36 @@ fn round_columnar(
&& matches!(decimal_places, ColumnarValue::Scalar(_));
let decimal_places_is_array = matches!(decimal_places, ColumnarValue::Array(_));
+ macro_rules! round_integer_array_to_float64 {
+ ($ARRAY_TYPE:ty) => {{
+ let result = calculate_binary_math::<$ARRAY_TYPE, Int32Type, Float64Type, _>(
+ value_array.as_ref(),
+ decimal_places,
+ |v, dp| round_float(v as f64, dp),
+ )?;
+ result as _
+ }};
+ }
+
let arr: ArrayRef = match (value_array.data_type(), return_type) {
+ (input_type, return_type)
Review Comment:
The no-op fast path requires `decimal_places` to be a non-negative scalar literal. When the scale is a *column* (or a negative literal), this guard is false and there's no other integer arm, so it hits `exec_err!`
```SQL
CREATE TABLE t(v BIGINT, dp INT) AS VALUES (125,1),(125,-1);
SELECT round(v, dp) FROM t; -- errored; worked (Float64) before this PR
```
##########
datafusion/functions/src/math/round.rs:
##########
@@ -308,6 +324,29 @@ impl ScalarUDFImpl for RoundFunc {
};
match (value_scalar, args.return_type()) {
+ (value_scalar, return_type)
+ if is_integer_data_type(return_type) && dp >= 0 =>
Review Comment:
only covers `dp >= 0`. For `dp < 0` with an integer value, needs handler for negative scale
##########
datafusion/functions/src/math/round.rs:
##########
@@ -308,6 +324,29 @@ impl ScalarUDFImpl for RoundFunc {
};
match (value_scalar, args.return_type()) {
+ (value_scalar, return_type)
+ if is_integer_data_type(return_type) && dp >= 0 =>
+ {
+ ColumnarValue::Scalar(value_scalar.clone()).cast_to(return_type, None)
+ }
+ (value_scalar, Float64)
+ if is_integer_data_type(&value_scalar.data_type()) =>
+ {
+ let value = ColumnarValue::Scalar(value_scalar.clone())
+ .cast_to(&Float64, None)?;
+ match value {
+ ColumnarValue::Scalar(ScalarValue::Float64(Some(v))) => {
+ let rounded = round_float(v, dp)?;
+ Ok(ColumnarValue::Scalar(ScalarValue::Float64(Some(rounded))))
+ }
+ ColumnarValue::Scalar(ScalarValue::Float64(None)) => {
+ Ok(ColumnarValue::Scalar(ScalarValue::Float64(None)))
+ }
+ _ => internal_err!(
+ "Unexpected datatype after casting integer argument to Float64"
+ ),
+ }
+ }
Review Comment:
This `(value_scalar, Float64) if is_integer_data_type(value)` arm and the`round_integer_array_to_float64!` macro + the eight `(IntN, Float64)` array arms are unreachable:
##########
datafusion/functions/src/math/round.rs:
##########
@@ -245,6 +260,7 @@ impl ScalarUDFImpl for RoundFunc {
// extra precision to accommodate potential carry-over.
let return_type =
match input_type {
+ input_type if is_integer_data_type(input_type) => input_type.clone(),
Review Comment:
returns the integer type for *all* scales, but only non-negative scales are handled downstream. `round(arrow_cast(125,'Int64'), -1)` now fails, it returned `130.0` before this PR.
##########
datafusion/sqllogictest/test_files/scalar.slt:
##########
Review Comment:
tests only cover non-negative scalar scale. There's no coverage for negative scale, column scale, or negative scale on a column
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4599394568
cc @neilconway
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4601734058
> Yeah ignore this, it's Github issue
So wdyt of my PR? is it good enough to be approved? or is it too early to tell?
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4647021969
> @pchintar let's add some tests for:
>
> * overflow-on-negative-scale case
> * unsigned 64-bit values
> * Rounding to a place larger than the number itself
Hi @kumarUjjawal , I've added tests for all three cases:
* overflow with `Int64::MAX` and negative scale
* `UInt64::MAX` with one-arg and two-arg `round`
* rounding `125` to a larger place (`-5`)
I also verified the queries manually.
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4817818572
Hi @Jefffrey I updated my integer rounding implementation to remove the `i128` conversion in the integer path. Instead, it now performs the rounding using native integer arithmetic through `PrimInt` together with `checked_pow`, while keeping separate signed and unsigned rounding logic so the existing overflow and tie-breaking behavior is preserved.
I then reran the function tests, Spark tests, and SQL logic tests, and also manually rechecked the large `Int64`, `UInt64`, overflow, negative-scale, and scale-larger-than-value cases to verify that the output is correct.
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#discussion_r3377961775
##########
datafusion/spark/src/function/math/round.rs:
##########
@@ -462,18 +462,22 @@ fn spark_round(args: &[ColumnarValue], enable_ansi_mode: bool) -> Result<Columna
impl_integer_array_round!(array, UInt32Type, scale, enable_ansi_mode)
}
DataType::UInt64 => {
- let array = array.as_primitive::<UInt64Type>();
- let result: PrimitiveArray<UInt64Type> = array.try_unary(|x| {
- let v_i64 = i64::try_from(x).map_err(|_| {
- (exec_err!(
- "round: UInt64 value {x} exceeds i64::MAX and cannot be rounded"
- ) as Result<(), _>)
- .unwrap_err()
Review Comment:
i wonder if we should be having this codepath in the first place, considering spark doesnt have a u64 type 🤔
##########
datafusion/functions/src/math/round.rs:
##########
@@ -630,6 +663,180 @@ fn round_columnar(
}
}
+fn round_integer_value(value: i128, decimal_places: i32) -> Result<i128, ArrowError> {
+ if decimal_places >= 0 {
+ return Ok(value);
+ }
+
+ let Some(factor) = 10_i128.checked_pow(decimal_places.unsigned_abs()) else {
+ return Ok(0);
+ };
+
+ let remainder = value % factor;
+ let threshold = factor / 2;
+
+ if remainder >= threshold {
+ value
+ .checked_sub(remainder)
+ .and_then(|v| v.checked_add(factor))
+ .ok_or_else(|| {
+ ArrowError::ComputeError("Overflow while rounding integer".to_string())
+ })
+ } else if remainder <= -threshold {
+ value
+ .checked_sub(remainder)
+ .and_then(|v| v.checked_sub(factor))
+ .ok_or_else(|| {
+ ArrowError::ComputeError("Overflow while rounding integer".to_string())
+ })
+ } else {
+ value.checked_sub(remainder).ok_or_else(|| {
+ ArrowError::ComputeError("Overflow while rounding integer".to_string())
+ })
+ }
+}
+
+fn round_integer_scalar(
+ value: &ScalarValue,
+ return_type: &DataType,
+ decimal_places: i32,
+) -> Result<ColumnarValue> {
+ let rounded = match value {
+ ScalarValue::Int8(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::Int16(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::Int32(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::Int64(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::UInt8(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::UInt16(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::UInt32(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::UInt64(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ _ => {
+ return internal_err!(
+ "Unexpected datatype for integer round: {}",
+ value.data_type()
+ );
+ }
+ };
+
+ let scalar = match return_type {
Review Comment:
it feels like we can combine these matches; the input and output types are always going to be the same, right?
##########
datafusion/functions/src/math/round.rs:
##########
@@ -630,6 +663,180 @@ fn round_columnar(
}
}
+fn round_integer_value(value: i128, decimal_places: i32) -> Result<i128, ArrowError> {
Review Comment:
is converting all ints to `i128` strictly needed for correctness, or its for ergonomics?
##########
datafusion/functions/src/math/round.rs:
##########
@@ -120,6 +122,13 @@ fn calculate_new_precision_scale<T: DecimalType>(
}
}
+fn is_integer_data_type(data_type: &DataType) -> bool {
Review Comment:
can already use [`DataType::is_integer`](https://docs.rs/arrow/latest/arrow/datatypes/enum.DataType.html#method.is_integer)
##########
datafusion/functions/src/math/round.rs:
##########
@@ -630,6 +663,180 @@ fn round_columnar(
}
}
+fn round_integer_value(value: i128, decimal_places: i32) -> Result<i128, ArrowError> {
+ if decimal_places >= 0 {
+ return Ok(value);
+ }
+
+ let Some(factor) = 10_i128.checked_pow(decimal_places.unsigned_abs()) else {
+ return Ok(0);
+ };
+
+ let remainder = value % factor;
+ let threshold = factor / 2;
+
+ if remainder >= threshold {
+ value
+ .checked_sub(remainder)
+ .and_then(|v| v.checked_add(factor))
+ .ok_or_else(|| {
+ ArrowError::ComputeError("Overflow while rounding integer".to_string())
+ })
+ } else if remainder <= -threshold {
+ value
+ .checked_sub(remainder)
+ .and_then(|v| v.checked_sub(factor))
+ .ok_or_else(|| {
+ ArrowError::ComputeError("Overflow while rounding integer".to_string())
+ })
+ } else {
+ value.checked_sub(remainder).ok_or_else(|| {
+ ArrowError::ComputeError("Overflow while rounding integer".to_string())
+ })
+ }
+}
+
+fn round_integer_scalar(
+ value: &ScalarValue,
+ return_type: &DataType,
+ decimal_places: i32,
+) -> Result<ColumnarValue> {
+ let rounded = match value {
+ ScalarValue::Int8(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::Int16(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::Int32(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::Int64(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::UInt8(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::UInt16(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::UInt32(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ ScalarValue::UInt64(Some(v)) => {
+ round_integer_value(i128::from(*v), decimal_places)?
+ }
+ _ => {
+ return internal_err!(
+ "Unexpected datatype for integer round: {}",
+ value.data_type()
+ );
+ }
+ };
+
+ let scalar = match return_type {
+ Int8 => ScalarValue::Int8(Some(i8::try_from(rounded).map_err(|_| {
+ ArrowError::ComputeError("Overflow while rounding Int8".to_string())
Review Comment:
these dont need to be `ArrowErrors` since we're return a datafusion error in this function
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4762910353
<!-- semver-check-comment -->
Thank you for opening this pull request!
Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).
<details>
<summary>Details</summary>
```
Cloning apache/main
Building datafusion-functions v54.0.0 (current)
Built [ 29.933s] (current)
Parsing datafusion-functions v54.0.0 (current)
Parsed [ 0.089s] (current)
Building datafusion-functions v54.0.0 (baseline)
Built [ 28.634s] (baseline)
Parsing datafusion-functions v54.0.0 (baseline)
Parsed [ 0.090s] (baseline)
Checking datafusion-functions v54.0.0 -> v54.0.0 (no change; assume patch)
Checked [ 0.616s] 223 checks: 223 pass, 30 skip
Summary no semver update required
Finished [ 60.737s] datafusion-functions
Building datafusion-spark v54.0.0 (current)
Built [ 57.051s] (current)
Parsing datafusion-spark v54.0.0 (current)
Parsed [ 0.066s] (current)
Building datafusion-spark v54.0.0 (baseline)
Built [ 56.711s] (baseline)
Parsing datafusion-spark v54.0.0 (baseline)
Parsed [ 0.066s] (baseline)
Checking datafusion-spark v54.0.0 -> v54.0.0 (no change; assume patch)
Checked [ 0.478s] 223 checks: 223 pass, 30 skip
Summary no semver update required
Finished [ 115.562s] datafusion-spark
error: running 'cargo update' on crate 'datafusion-sqllogictest' failed with output:
-----
Updating crates.io index
error: failed to get `num-complex` as a dependency of package `arrow-array v59.0.0`
... which satisfies dependency `arrow-array = "^59.0.0"` of package `arrow v59.0.0`
... which satisfies dependency `arrow = "^59.0.0"` of package `datafusion-sqllogictest v54.0.0 (/home/runner/work/datafusion/datafusion/datafusion/sqllogictest)`
... which satisfies path dependency `datafusion-sqllogictest` of package `placeholder v0.0.0 (/home/runner/work/datafusion/datafusion/target/semver-checks/local-datafusion_sqllogictest-54_0_0-x86_64_unknown_linux_gnu-efe455da045e166b)`
Caused by:
failed to load source for dependency `num-complex`
Caused by:
unable to update registry `crates-io`
Caused by:
download of nu/m-/num-complex failed
Caused by:
curl failed
Caused by:
[16] Error in the HTTP2 framing layer
-----
error: failed to update dependencies for crate datafusion-sqllogictest v54.0.0
note: this is unlikely to be a bug in cargo-semver-checks,
and is probably an issue with the crate's Cargo.toml
note: the following command can be used to reproduce the compilation error:
cargo new --lib example &&
cd example &&
echo '[workspace]' >> Cargo.toml &&
cargo add --path /home/runner/work/datafusion/datafusion/datafusion/sqllogictest --features avro,backtrace,bytes,chrono,datafusion-substrait,parquet_encryption,postgres,postgres-types,substrait,testcontainers-modules,tokio-postgres &&
cargo update
error: aborting due to failure to run 'cargo update' for crate datafusion-sqllogictest v54.0.0
```
</details>
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4601597944
@kumarUjjawal could you pls re-run the CI checks? thnx again
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4601750507
> Will review tomorrow!
Sure, thank you!
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4611897454
@kumarUjjawal all the checks seem to have passed
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on code in PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#discussion_r3347672444
##########
datafusion/functions/src/math/round.rs:
##########
@@ -468,6 +487,11 @@ fn round_columnar(
let decimal_places_is_array = matches!(decimal_places, ColumnarValue::Array(_));
let arr: ArrayRef = match (value_array.data_type(), return_type) {
Review Comment:
@kumarUjjawal I've restored the fast path for non-negative scalar `decimal_places` values while still using `round_integer_array` for negative and column-valued `decimal_places`.
Before:
```rust
(input_type, return_type)
if input_type == return_type && is_integer_data_type(return_type) =>
{
round_integer_array(value_array.as_ref(), decimal_places, return_type)?
}
```
Now:
```rust
(input_type, return_type)
if input_type == return_type && is_integer_data_type(return_type) =>
{
match decimal_places {
ColumnarValue::Scalar(ScalarValue::Int32(Some(dp))) if *dp >= 0 => {
value_array
}
_ => round_integer_array(
value_array.as_ref(),
decimal_places,
return_type,
)?,
}
}
```
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on code in PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#discussion_r3347682694
##########
datafusion/functions/src/math/round.rs:
##########
@@ -468,6 +487,11 @@ fn round_columnar(
let decimal_places_is_array = matches!(decimal_places, ColumnarValue::Array(_));
let arr: ArrayRef = match (value_array.data_type(), return_type) {
Review Comment:
@kumarUjjawal could you kindly pls re-run the checks now? thnx
--
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] fix: Preserve integer values in round() for large Int64 and UInt64 inputs [datafusion]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #22697:
URL: https://github.com/apache/datafusion/pull/22697#issuecomment-4611356407
@kumarUjjawal could you kindly pls re-run the checks now? thnx
--
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