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.

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

Cannot insert Floats (≥9.223372037e18 and <1e21) or (>-1e21 and ≤-9.223372037e18) #12651

Closed
cmd-johnson opened this issue Apr 4, 2022 · 4 comments · Fixed by #15879
Closed
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. tech/typescript Issue for tech TypeScript. topic: floating point types Topic related to floating point types and precision loss topic: Prisma's GraphQL protocol Prisma Client ↔ Query Engine communication protocol for queries
Milestone

Comments

@cmd-johnson
Copy link
Contributor

Bug description

Trying to set fields defined as Float in the Schema to values ≥ 9.223372037e18 and < 1e21 fails with the following error:

Error parsing GraphQL query: query parse error: Parse error at 3:11
number too large to fit in target type

Additionally, trying to set values > -1e21 and ≤ -9.223372037e18 throws as well:

Error parsing GraphQL query: query parse error: Parse error at 3:11
number too small to fit in target type

I've first encountered this with postgresql, but it happens with sqlite as well.

I'm guessing there is something going wrong while parsing the generated GraphQL query.
Calling the reproduction script with 1e20 throws while trying to execute the following query that was generated by prisma:client:

mutation {
  createOneTest(data: {
    test: 100000000000000000000
  }) {
    id
    test
  }
}

Running that same query manually through the query-engine's GraphQL playground yields the same error, but when writing the value in exponential notation (1e20), the mutation works just fine.

How to reproduce

  1. clone https://github.com/cmd-johnson/prisma-bug-reproduction
  2. install the project (either run npm install or yarn install)
    • The project's postinstall hook will automatically create a test.db sqlite DB and apply the only required migration
  3. running node index.js [number] will attempt to insert the value you pass it as an argument into the database. Try it with:
    value result
    1e20 error ("too large")
    1e21 ok
    9.223372037e18 error ("too large")
    9.223372036e18 ok
    -1e20 error ("too small")
    -1e21 ok
    -9.223372037e18 error ("too small")
    -9.223372036e18 ok

This can also be reproduced when running the Prisma query-engine directly, with some key differences:

  1. clone https://github.com/cmd-johnson/prisma-bug-reproduction
  2. in the project directory, run query-engine --datamodel-path ./schema.prisma -g
  3. go to http://localhost:4466/
    value result
    1e20 ok
    100000000000000000000 (== 1e20, but written out) error ("too large")

Expected behavior

All examples should work without throwing.

Prisma information

Schema:

generator client {
  provider = "prisma-client-js"
}

datasource db {
  url = "file:./test.db"
  provider = "sqlite"
}

model Test {
  id Int @id @default(autoincrement())
  test Float
}

Code: (see reproduction repo for more)

await prisma.test.create({ data: { test: 1e20 } })

Environment & setup

  • OS: Linux (Manjaro)
  • Database: PostgreSQL or SQLite, others probably as well
  • Node.js version: v17.8.0

Prisma Version

prisma                  : 3.11.1
@prisma/client          : 3.11.1
Current platform        : debian-openssl-1.1.x
Query Engine (Node-API) : libquery-engine 1a2506facaf1a4727b7c26850735e88ec779dee9 (at node_modules/@prisma/engines/libquery_engine-debian-openssl-1.1.x.so.node)
Migration Engine        : migration-engine-cli 1a2506facaf1a4727b7c26850735e88ec779dee9 (at node_modules/@prisma/engines/migration-engine-debian-openssl-1.1.x)
Introspection Engine    : introspection-core 1a2506facaf1a4727b7c26850735e88ec779dee9 (at node_modules/@prisma/engines/introspection-engine-debian-openssl-1.1.x)
Format Binary           : prisma-fmt 1a2506facaf1a4727b7c26850735e88ec779dee9 (at node_modules/@prisma/engines/prisma-fmt-debian-openssl-1.1.x)
Default Engines Hash    : 1a2506facaf1a4727b7c26850735e88ec779dee9
Studio                  : 0.458.0
@cmd-johnson cmd-johnson added the kind/bug A reported bug. label Apr 4, 2022
@janpio janpio added team/client Issue for team Client. bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. topic: floating point types Topic related to floating point types and precision loss topic: Prisma's GraphQL protocol Prisma Client ↔ Query Engine communication protocol for queries labels Apr 4, 2022
@cmd-johnson
Copy link
Contributor Author

My guess on the problem source is that JS by default only serializes numbers with an exponent >20 in scientific notation.
9.223372037e18 doesn't have a decimal separator when written out (9223372037000000000 (it's way larger than Number.MAX_SAFE_INTEGER as well)).
Thus, the GraphQL parser sees a \d+ and tries to deserialize it as an i64 (with a max value of 9223372036854775807, which is slightly less than 9.223372037e18) and fails.

@dpetrick
Copy link
Contributor

dpetrick commented Jun 1, 2022

With 3.15 a change ships that will surface better errors, but doesn't fully address this ticket yet. A follow-up ticket will fix that we currently can't utilise the entire float range because of GraphQL serialisation issues.

@Weakky Weakky added the tech/typescript Issue for tech TypeScript. label Jun 21, 2022
@Jolg42 Jolg42 added the topic: floating point types Topic related to floating point types and precision loss label Aug 31, 2022
@aqrln
Copy link
Member

aqrln commented Sep 1, 2022

Related: #13317

@pimeys pimeys self-assigned this Sep 2, 2022
@pimeys
Copy link
Contributor

pimeys commented Sep 2, 2022

I can reproduce this, even with the latest Prisma version at this point (4.3.1). Something to do with the validation logic.

@pimeys pimeys added bug/2-confirmed Bug has been reproduced and confirmed. and removed bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. labels Sep 2, 2022
@pimeys pimeys removed their assignment Sep 2, 2022
@Druue Druue assigned Druue and unassigned Druue Oct 13, 2022
cmd-johnson added a commit to cmd-johnson/prisma that referenced this issue Oct 19, 2022
cmd-johnson added a commit to cmd-johnson/prisma that referenced this issue Oct 19, 2022
cmd-johnson added a commit to cmd-johnson/prisma that referenced this issue Oct 19, 2022
SevInf added a commit that referenced this issue Oct 19, 2022
* test(client): unit test for #12651

* test(client): integration test for #12651

* fix(client): Serialize all floats in exponential notation

Fix #12651, fix #13317

* test(internals): Fix normalizeMigrateTimestamps applying to all long numbers

* test(client): Convert large-floats tests to functional

* test(client): Revert snapshot serializer changes

It does not seem that it was indendened for snapshot serizier to apply
to select.test.ts. Instead, we are fixing the failure by changing the
number so it won't be replaced by serializer.

* Revert "test(client): Revert snapshot serializer changes"

This reverts commit 7756be2.

* Update packages/internals/src/utils/jestSnapshotSerializer.js

Co-authored-by: Sergey Tatarintsev <sergey@tatarintsev.me>
Co-authored-by: Alexey Orlenko <alex@aqrln.net>
@janpio janpio added this to the 4.6.0 milestone Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. tech/typescript Issue for tech TypeScript. topic: floating point types Topic related to floating point types and precision loss topic: Prisma's GraphQL protocol Prisma Client ↔ Query Engine communication protocol for queries
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants