[
https://issues.apache.org/jira/browse/AVRO-2837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17111300#comment-17111300
]
Ryan Skraba edited comment on AVRO-2837 at 5/26/20, 4:12 PM:
-------------------------------------------------------------
Thanks for looking into this! For info, we don't use the patches attached to
JIRA any longer; the github PR is the preferred way to propose code changes.
Just to clarify your proposal, if we have a fixed logical type with *precision
5 and scale 2*, we'd expect:
|| Datum || P || S || 1.9.2 || Proposed||
| 123.45 | 5 | 2 | (/) | (/) | OK, perfect match to scale and precision
| 123.450 | 6 | 3 | (x) | (x) | Value can be represented in the logical type.
| 123.456 | 6 | 3 | (x) | (x) | Value can be *approximated* in the logical
type.
| 0123.45 | 6 | 2 | (/) (!) | (x) | Value can be represented in the logical
type.
| 9123.45 | 6 | 2 | (/) (!) | (x) | Value can't fit in the logical type.
| 23.450 | 5 | 3 | (x) | (?) | Value can be represented in the logical type.
| 23.456 | 5 | 3 | (x) | (?) | Value can be *approximated* in the logical type.
| 0123.4 | 5 | 1 | (x) | (/) | Value can be represented in the logical type.
| 9123.4 | 5 | 1 | (x) | (/) | Value can't fit in the logical type.
| 3.456 | 4 | 3 | (x) | (x) | Value can be *approximated* in the logical type.
| 23.45 | 4 | 2 | (/) | (/) | Value can be represented in the logical type.
| 123.4 | 4 | 1 | (x) | (/) | Value can be represented in the logical type.
| 9123 | 4 | 0 | (x) | (/) | Value can't fit in the logical type.
If I remember correctly, Oracle drops and pads decimal places happily to fit
the specified column scale, but fails for numbers too large to fit into the
specified column precision.
**EDIT** I confirmed the behaviour for Avro 1.9.2 column, and there's
definitely something wrong when the precision is too large, but the rules you
suggest (OK to have precision or scale less than expected) don't seem to be
quite right either.
It looks like our choices should be:
# Only accept BigDecimal that are set correctly for the logical type (because
neither 0123.40 or 123.4 equal 123.40)
# Also accept BigDecimal with other precision/scale if they can "fit" by
ignoring or adding zero padding (because both 0123.400 and 123.4 kind of equal
123.40)
# In addition to the above, also accept BigDecimals that would lose part of
their fraction when forced into the logical type (because if you shoehorn
123.456 into a scale of 2, you obviously don't care if you lose a couple
thousandths)
What do you think?
was (Author: ryanskraba):
Thanks for looking into this! For info, we don't use the patches attached to
JIRA any longer; the github PR is the preferred way to propose code changes.
Just to clarify your proposal, if we have a fixed logical type with *precision
5 and scale 2*, we'd expect:
|| Datum || P || X || Current ||Proposed||
| 123.45 | 5 | 2 | (/) | (/) | OK, perfect match to scale and precision
| 1123.45 | 6 | 2 | (?) | (x) | Precision too big
| 23.456 | 5 | 3 | (x) | (x) | Scale bigger than expected, could be
serialized with data loss
| 123.456 | 6 | 3 | (x) | (x) | Precision bigger than expected, could be
serialized with data loss
| 1234.5 | 5 | 1 | (x) | (x) | Scale smaller than expected, *might* be
serialized without data loss depending on leading zeros
| 23.45 | 4 | 2 | (?) | (/) | Precision smaller than expected, can be
stored without data loss.
| 123.4 | 4 | 1 | (x) | (/) | Store as 123.40, pad zeros to the scale,
*can* always be serialized without data loss
| 0123.4 | 5 | 1 | (x) | (/) | Store as 123.40, pad zeros to the scale,
*might* be serialized without data loss depending on leading zeros
I don't have this table entirely clear for the Java SDK, I just want to check
that I understand the proposal!
If I remember correctly, Oracle drops and pads decimal places happily to fit
the column type, but fails for numbers too large to fit into the specified
precision.
> 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)
