Decimal #197
Open
Decimal #197
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR, titled "Decimal", introduces support for the Decimal type by adding BigDecimal functionality and integrating it into query parameters, tests, and result wrappers.
- Added unit tests for decimal conversion in JavaScript and Rust.
- Extended the parameter wrappers and utility functions to handle the Decimal type.
- Updated the BigDecimal class to support new static method patterns.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/unit/query-parameter-tests.js | Adds a test case for the Decimal type using BigDecimal.fromString. |
| test/unit/cql-value-wrapper-tests.js | Adds tests to verify correct Decimal conversion from the Rust API. |
| src/tests/result_tests.rs | Adds a Rust test returning a Decimal CqlValueWrapper. |
| src/tests/request_values_tests.rs | Extends test cases to include Decimal type handling. |
| src/result.rs | Introduces a new API to extract the decimal buffer representation. |
| src/requests/parameter_wrappers.rs | Adds a from_decimal helper for Decimal values. |
| lib/types/results-wrapper.js | Updates getCqlObject to support Decimal conversion via BigDecimal. |
| lib/types/cql-utils.js | Adds Decimal handling in wrapValueBasedOnType. |
| lib/types/big-decimal.js | Refactors BigDecimal into a class with static methods and modern syntax. |
2293dec to
ec13185
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces support for the Decimal CQL type by integrating a new BigDecimal type into both the JavaScript and Rust test suites as well as the driver’s utility functions.
- Adds BigDecimal support in parameter wrappers and result handling.
- Updates unit tests to include Decimal type cases for queries and value wrapping.
- Adjusts related utility functions (e.g. in cql-utils.js and big-decimal.js) to process and convert BigDecimal values properly.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/query-parameter-tests.js | Adds test case for Decimal type using BigDecimal.fromString. |
| test/unit/cql-value-wrapper-tests.js | Introduces a unit test for Decimal type conversion via napi. |
| src/tests/result_tests.rs | Adds a Rust test function returning a Decimal wrapped value. |
| src/tests/request_values_tests.rs | Adds Decimal type handling in test functions for query parameter parsing. |
| src/result.rs | Implements get_decimal() to convert a Decimal CQL value into a Buffer. |
| src/requests/parameter_wrappers.rs | Adds from_decimal() for constructing a Decimal query parameter. |
| lib/types/results-wrapper.js | Integrates BigDecimal conversion in results processing. |
| lib/types/cql-utils.js | Adds Decimal handling support in value wrapping with BigDecimal conversion. |
| lib/types/big-decimal.js | Migrates BigDecimal to an ES6 class with static conversion methods. |
adespawn
requested changes
May 5, 2025
| * decimal point. If negative, the unscaled value of the number is | ||
| * multiplied by ten to the power of the negation of the scale. The | ||
| * value of the number represented by the `BigDecimal` is | ||
| * therefore <tt>(unscaledValue × 10<sup>-scale</sup>)</tt>. |
There was a problem hiding this comment.
Is it possible to remove those html tags?
There was a problem hiding this comment.
I don;t think so. Maybe @Chlor87 knows the answer
|
|
510c4f4 to
d061916
Compare
|
As previously discussed, please re-base onto #183 |
Before, we created QueryParameterWrapper with from_... functions, and then cloning this object when executing query. Now, with implementation of FromNapiValue trait, we pass parameters to queries directly, without any additional conversion steps. In order to correctly handle value types, each parameter is passed as a pair (type, value). Information about type is then used on the rust side to create correct CqlValue object.
Adds support for converting Decimal responses to a JavaScript object.
Adds support for convertin JS Decimal type into Rust Decimal type.
Added tests to verify Decimal encoding.
Added tests to verify Decimal decoding.
adespawn
requested changes
May 16, 2025
| return parseFloat(this.toString()); | ||
| }; | ||
| /** | ||
| * Returns a Number representation of this <code>BigDecimal</code>. |
There was a problem hiding this comment.
Why did you add a code tag here?
| let buf = get_element!(&[u8]); | ||
| CqlValue::Decimal(CqlDecimal::from_signed_be_bytes_slice_and_exponent( | ||
| &buf[4..], | ||
| i32::from_be_bytes(buf[0..4].try_into().unwrap()), |
Comment on lines
+69
to
+75
| "Decimal" => CqlValue::Decimal(CqlDecimal::from_signed_be_bytes_slice_and_exponent( | ||
| &[ | ||
| 1, 53, 169, 169, 173, 175, 83, 216, 15, 110, 137, 47, 175, 202, 192, 196, 222, 179, | ||
| 11, 93, 98, 127, 51, 6, 161, 141, 90, 11, 80, 251, 28, | ||
| ], | ||
| 69, | ||
| )), |
There was a problem hiding this comment.
Pleas also add at least one test with negative value, and one test with negative exponent
There was a problem hiding this comment.
(Ditto) Pleas also add at least one test with negative value, and one test with negative exponent
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.