KAFKA-9667: Connect JSON serde strip trailing zeros#8230
Merged
rhauch merged 2 commits intoapache:trunkfrom May 7, 2020
Merged
KAFKA-9667: Connect JSON serde strip trailing zeros#8230rhauch merged 2 commits intoapache:trunkfrom
rhauch merged 2 commits intoapache:trunkfrom
Conversation
This change turns on exact decimal processing in Jackson for decimals, meaning trailing zeros are maintained. This means a value of `1.2300` can be deserialized and re-serialized to JSON without any loss of information.
2 tasks
Contributor
Author
|
Related to confluentinc/ksql#4710 |
agavra
reviewed
Mar 5, 2020
Contributor
agavra
left a comment
agavra
left a comment
There was a problem hiding this comment.
LGTM! (Not approving since I'm not a committer)
rhauch
reviewed
Mar 5, 2020
Contributor
rhauch
left a comment
rhauch
left a comment
There was a problem hiding this comment.
Thanks, @big-andy-coates. The changes look great, but I have a few comments in-line.
Contributor
Author
|
Waiting on code freeze to lift before merging. |
2 tasks
rayokota
added a commit
to rayokota/schema-registry
that referenced
this pull request
Mar 12, 2020
This is the same fix as KAFKA-9667. See apache/kafka#8230
rayokota
added a commit
to confluentinc/schema-registry
that referenced
this pull request
Mar 12, 2020
* DG-295 Fix trailing zeroes being stripped This is the same fix as KAFKA-9667. See apache/kafka#8230 * Minor cleanup * Fix decimal handling in tests
Contributor
Author
|
@rhauch can this be merged now? |
Contributor
|
okay to test |
rhauch
approved these changes
May 6, 2020
Contributor
rhauch
left a comment
rhauch
left a comment
There was a problem hiding this comment.
LGTM. Thanks, @big-andy-coates.
Will merge pending a green build.
Contributor
|
retest this please |
rhauch
pushed a commit
that referenced
this pull request
May 7, 2020
This change turns on exact decimal processing in JSON Converter for deserializing decimals, meaning trailing zeros are maintained. Serialization was already using the decimal scale to output the right value, so this change means a value of `1.2300` can now be serialized to JSON and deserialized back to Connect without any loss of information. Author: Andy Coates <big-andy-coates@users.noreply.github.com> Reviewers: Randall Hauch <rhauch@gmail.com>, Almog Gavra <almog@confluent.io>
rhauch
pushed a commit
that referenced
this pull request
May 7, 2020
This change turns on exact decimal processing in JSON Converter for deserializing decimals, meaning trailing zeros are maintained. Serialization was already using the decimal scale to output the right value, so this change means a value of `1.2300` can now be serialized to JSON and deserialized back to Connect without any loss of information. Author: Andy Coates <big-andy-coates@users.noreply.github.com> Reviewers: Randall Hauch <rhauch@gmail.com>, Almog Gavra <almog@confluent.io>
Kvicii
pushed a commit
to Kvicii/kafka
that referenced
this pull request
May 8, 2020
* 'trunk' of github.com:apache/kafka: KAFKA-9290: Update IQ related JavaDocs (apache#8114) KAFKA-9928: Fix flaky GlobalKTableEOSIntegrationTest (apache#8600) KAFKA-6145: Set HighAvailabilityTaskAssignor as default in streams_upgrade_test.py (apache#8613) KAFKA-9667: Connect JSON serde strip trailing zeros (apache#8230) MINOR: Log4j Improvements on Fetcher (apache#8629)
qq619618919
pushed a commit
to qq619618919/kafka
that referenced
this pull request
May 12, 2020
This change turns on exact decimal processing in JSON Converter for deserializing decimals, meaning trailing zeros are maintained. Serialization was already using the decimal scale to output the right value, so this change means a value of `1.2300` can now be serialized to JSON and deserialized back to Connect without any loss of information. Author: Andy Coates <big-andy-coates@users.noreply.github.com> Reviewers: Randall Hauch <rhauch@gmail.com>, Almog Gavra <almog@confluent.io>
jwijgerd
pushed a commit
to buxapp/kafka
that referenced
this pull request
May 14, 2020
This change turns on exact decimal processing in JSON Converter for deserializing decimals, meaning trailing zeros are maintained. Serialization was already using the decimal scale to output the right value, so this change means a value of `1.2300` can now be serialized to JSON and deserialized back to Connect without any loss of information. Author: Andy Coates <big-andy-coates@users.noreply.github.com> Reviewers: Randall Hauch <rhauch@gmail.com>, Almog Gavra <almog@confluent.io>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Fixes: KAFKA-9667
The Connect Json serde was recently enhanced to support serializing decimals as standard JSON numbers, (Original work done under KAFKA-8595), e.g.
1.23. However, there is a bug in the implementation: it's stripping trailing zeros!1.23is not the same as1.230. The first is a number accurate to 2 decimal places, where as the later is accurate to 3 dp.It is important that trailing zeros are not dropped when de(serializing) decimals. For some use-cases it may be acceptable to drop the trailing zeros, but for others it definitely is not.
Current Functionality
If a JSON object was to contain the number
1.230then the Java JsonDeserializer would correctly deserialize this into aBigDecimal. The BigDecimal would have a scale of 3, which is correct.However, if that same BigDecimal was then serialized back to JSON using the Java JsonSerializer it would incorrectly strip the zeros, serializing to
1.23.Expected Functionality
When serializing, trailing zeros should be maintained. For example, a BigDecimal such as
1.230, (3 dp), should be serialized as1.230.Compatibility
Both the old serialized number, e.g.
1.23, and the proposed corrected serialized number, e.g.1.230, are valid JSON numbers. Downstream consumers should have no issue deserializing either.This is not super urgent, but would be good to get into the next point releases of 2.4 and 2.5.
Committer Checklist (excluded from commit message)