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

types/basetypes: Fix Float64Attribute precision comparisons #817

Merged
merged 15 commits into from
Aug 3, 2023

Conversation

austinvalle
Copy link
Member

Closes #815
Related corner tests: hashicorp/terraform-provider-corner#170

Context

Terraform core's number type can store up to 512 bits of precision for floating numbers. This level of precision is typically only encountered when performing an arithmetic operation like below:

 $ terraform console
> 1 - 0.99
0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003

The Float64Attribute underlying value of Go's built-in float64 type was losing this precision and responding with invalid plan value errors seen in #815 .

Solution

This PR replaces the underlying value of Float64Attribute with a *big.Float type, which is already supported in terraform-plugin-go. Semantic equality logic has also been implemented to ensure that Computed values do not replace more precise prior values.

@austinvalle austinvalle requested a review from a team as a code owner August 3, 2023 16:02
.changes/unreleased/BUG FIXES-20230803-120411.yaml Outdated Show resolved Hide resolved
types/basetypes/float64_type.go Outdated Show resolved Hide resolved
types/basetypes/float64_value.go Show resolved Hide resolved
types/basetypes/float64_value.go Show resolved Hide resolved
Copy link
Member

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks good to me 🚀

types/basetypes/float64_value.go Show resolved Hide resolved
types/basetypes/float64_value.go Outdated Show resolved Hide resolved
@bflad bflad added this to the v1.3.4 milestone Aug 3, 2023
Co-authored-by: Brian Flad <bflad417@gmail.com>
@bflad bflad added bug Something isn't working types Issues and pull requests about our types abstraction and implementations. labels Aug 3, 2023
@bflad
Copy link
Member

bflad commented Aug 3, 2023

Oh wow, surprised that (*big.Float).Cmp() will panic when passed a nil value -- never mind about that I guess.

@austinvalle austinvalle merged commit a4b8bc6 into main Aug 3, 2023
19 checks passed
@austinvalle austinvalle deleted the av/float64-to-bigfloat branch August 3, 2023 18:31
jharley added a commit to honeycombio/terraform-provider-honeycombio that referenced this pull request Aug 3, 2023
Fix included in terraform-plugin-framework v1.3.4 (via
hashicorp/terraform-plugin-framework#817)

- Closes #335
@github-actions
Copy link

github-actions bot commented Sep 3, 2023

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working types Issues and pull requests about our types abstraction and implementations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Floating point precision differences after migrating from SDKv2
2 participants