Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for Decimal with 256 bits of precision. #131

Closed
alamb opened this issue Apr 26, 2021 · 7 comments
Closed

Implement support for Decimal with 256 bits of precision. #131

alamb opened this issue Apr 26, 2021 · 7 comments
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog help wanted

Comments

@alamb
Copy link
Contributor

alamb commented Apr 26, 2021

Note: migrated from original JIRA: https://issues.apache.org/jira/browse/ARROW-10602

The specification now supports it and there are basic implementations in C++/Java

The basic idea would be to:

  1. Study the C++ implementation that allows both i128 and i256 backing
  2. Propose a Rust implementation
  3. Implement said Rust implementation (ideally by breaking it down into smaller issues that the community could help with)
@alexlee85
Copy link

It's been over a year ......

@alamb
Copy link
Contributor Author

alamb commented Jun 7, 2022

Thanks for the nudge @alexlee85 -- I left some ideas on how to proceed. Let's see if we can find someone to help!

@viirya
Copy link
Member

viirya commented Jun 12, 2022

I'm looking around at Decimal support in C++. Roughly I think we have some subtasks:

In C++ implementations, there are Decimal128 and Decimal256 classes. They are used in Decimal128Builder and Decimal256Builder APIs. So users can construct 128/256-decimal values during building the arrays. Although Decimal128Array and Decimal256Array don't provide APIs to get Decimal128 and Decimal256 values directly now.

Basically in Decimal128 and Decimal256 classes, decimal value is represented in an uint64_t array. I think we can follow this up to have Decimal with 256 bits.

In current Rust implementations, we use i128 in DecimalBuilder and DecimalArray APIs. To maintain consistency on Decimal related APIs, we need to change from i128 to Decimal128 in these APIs.

I think we may also need some works in IPC and Parquet for the new Decimal256 support.

@viirya
Copy link
Member

viirya commented Jun 12, 2022

I plan to tackle these issues one by one.

@alamb
Copy link
Contributor Author

alamb commented Jun 13, 2022

In current Rust implementations, we use i128 in DecimalBuilder and DecimalArray APIs. To maintain consistency on Decimal related APIs, we need to change from i128 to Decimal128 in these APIs.

What do you think about using generics (so the APIs are compatible).

So for functions that take i128 like

fn decimal_thing(val: i128) {
...
}

Could be something like

fn decimal_thing(val: impl Into<i128>) {
}

This may be a silly idea that won't make sense as you start implementation 🤔

@viirya
Copy link
Member

viirya commented Jun 13, 2022

I think you mean to make Decimal128 to be compatible with i128 so it won't break existing user code?

It makes sense to me.

@alamb
Copy link
Contributor Author

alamb commented Aug 4, 2022

@viirya has completed this work I believe, including enabling integration tests 🎉 with other arrow implementations
apache/arrow#13685

@alamb alamb closed this as completed Aug 4, 2022
@alamb alamb added the enhancement Any new improvement worthy of a entry in the changelog label Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog help wanted
Projects
None yet
Development

No branches or pull requests

3 participants