Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow mapped integers to be mapped back to integral BigDecimals #3965

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Apr 18, 2023

Identify the Bug or Feature request

Fixes #3895

Description of the Change

The root of the problem is that when we send execFunction parameters to clients, each parameter is represented as a ScriptTypeDto which supports doubles strings and JSON values. Numbers (represented as BigDecimal) are represented as doubles regardless of whether they are integral or not. When the client receives the parameters, the doubles are converted back to BigDecimal, but integral values still keep their fractional part and thus the string representation can't be parsed as integers.

This PR changes the client side conversion to remove zero fractional parts that might exist in the BigDecimal on the client side. This guarantees the string representation can be parsed by functions like Integer.valueOf(). Note that this change only applies to parameters of execFunction().

I've also added logging for when the callee of execFunction fails, so that similar issues in the future can be more easily diagnosed.

Possible Drawbacks

None

Documentation Notes

N/A

Release Notes

  • Fixed a bug where execFunction() could not run playStream() on clients if two or more parameters were passed.

This change is Reviewable

Also add error logging when the function executed by `execFunction` fails.
@cwisniew cwisniew merged commit abefb6d into RPTools:release-1.13 Apr 18, 2023
4 checks passed
@cwisniew cwisniew added the bug label Apr 18, 2023
@kwvanderlinde kwvanderlinde deleted the bugfix/3895-execFunction-does-not-preserve-integerness-1.13 branch April 19, 2023 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants