feat: Implement Decimal from/to bytes represents #665
Conversation
Signed-off-by: Xuanwo <[email protected]>
sdd
left a comment
sdd
left a comment
There was a problem hiding this comment.
The values in the tests match what I'd expect from converting manually 👍🏼
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @Xuanwo for fixing this, generally LGTM! Left some minor points to fix.
Signed-off-by: Xuanwo <[email protected]>
Signed-off-by: Xuanwo <[email protected]>
Signed-off-by: Xuanwo <[email protected]>
|
cc @liurenjie1024 and @Fokko to take another look, sorry for the long wait. |
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @Xuanwo for this great pr, left some minor suggestions!
| if let Type::Primitive(p) = r#type { | ||
| Ok(Self { | ||
| r#type: p, | ||
| literal: PrimitiveLiteral::Int128(decimal.mantissa()), |
There was a problem hiding this comment.
This is incorrect, we need to validate that the precision is large enough to hold this value. For example, if the input decimal is 123456789, and the required precision is 4, then we should throw an error here.
There was a problem hiding this comment.
Fixed, and I added a new unit test to cover this case. PTAL, thank you!
Signed-off-by: Xuanwo <[email protected]>
|
cc @Fokko, would you like to take another look? I believe it's good for merging now. |
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @Xuanwo , LGTM!
4 participants
Close #663
This PR will implement Decimal from/to bytes represents.