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.

Sign up for GitHub

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

fix: Handle proper type conversion on sort nodes. #228

Merged
merged 2 commits into from
Mar 2, 2022
Merged

fix: Handle proper type conversion on sort nodes. #228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63f227e
Handled CBOR encoding edge case
jsimnz Feb 24, 2022
097cc08
Fixed tests which manually casted results to uint64 due to previous e…
jsimnz Feb 24, 2022
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
7 changes: 6 additions & 1 deletion db/fetcher/versioned_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,11 @@ func compareVersionedDocs(t *testing.T, doc, expected map[string]interface{}) {
// make sure our floats are converted
if f, ok := expected[k].(float64); ok {
if f == float64(int64(f)) {
expected[k] = int64(f)
expected[k] = float64(f)
}

if u, ok := v.(uint64); ok {
v = float64(u)
}
}

Expand All @@ -396,6 +400,7 @@ func compareVersionedDocs(t *testing.T, doc, expected map[string]interface{}) {
v = int64(i)
}
}

assert.Equal(t, expected[k], v)
}
}
Expand Down
2 changes: 1 addition & 1 deletion db/tests/query/one_to_many/with_group_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func TestQueryOneToManyWithParentJoinGroupNumber(t *testing.T) {
"published": []map[string]interface{}{
{
"name": "Histoiare des Celtes et particulierement des Gaulois et des Germains depuis les temps fabuleux jusqua la prise de Roze par les Gaulois",
"rating": uint64(2),
"rating": float64(2),
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions db/tests/query/one_to_two_many/simple_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ func TestQueryOneToTwoManyWithNamedAndUnnamedRelationships(t *testing.T) {
},
"price": map[string]interface{}{
"currency": "SEK",
"value": uint64(129),
"value": float64(129),
},
},
{
Expand Down Expand Up @@ -414,7 +414,7 @@ func TestQueryOneToTwoManyWithNamedAndUnnamedRelationships(t *testing.T) {
{
"name": "A Time for Mercy",
"price": map[string]interface{}{
"value": uint64(129),
"value": float64(129),
},
},
{
Expand Down
14 changes: 14 additions & 0 deletions document/encoded.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,20 @@ func (e EncProperty) Decode() (core.CType, interface{}, error) {
}
val = stringArray
}
} else { // CBOR often encodes values typed as floats as ints
switch e.Desc.Kind {
case base.FieldKind_FLOAT:
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure all these cases are needed - only uint64 is hit according to codecov - it would be wierd if it used int/unit, but int64 might be due to lack of negative number testing. Happy(ier?) for them all to stay in for now, but maybe (in an ideal world with lots of free time) we could revist this

Copy link
Member Author

Choose a reason for hiding this comment

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

I know at the very least it uses int64 and uint64 depending on if its a negative number. Im pretty sure I can drop the other cases.

switch v := val.(type) {
case int64:
return ctype, float64(v), nil
case int:
return ctype, float64(v), nil
case uint64:
return ctype, float64(v), nil
case uint:
return ctype, float64(v), nil
}
}
}

return ctype, val, nil
Expand Down