ARROW-14843 [R] Implement decimal128() (to replace decimal())

[dragosmg]

This PR looks to swap decimal() for decimal128() in preparation for adding decimal256(). At present both decimal() and decimal128() work, but we might decide to deprecate decimal() at some point in the future.

[github-actions]

https://issues.apache.org/jira/browse/ARROW-14843

[github-actions]

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

[thisisnic]

Looks good so far - out of interest, I see another ticket open for decimal256() - would it be possible to implement that here?

[dragosmg]

I think decimal256() is a bit more involved (not yet surfaced from C++). Hence the intention to implement them in 2 separate PRs. For decimal128() we didn't have to add a binding. It was merely a matter of porting the code for decimal().

[dragosmg]

Do we need to change this to point to DECIMAL128?

arrow/r/src/datatype.cpp

Line 83 in 8581f36

case Type::DECIMAL:

I think there might be several changes needed in datatype.cpp.

[thisisnic]

I think decimal256() is a bit more involved (not yet surfaced from C++). Hence the intention to implement them in 2 separate PRs. For decimal128() we didn't have to add a binding. It was merely a matter of porting the code for decimal().

That's fair enough!

Do we need to change this to point to DECIMAL128?

I think so - I took a look at how TimeType + Time32 + Time64 are all implemented here and in arrow/r/R/type.R to try to work this out.

I also found potential changes needed to the function nse_funcs$is.numeric(), and there's also this which could need updating:

arrow/r/R/enums.R

Line 68 in 236362a

DECIMAL = 23L,

[paleolimbot]

I scanned this for typos and anything that looked off to me and found nothing!

[dragosmg]

Thanks @thisisnic and @paleolimbot for the comments. By the looks of it we have quite a few of the pieces for decimal256() in place.

[dragosmg]

There's also this reference, which might need updating.

arrow/r/src/array_to_vector.cpp

Line 1219 in 236362a

case Type::DECIMAL:

[jonkeane]

This is looking good, a few comments/suggestions

[jonkeane]

This looks good — two last comments that you can take or leave. Thank you!

[ursabot]

Benchmark runs are scheduled for baseline = ee32aea and contender = 310fca9. 310fca9 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.18% ⬆️0.04%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] <https://buildkite.com/ursa-computing/benchmarks-ec2-t3-xlarge-us-east-2/builds/1646| 310fca93 ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/ursa-computing/benchmarks-ursa-i9-9960x/builds/1649| 310fca93 ursa-i9-9960x>
[Finished] <https://buildkite.com/ursa-computing/benchmarks-ursa-thinkcentre-m75q/builds/1587| 310fca93 ursa-thinkcentre-m75q>
[Finished] <https://buildkite.com/ursa-computing/benchmarks-ec2-t3-xlarge-us-east-2/builds/1645| ee32aeaa ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/ursa-computing/benchmarks-ursa-i9-9960x/builds/1648| ee32aeaa ursa-i9-9960x>
[Finished] <https://buildkite.com/ursa-computing/benchmarks-ursa-thinkcentre-m75q/builds/1586| ee32aeaa ursa-thinkcentre-m75q>
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