Skip to content

Accept cdata number as float and forbid NaN/Inf #49

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 2 commits into from
Apr 4, 2022

Conversation

Totktonada
Copy link
Member

This pull request contains two changes related to processing of variables of the Float type.

Accept a cdata number as value of a Float variable

It is quite usual that a floating point number is encoded using JSON, goes over HTTP, received on tarantool server, decoded from JSON and passed as a variable into the GraphQL executor.

A number out of the [-10^14+1; 10^14-1] range is decoded into Lua as cdata number1 to don't loss the precision. A JSON decoder don't know that we intend to use this value as the floating point one and chooses the conservative option: decode the value into cdata number to don't loss the precision.

In GraphQL we know that it is actually the floating point value (disregarding its representation in Lua). So it looks correct to accept a cdata number as value of a Float variable.

The similar idea was expressed in tarantool/tarantool#5933 against tarantool itself: let it accept a number without fractional part as a value suitable for the float field type.

Forbid NaN and Infinity as a value Float type

The GraphQL specification explicitly forbids it:

Non-finite floating-point internal values (NaN and Infinity) cannot
be coerced to Float and must raise a field error.

http://spec.graphql.org/October2021/#sec-Float


Fixes #47
Supersedes PR #48

Footnotes

  1. cdata<int64_t> or cdata<uint64_t>

@Totktonada Totktonada requested a review from olegrok April 2, 2022 07:25
It is quite usual that a floating point number is encoded using JSON,
goes over HTTP, received on tarantool server, decoded from JSON and
passed as a variable into the GraphQL executor.

A number out of the [-10^14+1; 10^14-1] range is decoded into Lua as
cdata number[^1] to don't loss the precision. A JSON decoder don't know
that we intend to use this value as the floating point one and chooses
the conservative option: decode the value into cdata number to don't
loss the precision.

In GraphQL we know that it is actually the floating point value
(disregarding its representation in Lua). So it looks correct to accept
a cdata number as value of a Float variable.

The similar idea was expressed in [1] against tarantool itself: let it
accept a number without fractional part as a value suitable for the
`float` field type.

[1]: tarantool/tarantool#5933

[^1]: `cdata<int64_t>` or `cdata<uint64_t>`

Fixes #47
The GraphQL specification explicitly forbids it:

 | Non-finite floating-point internal values (NaN and Infinity) cannot
 | be coerced to Float and must raise a field error.

http://spec.graphql.org/October2021/#sec-Float

Follows up #47
@Totktonada Totktonada force-pushed the Totktonada/accept-cdata-number-as-float branch from 6b906d2 to ec48eac Compare April 4, 2022 13:25
@Totktonada Totktonada merged commit 4620410 into master Apr 4, 2022
@Totktonada Totktonada deleted the Totktonada/accept-cdata-number-as-float branch April 4, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong variable "var" for the Scalar "Float" if value is greater than 10^15
3 participants