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

support scientific notation big floats #1809

Conversation

yungtrizzle
Copy link
Contributor

@yungtrizzle yungtrizzle commented Nov 23, 2023

Adds some support following discovery of #1758 by

  1. detecting scientific notation
  2. scan into big/Float
  3. replace original string with a true numeric string
  4. ensure using minimal significant figures after the decimal

@yungtrizzle yungtrizzle marked this pull request as ready for review November 23, 2023 23:44
@jackc
Copy link
Owner

jackc commented Nov 25, 2023

Does PostgreSQL ever return numerics in scientific notation? If not, I'm not sure that Scan is the right place for that logic.

@yungtrizzle
Copy link
Contributor Author

Does PostgreSQL ever return numerics in scientific notation? If not, I'm not sure that Scan is the right place for that logic.

Not necessarily but Scan is the only method to instantiate a numeric when it's not a returned value from a query. This is a common case for sending numerics to PostgreSQL

@jackc
Copy link
Owner

jackc commented Dec 2, 2023

Would it make sense to instead add a ScanScientific or something like that? It seems slightly off to me to make a function whose purpose is to scan values PostgreSQL produces also do something else. Numeric already has a couple Scan* methods so it wouldn't be unprecedented.

Also, I'm a little surprised that Numeric is being used directly. It is included so pgx can represent numeric values without an external dependency, but I expected anyone who was actually using PostgreSQL numerics would use something like https://github.com/shopspring/decimal.

@yungtrizzle
Copy link
Contributor Author

yungtrizzle commented Dec 4, 2023

I'm okay with making it a separate function, didn't realize the others were public. Let me push that update.

I expected anyone who was actually using PostgreSQL numerics would use something like https://github.com/shopspring/decimal.

In many cases, there isn't a need for all that it provides. In our case at $WORK, we only need to represent big numbers, no math needed so pulling it in is like using a sledgehammer to crack a nut. We've found that math/big + pgx is quite robust

@jackc jackc merged commit 20bf953 into jackc:master Dec 9, 2023
14 checks passed
@yungtrizzle yungtrizzle deleted the yungtrizzle-numeric-support-scientific-notation branch December 11, 2023 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants