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

Scientific/exponential format silently behaves unexpectedly #2860

Closed
2 tasks done
dan-robertson opened this issue Jun 19, 2023 · 3 comments
Closed
2 tasks done

Scientific/exponential format silently behaves unexpectedly #2860

dan-robertson opened this issue Jun 19, 2023 · 3 comments
Labels
bug Invalid compiler output or panic compiler

Comments

@dan-robertson
Copy link

dan-robertson commented Jun 19, 2023

What happened?

Possibly a duplicate of #1118 (though this issue seems different in that these inputs were rejected before).

Entering numbers with scientific notation silently produces the wrong number.

I think instead such inputs should either be rejected or passed through to sql.

PRQL input

from numbers
select [ exponential = 1e9 ]

SQL output

SELECT
  19 AS exponential
FROM
  numbers

Expected SQL output

SELECT
  1E9 AS exponential
FROM
  numbers
-- Or the prql compiler rejects the input

MVCE confirmation

  • Minimal example
  • New issue

Anything else?

This was on the playground. Perhaps it is different in other versions.

@dan-robertson dan-robertson added the bug Invalid compiler output or panic label Jun 19, 2023
@eitsupi
Copy link
Member

eitsupi commented Jun 19, 2023

Hi, thank you for reporting this!
I can reproduce with the latest dev version.

@max-sixty
Copy link
Member

max-sixty commented Jun 19, 2023

Some options for the implementation:

  • Convert to float in rust, then convert to scientific when writing. But not trivial, likely requires heuristics on when to use scientific notation, and some float approximation.
    • We could also store smaller numbers as integers, up to 2^32 for i32...
  • Store as string
  • Raise an error

Probably string is easiest, float is best. Any thoughts from others?

@max-sixty
Copy link
Member

Actually converting to a float was easiest, in #2865

It currently will write the whole number out, which isn't ideal with huge numbers, but fixing that is a bit more work, as it requires writing a heuristic for when to express fully vs in scientific notation. (Contributions welcome if anyone wants to take a swing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Invalid compiler output or panic compiler
Projects
None yet
Development

No branches or pull requests

3 participants