You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/22 01:07:46 UTC
[GitHub] [arrow] viirya opened a new pull request, #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
viirya opened a new pull request, #13685:
URL: https://github.com/apache/arrow/pull/13685
arrow-rs has added decimal256 support recently. We should activate generate_decimal256_case integration test.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] viirya commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1200248156
@alamb Ah, sorry, after rechecking it, I made a mistake when filling maximum and minimum decimal values for precision > 38. So for `generate_decimal256_case` it is a false alarm. Sorry for confusion.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] viirya commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1192184041
cc @alamb
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1192560815
🎉 🦜
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1199935840
I will watch https://github.com/apache/arrow/runs/7585938940?check_suite_focus=true and https://github.com/apache/arrow/runs/7585938943?check_suite_focus=true to make sure they pass -- and if not I will revert this PR
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] viirya commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1199962817
Ah, there was a failure:
```
Error: InvalidArgumentError("-555221045220379430784912070743352079671 is too small to store in a Decimal256 of precision 39. Min is -99999999999999999999999999999999999999")
```
Like Decimal128 case, we also need to disable validation for Decimal256 case for this integration.
Proposed a fix at https://github.com/apache/arrow-rs/pull/2232
cc @alamb
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] viirya commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1199947797
Thank you @alamb !
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb merged pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
alamb merged PR #13685:
URL: https://github.com/apache/arrow/pull/13685
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1201168300
Thank you @viirya !
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1200144623
Thanks @viirya -- as I mentioned in https://github.com/apache/arrow-rs/pull/2232#pullrequestreview-1056296021 something seems strange if the integration test is using invalid values for Decimal -- either the integration test should be fixed or maybe we have mis-interpreted what is valid in the Rust implementation 🤔
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1200150677
FYI @zeroshade
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1200057058
Benchmark runs are scheduled for baseline = 778d574b1aa4d45742cab04c0cc4d71141f0420a and contender = f4c0cda01fa416407df27a1d69140aea4b36c877. f4c0cda01fa416407df27a1d69140aea4b36c877 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/3f40488e73354fe188a8034eba0b8b13...9ac8c1eea2e346c6b001f48a1c138263/)
[Finished :arrow_down:0.44% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/230d7df99d064d8da92e9af3feb159d0...2341c71d72174d659def7d306da0d0df/)
[Failed :arrow_down:0.54% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/eb6497945a7e43e7b13e88a172ada33b...4b5e0a2a88614297a8470421f7305a93/)
[Finished :arrow_down:0.14% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0ebbdaaeb986466b9d555ca9226ac0cd...4c3fcac642ad4a2ca2af6ecc879aa05d/)
Buildkite builds:
[Failed] [`f4c0cda0` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1229)
[Finished] [`f4c0cda0` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1241)
[Finished] [`f4c0cda0` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1223)
[Finished] [`f4c0cda0` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1243)
[Failed] [`778d574b` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1228)
[Finished] [`778d574b` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1240)
[Failed] [`778d574b` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1222)
[Finished] [`778d574b` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1242)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] viirya commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1200248799
Going to fix it at https://github.com/apache/arrow-rs/pull/2245
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1192077510
:warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1192077501
https://issues.apache.org/jira/browse/ARROW-17176
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] viirya commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1199785475
@alamb Can you help merge this? If you are busy, please let me know how to merge this. Thanks. 😄
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #13685: ARROW-17176: [Rust] Activate generate_decimal256_case integration test for rust
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #13685:
URL: https://github.com/apache/arrow/pull/13685#issuecomment-1199935385
Hi @viirya -- I think we can just merge. Thanks!
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org