Skip to content

types/basetypes: Fix Float64Attribute precision comparisons #817

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

Merged
merged 15 commits into from
Aug 3, 2023
6 changes: 6 additions & 0 deletions .changes/unreleased/BUG FIXES-20230803-120411.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: BUG FIXES
body: 'types/basetypes: Prevented Float64Value Terraform data consistency errors for
numbers with high precision floating point rounding errors'
time: 2023-08-03T12:04:11.996955-04:00
custom:
Issue: "817"
6 changes: 5 additions & 1 deletion types/basetypes/float64_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,11 @@ func (t Float64Type) ValueFromTerraform(ctx context.Context, in tftypes.Value) (
return nil, fmt.Errorf("Value %s cannot be represented as a 64-bit floating point.", bigF)
}

return NewFloat64Value(f), nil
// Underlying *big.Float values are not exposed with helper functions, so creating Float64Value via struct literal
return Float64Value{
state: attr.ValueStateKnown,
value: bigF,
}, nil
}

// ValueType returns the Value type.
Expand Down
31 changes: 25 additions & 6 deletions types/basetypes/float64_type_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,36 @@ func TestFloat64TypeValueFromTerraform(t *testing.T) {
expectedErr: "can't unmarshal tftypes.String into *big.Float, expected *big.Float",
},
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/647
// To ensure underlying *big.Float precision matches, create `expectation` via struct literal
"zero-string-float": {
input: tftypes.NewValue(tftypes.Number, testMustParseFloat("0.0")),
expectation: NewFloat64Value(0.0),
input: tftypes.NewValue(tftypes.Number, testMustParseFloat("0.0")),
expectation: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.0"),
},
},
"positive-string-float": {
input: tftypes.NewValue(tftypes.Number, testMustParseFloat("123.2")),
expectation: NewFloat64Value(123.2),
input: tftypes.NewValue(tftypes.Number, testMustParseFloat("123.2")),
expectation: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("123.2"),
},
},
"negative-string-float": {
input: tftypes.NewValue(tftypes.Number, testMustParseFloat("-123.2")),
expectation: NewFloat64Value(-123.2),
input: tftypes.NewValue(tftypes.Number, testMustParseFloat("-123.2")),
expectation: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("-123.2"),
},
},
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/815
// To ensure underlying *big.Float precision matches, create `expectation` via struct literal
"retain-string-float-512-precision": {
input: tftypes.NewValue(tftypes.Number, testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003")),
expectation: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003"),
},
},
// Reference: https://pkg.go.dev/math/big#Float.Float64
// Reference: https://pkg.go.dev/math#pkg-constants
Expand Down
58 changes: 50 additions & 8 deletions types/basetypes/float64_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package basetypes
import (
"context"
"fmt"
"math/big"

"github.com/hashicorp/terraform-plugin-go/tftypes"

Expand All @@ -14,7 +15,8 @@ import (
)

var (
_ Float64Valuable = Float64Value{}
_ Float64Valuable = Float64Value{}
_ Float64ValuableWithSemanticEquals = Float64Value{}
)

// Float64Valuable extends attr.Value for float64 value types.
Expand Down Expand Up @@ -61,19 +63,20 @@ func NewFloat64Unknown() Float64Value {
}

// Float64Value creates a Float64 with a known value. Access the value via the Float64
// type ValueFloat64 method.
// type ValueFloat64 method. Passing a value of `NaN` will result in a panic.
//
// Setting the deprecated Float64 type Null, Unknown, or Value fields after
// creating a Float64 with this function has no effect.
func NewFloat64Value(value float64) Float64Value {
return Float64Value{
state: attr.ValueStateKnown,
value: value,
value: big.NewFloat(value),
}
}

// NewFloat64PointerValue creates a Float64 with a null value if nil or a known
// value. Access the value via the Float64 type ValueFloat64Pointer method.
// Passing a value of `NaN` will result in a panic.
func NewFloat64PointerValue(value *float64) Float64Value {
if value == nil {
return NewFloat64Null()
Expand All @@ -89,7 +92,29 @@ type Float64Value struct {
state attr.ValueState

// value contains the known value, if not null or unknown.
value float64
value *big.Float
}

// Float64SemanticEquals returns true if the given Float64Value is semantically equal to the current Float64Value.
// The underlying value *big.Float can store more precise float values then the Go built-in float64 type. This only
// compares the precision of the value that can be represented as the Go built-in float64, which is 53 bits of precision.
func (f Float64Value) Float64SemanticEquals(ctx context.Context, newValuable Float64Valuable) (bool, diag.Diagnostics) {
var diags diag.Diagnostics

newValue, ok := newValuable.(Float64Value)
if !ok {
diags.AddError(
"Semantic Equality Check Error",
"An unexpected value type was received while performing semantic equality checks. "+
"Please report this to the provider developers.\n\n"+
"Expected Value Type: "+fmt.Sprintf("%T", f)+"\n"+
"Got Value Type: "+fmt.Sprintf("%T", newValuable),
)

return false, diags
}

return f.ValueFloat64() == newValue.ValueFloat64(), diags
}

// Equal returns true if `other` is a Float64 and has the same value as `f`.
Expand All @@ -108,7 +133,12 @@ func (f Float64Value) Equal(other attr.Value) bool {
return true
}

return f.value == o.value
// Not possible to create a known Float64Value with a nil value, but check anyways
if f.value == nil || o.value == nil {
return f.value == o.value
}

return f.value.Cmp(o.value) == 0
}

// ToTerraformValue returns the data contained in the Float64 as a tftypes.Value.
Expand Down Expand Up @@ -156,13 +186,19 @@ func (f Float64Value) String() string {
return attr.NullValueString
}

return fmt.Sprintf("%f", f.value)
f64 := f.ValueFloat64()
return fmt.Sprintf("%f", f64)
}

// ValueFloat64 returns the known float64 value. If Float64 is null or unknown, returns
// 0.0.
func (f Float64Value) ValueFloat64() float64 {
return f.value
if f.IsNull() || f.IsUnknown() {
return float64(0.0)
}

f64, _ := f.value.Float64()
return f64
}

// ValueFloat64Pointer returns a pointer to the known float64 value, nil for a
Expand All @@ -172,7 +208,13 @@ func (f Float64Value) ValueFloat64Pointer() *float64 {
return nil
}

return &f.value
if f.IsUnknown() {
f64 := float64(0.0)
return &f64
}

f64, _ := f.value.Float64()
return &f64
}

// ToFloat64Value returns Float64.
Expand Down
154 changes: 148 additions & 6 deletions types/basetypes/float64_value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

Expand Down Expand Up @@ -97,14 +98,79 @@ func TestFloat64ValueEqual(t *testing.T) {
expectation bool
}
tests := map[string]testCase{
"known-known-same": {
input: NewFloat64Value(123),
candidate: NewFloat64Value(123),
"known-known-53-precison-same": {
input: NewFloat64Value(123.123),
candidate: NewFloat64Value(123.123),
expectation: true,
},
"known-known-diff": {
input: NewFloat64Value(123),
candidate: NewFloat64Value(456),
"known-known-53-precison-diff": {
input: NewFloat64Value(123.123),
candidate: NewFloat64Value(456.456),
expectation: false,
},
"known-known-512-precision-same": {
input: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001"),
},
candidate: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001"),
},
expectation: true,
},
"known-known-512-precision-diff": {
input: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001"),
},
candidate: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000009"),
},
expectation: false,
},
"known-known-512-precision-mantissa-diff": {
input: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001"),
},
candidate: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.01"),
},
expectation: false,
},
"known-known-precisiondiff-mantissa-same": {
input: NewFloat64Value(123),
candidate: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("123"),
},
expectation: true,
},
"known-known-precisiondiff-mantissa-diff": {
input: NewFloat64Value(0.1),
candidate: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.1"),
},
expectation: false,
},
"knownnil-known": {
input: Float64Value{
state: attr.ValueStateKnown,
value: nil,
},
candidate: NewFloat64Value(0.1),
expectation: false,
},
"known-knownnil": {
input: NewFloat64Value(0.1),
candidate: Float64Value{
state: attr.ValueStateKnown,
value: nil,
},
expectation: false,
},
"known-unknown": {
Expand Down Expand Up @@ -395,3 +461,79 @@ func TestNewFloat64PointerValue(t *testing.T) {
})
}
}

func TestFloat64ValueFloat64SemanticEquals(t *testing.T) {
t.Parallel()

testCases := map[string]struct {
currentFloat64 Float64Value
givenFloat64 Float64Value
expectedMatch bool
expectedDiags diag.Diagnostics
}{
"not equal - whole number": {
currentFloat64: NewFloat64Value(1),
givenFloat64: NewFloat64Value(2),
expectedMatch: false,
},
"not equal - float": {
currentFloat64: NewFloat64Value(1.1),
givenFloat64: NewFloat64Value(1.2),
expectedMatch: false,
},
"not equal - float differing precision": {
currentFloat64: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.01"),
},
givenFloat64: NewFloat64Value(0.02),
expectedMatch: false,
},
"semantically equal - whole number": {
currentFloat64: NewFloat64Value(1),
givenFloat64: NewFloat64Value(1),
expectedMatch: true,
},
"semantically equal - float": {
currentFloat64: NewFloat64Value(1.1),
givenFloat64: NewFloat64Value(1.1),
expectedMatch: true,
},
"semantically equal - float differing precision": {
currentFloat64: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.01"),
},
givenFloat64: NewFloat64Value(0.01),
expectedMatch: true,
},
// Only 53 bits of precision are compared, Go built-in float64
"semantically equal - float 512 precision, different value not significant": {
currentFloat64: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001"),
},
givenFloat64: Float64Value{
state: attr.ValueStateKnown,
value: testMustParseFloat("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000009"),
},
expectedMatch: true,
},
}
for name, testCase := range testCases {
name, testCase := name, testCase
t.Run(name, func(t *testing.T) {
t.Parallel()

match, diags := testCase.currentFloat64.Float64SemanticEquals(context.Background(), testCase.givenFloat64)

if testCase.expectedMatch != match {
t.Errorf("Expected Float64SemanticEquals to return: %t, but got: %t", testCase.expectedMatch, match)
}

if diff := cmp.Diff(diags, testCase.expectedDiags); diff != "" {
t.Errorf("Unexpected diagnostics (-got, +expected): %s", diff)
}
})
}
}