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

Auto formats small default floats values into scientific notation breaking prisma syntax #646

Closed
sitch opened this issue Aug 23, 2023 · 12 comments

Comments

@sitch
Copy link
Contributor

sitch commented Aug 23, 2023

Description and expected behavior

Very small floats < 0.000001 and smaller translate into scientific notation in the @default field, reguardless of Decimal or Float

Zenstack

model Example {
  epsilon Decimal @default(0.00000001)
}

Becomes:

Prisma

model Example {
  epsilon Decimal @default(1e-8)
}

Which is invalid because prisma doesn't support scientific notation

Environment (please complete the following information):

  • ZenStack version: [latest beta 20]
  • Prisma version: [e.g., 4.8
  • Database type: [e.g. Postgresql]
@ymc9
Copy link
Member

ymc9 commented Aug 24, 2023

Thanks, good catch @sitch ! Will include it in the next release. Let me know if you're interested in making a PR 😄.

@sitch
Copy link
Contributor Author

sitch commented Aug 24, 2023

No prob @ymc9 -- I'd be more than happy to open a PR but I'm not familiar enough with the codebase to figure out where this goes in a time efficient fashion. It should just be some AST -> FloatDecl handler thing that should either use Decimal internally or modify some toString. Just point me in the right direction and I'll open up the PR

@ymc9
Copy link
Member

ymc9 commented Aug 24, 2023

No prob @ymc9 -- I'd be more than happy to open a PR but I'm not familiar enough with the codebase to figure out where this goes in a time efficient fashion. It should just be some AST -> FloatDecl handler thing that should either use Decimal internally or modify some toString. Just point me in the right direction and I'll open up the PR

Thank you for offering help @sitch !

I suspect the problem is in the PrismaBuilder: https://github.com/zenstackhq/zenstack/blob/02a4a179b8c32d0320b27adbefca196cd983dd05/packages/schema/src/plugins/prisma/prisma-builder.ts

It calls toString directly on number, but I don't really know how to suppress the e-notation output 😅

@sitch
Copy link
Contributor Author

sitch commented Aug 24, 2023

So after a little more investigation, this is fundamentally a JS quirk (what a lovely language). Any float 1e-7 or smaller automatically gets translated into scientific notation.

num = 0.000001
console.log(num)
// 0.000001
num = 0.0000001
console.log(num)
// 1e-7

There are some hacks that would work most of the time, but aren't concretely correct due to precision that has already been lost at parse time

function inferredPrecisionFloatToString(num: Number) : String {
  const exp = parseInt(num.toString().split("e-").slice(-1) ?? "0")
  return exp > 0 ? num.toFixed(exp) : num.toString()
}

So you would have:

inferredPrecisionFloatToString(0.00000001)
// "0.00000001"

Seems like this function could just be applied here:

But a better solution would be to get the raw token from the AST parse and use that or to not actually interpret the token as Number, but with Decimal internally

@sitch
Copy link
Contributor Author

sitch commented Aug 24, 2023

Also, a small discrepancy but:

In prisma you can do:

model Example {
  id String @id
  ep1 Decimal @default("0.00000001")
  ep2 Decimal @default(0.00000001)
  ep3 Float @default(0.00000001)
}

But in zmodel:

model Example {
  id String @id
  ep1 Decimal @default("0.00000001") // doesn't work `Value is not assignable to parameter`
  ep2 Decimal @default(0.00000001)
  ep3 Float @default(0.00000001)
}

@sitch
Copy link
Contributor Author

sitch commented Aug 24, 2023

Basically a "correct" solution becomes hairy at the prisma level bc a column can have custom precision settings which would then require validating the attribute value to see if it's string representation is representable in float space, given the columns precision settings

@sitch
Copy link
Contributor Author

sitch commented Aug 24, 2023

Seems like this solution needs to be elevated to the prisma repo. But here zenstack should at the very least support string types into the default decimal constructor and give a warning about using floats as default decimal values

@ymc9
Copy link
Member

ymc9 commented Aug 25, 2023

Many thanks for putting time into investigating this @sitch ! I agree with you the best solution seems to use the raw token to output to prisma schema, so at least we can be completely consistent with Prisma ... Yes, passing string values also need to be fixed.

Now the issue sounds more complex than I initially thought. I can further check how to change the parser part, but let me know if you plan to dig deeper there so we don't spend duplicated efforts 😄.

@ymc9
Copy link
Member

ymc9 commented Aug 25, 2023

Btw is this blocking you right now? A workaround could be:

model Example {
  epsilon Decimal @prisma.passthrough('@default(0.00000001)')
}

@sitch
Copy link
Contributor Author

sitch commented Aug 29, 2023

Btw is this blocking you right now? A workaround could be:

model Example {
  epsilon Decimal @prisma.passthrough('@default(0.00000001)')
}

This is what I'm currently doing

@sitch
Copy link
Contributor Author

sitch commented Aug 29, 2023

I think just updating the parser should be good. Also the zenstack number functions currently apply to decimal as well, but I'll open a separate ticket for that! Cheers @ymc9

@ymc9
Copy link
Member

ymc9 commented Sep 5, 2023

Fixed in 1.0.0-beta.21

@ymc9 ymc9 closed this as completed Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants