close
Skip to content

[Fix][Serialize] prevent scientific notation for decimal numbers in JSON output​#9186

Merged
corgy-w merged 8 commits into
apache:devfrom
misi1987107:bugfix-8949
Apr 22, 2025
Merged

[Fix][Serialize] prevent scientific notation for decimal numbers in JSON output​#9186
corgy-w merged 8 commits into
apache:devfrom
misi1987107:bugfix-8949

Conversation

@misi1987107

Copy link
Copy Markdown
Contributor

Purpose of this pull request

subtask of #8949

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@misi1987107

misi1987107 commented Apr 16, 2025

Copy link
Copy Markdown
Contributor Author

copy PR from #9126, it can't pass ci test ,so i copy the PR

@davidzollo davidzollo requested a review from Copilot April 16, 2025 06:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request resolves bug 8949 by adjusting JSON serialization behavior for BigDecimal values and enhancing test coverage.

  • Added a new unit test (testSerializationWithNumber) to verify that BigDecimal values are serialized as plain numbers.
  • Configured the underlying Jackson mapper to write BigDecimal values in plain format.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
seatunnel-formats/seatunnel-format-json/src/test/java/org/apache/seatunnel/format/json/JsonRowDataSerDeSchemaTest.java Added a new test to check proper serialization of numeric types with BigDecimal specialization.
seatunnel-formats/seatunnel-format-json/src/main/java/org/apache/seatunnel/format/json/JsonSerializationSchema.java Configured the Jackson mapper to write BigDecimal as plain text to support the fix.
Comments suppressed due to low confidence (1)

seatunnel-formats/seatunnel-format-json/src/test/java/org/apache/seatunnel/format/json/JsonRowDataSerDeSchemaTest.java:690

  • [nitpick] Consider using StandardCharsets.UTF_8 instead of Charset.forName("UTF-8") for consistency with the rest of the code.
JsonSerializationSchema jsonSerializationSchema = new JsonSerializationSchema(schema, Charset.forName("UTF-8"));

private final RowToJsonConverters.RowToJsonConverter runtimeConverter;

{
mapper.configure(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN, true);

Copilot AI Apr 16, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider moving the mapper configuration from the instance initializer block into the constructor to ensure the initialization order is explicit.

Copilot uses AI. Check for mistakes.
@nielifeng nielifeng requested a review from Copilot April 16, 2025 06:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

seatunnel-formats/seatunnel-format-json/src/test/java/org/apache/seatunnel/format/json/JsonRowDataSerDeSchemaTest.java:691

  • [nitpick] Consider adding additional test cases for BigDecimal values (e.g., with trailing zeros or varying precision) to further validate the serialization behavior.
Object[] fields = new Object[] {1, "1001015", BigDecimal.valueOf(80.00)};

@corgy-w corgy-w changed the title [Bug] resolve bug 8949 [Fix][Serialize] prevent scientific notation for decimal numbers in JSON output​ Apr 17, 2025
[bugfix] add test case

[bugfix] code format

[improve] test annotation

[improve] remove annotation

[improve] modify character set

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the serialization behavior for BigDecimal values by preventing scientific notation in JSON outputs. It adds a new test case to validate that a decimal value is output in plain format and configures the JSON mapper accordingly.

  • Added a test case for decimal number serialization.
  • Configured the ObjectMapper to use plain BigDecimal formatting.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
seatunnel-format-json/src/test/java/org/apache/seatunnel/format/json/JsonRowDataSerDeSchemaTest.java Added a test case to verify that a BigDecimal value is not output in scientific notation.
seatunnel-format-json/src/main/java/org/apache/seatunnel/format/json/JsonSerializationSchema.java Updated the ObjectMapper configuration to write BigDecimal values as plain numbers.
Comments suppressed due to low confidence (1)

seatunnel-formats/seatunnel-format-json/src/test/java/org/apache/seatunnel/format/json/JsonRowDataSerDeSchemaTest.java:691

  • Consider adding additional test cases with a range of BigDecimal values (including very small and very large numbers) to further ensure that scientific notation is consistently prevented.
Object[] fields = new Object[] {1, "1001015", BigDecimal.valueOf(80.00)};

@corgy-w

corgy-w commented Apr 22, 2025

Copy link
Copy Markdown
Contributor

@misi1987107 CI Try again and try again, feels successful soon

@corgy-w

corgy-w commented Apr 22, 2025

Copy link
Copy Markdown
Contributor
image

@corgy-w corgy-w merged commit 2a082b1 into apache:dev Apr 22, 2025
@misi1987107

Copy link
Copy Markdown
Contributor Author

Thanks.

jia17 pushed a commit to jia17/seatunnel that referenced this pull request Apr 22, 2025
…SON output​ (apache#9186)

Co-authored-by: misi <gientech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants