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
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: Fix Float64Attribute bug by preserving precision from number
values'
time: 2023-08-03T12:04:11.996955-04:00
custom:
Issue: "817"
17 changes: 12 additions & 5 deletions types/basetypes/float64_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,26 +136,33 @@ func (t Float64Type) ValueFromTerraform(ctx context.Context, in tftypes.Value) (

var bigF *big.Float
err := in.As(&bigF)

if err != nil {
return nil, err
}

f, accuracy := bigF.Float64()
// Copy the float retrieved from tftypes.Value to avoid any unexpected pass-by-reference shenanigans
copiedBigF := new(big.Float)
copiedBigF.Copy(bigF)

f, accuracy := copiedBigF.Float64()

// Underflow
// Reference: https://pkg.go.dev/math/big#Float.Float64
if f == 0 && accuracy != big.Exact {
return nil, fmt.Errorf("Value %s cannot be represented as a 64-bit floating point.", bigF)
return nil, fmt.Errorf("Value %s cannot be represented as a 64-bit floating point.", copiedBigF)
}

// Overflow
// Reference: https://pkg.go.dev/math/big#Float.Float64
if math.IsInf(f, 0) {
return nil, fmt.Errorf("Value %s cannot be represented as a 64-bit floating point.", bigF)
return nil, fmt.Errorf("Value %s cannot be represented as a 64-bit floating point.", copiedBigF)
}

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: copiedBigF,
}, 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
50 changes: 43 additions & 7 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 @@ -68,7 +70,7 @@ func NewFloat64Unknown() Float64Value {
func NewFloat64Value(value float64) Float64Value {
return Float64Value{
state: attr.ValueStateKnown,
value: value,
value: big.NewFloat(value),
}
}

Expand All @@ -89,7 +91,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 +132,7 @@ func (f Float64Value) Equal(other attr.Value) bool {
return true
}

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 +180,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 +202,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
138 changes: 132 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,63 @@ 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,
},
"known-unknown": {
Expand Down Expand Up @@ -395,3 +445,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)
}
})
}
}