Fix Float rejecting decimals like 144.75999999999999#1267
Conversation
TidyMaze
changed the title
|
Can you reformat with |
|
Right, done! |
|
damn, not compiling tests, sorry about that, next time will be the good one I hope |
|
Should be OK now |
Another opportunity for you to contribute 😉 |
| result.get("errors") should equal(None) | ||
| val data = result("data").asInstanceOf[Map[String, Any]] | ||
| val out = data("fieldWithFloatInput").asInstanceOf[String] | ||
| out should include("144.76") // nearest double string representation |
There was a problem hiding this comment.
Is it really the behavior we want? It's a bit surprising and dangerous. Do the spec mention something about this?
There was a problem hiding this comment.
The GraphQL spec says Float is "signed double-precision finite values" per IEEE 754, and for input:
"If the input value otherwise represents a value not representable by finite IEEE 754 (e.g. NaN, Infinity, or a value outside the available precision), a request error must be raised."
(GraphQL spec section 3.5.2 Float)
It does not define "outside the available precision." One reading is to reject only what is not a valid finite double (overflow, NaN, Infinity). Another is to reject any decimal that does not round-trip exactly. IEEE 754 uses a fixed 64-bit binary format, so many decimals (e.g. 0.1, 144.76) have no exact representation and rounding is built into the type.
Accepting and rounding to the nearest double can be surprising (e.g. 144.75999999999999 becomes the same bits as 144.76) and risky if people assume exact decimals (e.g. money). Rejecting all inexact decimals would avoid that but would mean rejecting 0.1, 0.2, 144.76, and many normal literals, which the spec does not clearly require. In that gap, a choice is to accept and round and only reject non-finite and overflow, because that fits a natural reading of "not representable by finite IEEE 754." The spec does not settle it. For exact decimals it points to String or a custom scalar. So I think it is reasonable to allow the small approximation and document that Double does not give exact decimal behaviour. I may be wrong though.
There was a problem hiding this comment.
What is your motivation in suggesting this change? Do you have a concrete use-case where the current implementation is an issue?
…malDouble - FloatType: coerce BigDecimal to double when result is finite instead of requiring isDecimalDouble (exact decimal round-trip). Fixes rejection of values like 144.75999999999999 that fit in double but have no exact decimal representation. - ResolverBasedAstSchemaBuilder: same logic for FloatType. - Tests: CoercionSpec (144.75999999999, 144.76, huge BigDecimal), ValueCoercionHelperSpec (GraphQL string literal), VariablesSpec (TestInputWithFloat variable round-trip). Made-with: Cursor
TidyMaze
force-pushed
the
approximated-float-parsing
branch
from
55dc361 to
d5e29b6
Compare
2 participants
Float inputs like
144.75999999999999were being rejected with "too big to fit in double" even though they fit in a double. The old logic required an exact decimal round-trip (isDecimalDouble), so any value that doesn’t round-trip exactly (common with JSON or GraphQL literals) failed.We now coerce to the nearest double and only reject when the value actually overflows to infinity. So those numbers work as expected, and the behaviour matches what people expect from a Float type. Fully backward compatible.
Technical change:
FloatTypeandResolverBasedAstSchemaBuilderuseisFinite(d.doubleValue)instead ofisDecimalDoubleforBigDecimal. Tests added in CoercionSpec, ValueCoercionHelperSpec, and VariablesSpec.