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

Add numeric overload for math functions #15632

Open
surister opened this issue Feb 28, 2024 · 5 comments · May be fixed by #15644
Open

Add numeric overload for math functions #15632

surister opened this issue Feb 28, 2024 · 5 comments · May be fixed by #15644
Assignees

Comments

@surister
Copy link
Contributor

surister commented Feb 28, 2024

CrateDB version

5.5.2, 5.6.2

CrateDB setup information

I think it's not relevant.

Steps to Reproduce

SELECT
  ABS(CAST((1) AS NUMERIC) * SQRT(3))

Actual Result

SQLParseException[Cannot cast 1.7320508075688772 of type numeric to type byte

Expected Result

1.7320508075688772 (This query runs fine in Postgres)

@surister surister added the triage An issue that needs to be triaged by a maintainer label Feb 28, 2024
@proddata
Copy link
Member

Interesting .... I think most of the mathematical scalar functions are only implemented for primitive numerics:

  • DOUBLE
  • FLOAT
  • BYTE
  • SHORT
  • INTEGER
  • LONG
cr> SELECT abs(power(2,7)::NUMERIC);
SQLParseException[Cannot cast `128.0` of type `numeric` to type `byte`]

cr> SELECT abs(power(2,6)::NUMERIC);
+----+
| 64 |
+----+
| 64 |
+----+
SELECT 1 row in set (0.008 sec)

@surister
Copy link
Contributor Author

surister commented Feb 29, 2024

Right

SELECT abs(127::NUMERIC);
> 127

SELECT abs(128::NUMERIC);
> SQLParseException[Cannot cast `128` of type `numeric` to type `byte`]

SELECT ceil(128::NUMERIC);
> SQLParseException[Cannot cast `128` of type `numeric` to type `byte`]

SELECT ceil(128::INTEGER);
> 128

@jeeminso jeeminso added bug Clear identification of incorrect behaviour and removed triage An issue that needs to be triaged by a maintainer labels Mar 3, 2024
@jeeminso
Copy link
Contributor

jeeminso commented Mar 3, 2024

Thanks for reporting and further investigating @surister @proddata . I also think that we need to add abs(numeric) -> numeric variant which is required to prevent any loss of precision. We could reference https://www.postgresql.org/docs/current/functions-math.html to see what other scalars require f(numeric) -> numeric variant.

@jeeminso jeeminso linked a pull request Mar 3, 2024 that will close this issue
5 tasks
@surister
Copy link
Contributor Author

surister commented Mar 4, 2024

Confirmed:
abs
ceil
ceiling
exp
floor
round
sqrt

@jeeminso jeeminso self-assigned this Mar 4, 2024
@jeeminso
Copy link
Contributor

jeeminso commented Mar 6, 2024

This leads to breaking changes, for example round, CrateDB documents that it Returns: bigint or integer, however in order to be Postgres compatible, we need round(numeric) -> numeric overload. I think this could also be postponed to the next major release(Or 5.7? similar to https://github.com/crate/crate/pull/15643/files#r1510845633 cc @mfussenegger).

@jeeminso jeeminso changed the title Cannot cast number of type numeric into byte Add numeric overload for math functions Mar 6, 2024
@jeeminso jeeminso added breaking change and removed bug Clear identification of incorrect behaviour labels Mar 6, 2024
@jeeminso jeeminso removed their assignment Mar 6, 2024
@jeeminso jeeminso self-assigned this Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Candidates
Development

Successfully merging a pull request may close this issue.

3 participants