zenstackhq / zenstack

Fullstack TypeScript toolkit that enhances Prisma ORM with flexible Authorization layer for RBAC/ABAC/PBAC/ReBAC, offering auto-generated type-safe APIs and frontend hooks.
https://zenstack.dev
MIT License
2.07k stars 88 forks source link

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

Closed sitch closed 1 year ago

sitch commented 1 year ago

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):

ymc9 commented 1 year ago

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

sitch commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/zenstackhq/zenstack/blob/02a4a179b8c32d0320b27adbefca196cd983dd05/packages/schema/src/plugins/prisma/prisma-builder.ts#L293

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

model Example {
  epsilon Decimal @prisma.passthrough('@default(0.00000001)')
}
sitch commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Fixed in 1.0.0-beta.21