Skip to content
This repository has been archived by the owner on Jan 11, 2021. It is now read-only.

Add Decimal support 2 #103

Merged
merged 6 commits into from
Apr 30, 2018
Merged

Add Decimal support 2 #103

merged 6 commits into from
Apr 30, 2018

Conversation

sadikovi
Copy link
Collaborator

This PR attempts to introduce Decimal again. This time changes include:

  • New value type Decimal in basic.rs alongside ByteArray and Int96. Since decimal can be created from i32, i64 or ByteArray, we have enum with 3 variations of underlying storage (either array of clone of ByteArray).
  • Added conversion from INT32, INT64 and BYTE_ARRAY, and FIXED_BYTE_ARRAY.
  • Added method to convert decimal to a string representation, similar to timestamp and date.
  • Had to add dependency on num-bigint because of that.
  • Added tests.

As it was previously noted, Parquet-cpp/mr, Arrow explicitly support decimal as int128 value. Spark uses Java BigDecimal, which supports arbitrary precision values, but Spark itself limits Decimal to precision 38, which is int128.

We do not store decimal as a value, but rather 3 parts: unscaled value as bytes, precision and scale, which is different from Spark, for example, where they try storing it as long. When converting to a string we use num-bigint, which, similar to Java BigDecimal, should support arbitrary precision.

@coveralls
Copy link

coveralls commented Apr 28, 2018

Coverage Status

Coverage increased (+6.0e-05%) to 94.96% when pulling f0cc2f0 on sadikovi:add-decimal-2 into faf33c8 on sunchao:master.

@sunchao sunchao merged commit a851c94 into sunchao:master Apr 30, 2018
@sunchao
Copy link
Owner

sunchao commented Apr 30, 2018

Merged. Thanks @sadikovi !

@sadikovi sadikovi deleted the add-decimal-2 branch April 30, 2018 06:46
@sadikovi
Copy link
Collaborator Author

Thank you very much!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants