Skip to content

Teach XContentGenerator#copyCurrentEvent to handle BigInteger and BigDecimal#111936

Closed
lkts wants to merge 3 commits intoelastic:mainfrom
lkts:fix/111812
Closed

Teach XContentGenerator#copyCurrentEvent to handle BigInteger and BigDecimal#111936
lkts wants to merge 3 commits intoelastic:mainfrom
lkts:fix/111812

Conversation

@lkts
Copy link
Contributor

@lkts lkts commented Aug 15, 2024

This logic was simply missing from the implementation. It's a pretty tricky case since it only reproduces with wrapped parsers but works fine when directly using JsonXContentParser.

Closes #111812.

@lkts lkts added >bug :StorageEngine/Logs You know, for Logs labels Aug 15, 2024
@lkts lkts requested review from dnhatn and martijnvg August 15, 2024 19:06
@lkts lkts self-assigned this Aug 15, 2024
@lkts lkts requested a review from a team as a code owner August 15, 2024 19:06
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@elasticsearchmachine
Copy link
Collaborator

Hi @lkts, I've created a changelog YAML for you.

case VALUE_NUMBER:
switch (parser.numberType()) {
case INT -> writeNumber(parser.intValue());
case BIG_INTEGER -> writeNumber((BigInteger) parser.numberValue());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see a way to do this without casting.

@rjernst rjernst added :Core/Infra/Core Core issues without another label team-discuss labels Aug 15, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Aug 15, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@rjernst rjernst self-requested a review August 15, 2024 20:02
@rjernst
Copy link
Member

rjernst commented Aug 21, 2024

@lkts I've addressed the underlying issue in #111937, so I hope you don't mind I close this PR.

@rjernst rjernst closed this Aug 21, 2024
@lkts
Copy link
Contributor Author

lkts commented Aug 21, 2024

thanks @rjernst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Core/Infra/Core Core issues without another label :StorageEngine/Logs You know, for Logs Team:Core/Infra Meta label for core/infra team Team:StorageEngine team-discuss v8.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XContentGenerator#copyCurrentEvent does not handle BigInteger and BigDecimal

3 participants