[ 
https://issues.apache.org/jira/browse/AVRO-2837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17117464#comment-17117464
 ] 

Matthew McMahon commented on AVRO-2837:
---------------------------------------

Hi [~rskraba]

Thanks for taking the time to review.

I've looked into what is happening, and from what I understand, it appears that 
both precision and scale need to be verified on the BigDecimal logical type.

I tried an example of 1234.5 in the (precision-5, scale-2) case. I found that 
this should fail, due to:
 * The precision is 5, and scale is 1. We could assume this fits and encode the 
value. However on decode this pulls out an incorrect value of 123.45. On decode 
its not possible to know what scale we had initially, which wouldn't be a good 
outcome.

I don't believe we should do any rounding that would lose any precision. If 
that is ok in the user specific case, than the scaling can be done in user code.

But if the scaling is safe (no rounding required) to pad/drop trailing zeros to 
get the required scale, we should do it, and then check the precision after the 
scaling is set, to ensure will still fit for encoding and produce correct 
result on decoding.

I've added a few extra test cases, and improved error messaging to 
[https://github.com/apache/avro/pull/884] along with the attempted dropping of 
trailing zeros in the scale.

I also added a test with my example above, to show why it appears to me that 
scale is important, as well as precision.

Please let me know what you think.

Matt

> Java DecimalConversion handling of scale and precision
> ------------------------------------------------------
>
>                 Key: AVRO-2837
>                 URL: https://issues.apache.org/jira/browse/AVRO-2837
>             Project: Apache Avro
>          Issue Type: Bug
>          Components: java, logical types
>    Affects Versions: 1.8.2, 1.9.2
>            Reporter: Matthew McMahon
>            Priority: Major
>         Attachments: AVRO-2837.patch, AVRO-2837.patch
>
>
> Came across an interesting issue in Avro 1.8.2
> Configured a decimal logical type (Fixed type of size 12 with scale of 15 and 
> precision of 28).
> Due to an upstream bug, a value of 1070464558597365250.000000000000000 
> (1.07046455859736525E+18 that is then rescaled to 15) appears, and the 
> DecimalConversion attempts to turn it into a Fixed type.
> This should have failed, as it has a precision of 34 and won't fit into the 
> 12 bytes (needs 14). However in 1.8.2 this still writes a value that 
> downstream processing then works out is invalid and errors.
> Basically the top 2 bytes are thrown away.
> This problem is fixed in 1.9.2 due to the change in 
> https://issues.apache.org/jira/browse/AVRO-2309 as this particular issue 
> fails when it attempts to pass an offset of -2 to the System.arraycopy method.
> That seems ok, but is a bit of a red herring to the actual issue, and 
> precision is still not actually being checked.
> Proposing a couple changes to the DecimalConversion:
>  * Check precision set on the decimal logical type. If value has greater 
> precision then error with more informative message
>  * Still check scale and error if value has a greater scale. However if the 
> scale in value is less, than it seems safe to update the scale and pad zero's 
> rather than error
>  * Do this for both Bytes and Fixed types
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to