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

Invalid return type for functions with INT128 /Numeric(38)/ argument [CORE6344] #6585

Closed
firebird-issue-importer opened this issue Jun 24, 2020 · 12 comments

Comments

@firebird-issue-importer

Submitted by: @AlexPeshkoff

Some builtin functions, using int128 argument, should return same type value (CEIL, FLOOR). Others (like SQRT, LOG, EXP) should return decfloat(34) and use it for internal calculations. Currently all of them return and use internally double precision.

Commits: fd9600f 17b287f 57551c3 3192818 b728ff3 22d9194 3e8d810 FirebirdSQL/fbt-repository@2f69435

====== Test Details ======

See:
functional\datatypes\int128-math-functions.fbt
functional\datatypes\int128-agregate-functions.fbt

NOTE-1: currently (06-jun-2020) there are several issues related to TRUNC() and ROUND() results. See .fbt for details.
Test can be adjusted later after additional fixes.

NOTE-2:
Test for windowed functions deferred, see CORE6357

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jun 24, 2020

Modified by: @AlexPeshkoff

assignee: Alexander Peshkov [ alexpeshkoff ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jun 24, 2020

Commented by: @asfernandes

Because this change will make existing code (not updated with any direct usage of the new types) use them, I think it requires a compatibility option for client libraries not supporting the new types returned.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jun 25, 2020

Commented by: @AlexPeshkoff

It exists in B2, DataTypeCompatibility in firebird.conf

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 6, 2020

Commented by: @pavel-zotov

SQL> set heading off;
SQL> select cos( 9223372036854775807) from rdb$database;

0\.01180007651280024

But:

https://www.wolframalpha.com/input/?i=cos%289223372036854775807%29&assumption=%22TrigRD%22+-%3E+%22R%22

-- shows:
0.847788007348018782808156148748237678724694266423254799125...

PS

All fine for values up to 2^32 and even more:

SQL> select cos(2147483648) from rdb$database;

 0\.2378161945728034

SQL> select cos(2147499999) from rdb$database;

0\.6813789777108772

SQL> select cos(4294967296) from rdb$database;

\-0\.8868869151978210

SQL> select cos(5000000000) from rdb$database;

\-0\.9677602034277024
@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 6, 2020

Commented by: @pavel-zotov

1) select round(170141183460469231731687303715884105727,0) from rdb$database;
2) select round(170141183460469231731687303715884105727,1) from rdb$database;

Second statement issues:

Statement failed, SQLSTATE = 22003
arithmetic exception, numeric overflow, or string truncation
-numeric value is out of range

3) select round(-170141183460469231731687303715884105728,0) from rdb$database;

Statement failed, SQLSTATE = 22000
Decimal float invalid operation. An indeterminant error occurred during an operation.

4) select round(-170141183460469231731687303715884105728,1) from rdb$database;

Statement failed, SQLSTATE = 22003
arithmetic exception, numeric overflow, or string truncation
-numeric value is out of range

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 6, 2020

Modified by: @pavel-zotov

status: Open [ 1 ] => Open [ 1 ]

QA Status: No test => Covered by another test(s)

Test Details: See: functional\datatypes\int128-math-functions.fbt

NOTE: currently (06-jul-2020) there are several issues related to TRUNC() and ROUND() results. See .fbt for details.
Test can be adjusted later after additional fixes.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 6, 2020

Modified by: @pavel-zotov

status: Open [ 1 ] => Open [ 1 ]

Test Details: See: functional\datatypes\int128-math-functions.fbt

NOTE: currently (06-jul-2020) there are several issues related to TRUNC() and ROUND() results. See .fbt for details.
Test can be adjusted later after additional fixes.

=>

See:
functional\datatypes\int128-math-functions.fbt
functional\datatypes\int128-agregate-functions.fbt

NOTE: currently (06-jul-2020) there are several issues related to TRUNC() and ROUND() results. See .fbt for details.
Test can be adjusted later after additional fixes.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 6, 2020

Modified by: @pavel-zotov

status: Open [ 1 ] => Open [ 1 ]

Test Details: See:
functional\datatypes\int128-math-functions.fbt
functional\datatypes\int128-agregate-functions.fbt

NOTE: currently (06-jul-2020) there are several issues related to TRUNC() and ROUND() results. See .fbt for details.
Test can be adjusted later after additional fixes.

=>

See:
functional\datatypes\int128-math-functions.fbt
functional\datatypes\int128-agregate-functions.fbt

NOTE-1: currently (06-jun-2020) there are several issues related to TRUNC() and ROUND() results. See .fbt for details.
Test can be adjusted later after additional fixes.

NOTE-2:
Test for windowed functions deferred, see CORE6357

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 8, 2020

Commented by: @AlexPeshkoff

A cos() function always converts it's argument to double precision - therefore it's OK as long as argument's precision does not overflow double's precision. With values larger than it cos() as implemented in firebird becomes a kind of random generator, which is bad, but absolutely not related with this ticket.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 9, 2020

Commented by: @pavel-zotov

> A cos() function always converts it's argument to double precision - therefore it's OK as long as argument's precision does not overflow double's precision

COS() is periodical function, its full period 2*pi().

Using DECFLOAT34 datatype it is easy to get values for such numbers as 2^63 by emulating MOD() and substitute literal value for PI (first 39 digits) instead of call FB math function PI().

set sqlda_display on;
-- Value of PI from Wolfram: 3.14159265358979323846264338327950288419716939937510582097494459230781640628620899862803482534211706798214808651328...
recreate table test(x decfloat);
insert into test(x) values(9223372036854775807);

select
x as SOURCE_ANGLE_IN_RADIANS
,cos( x - floor( x / 3.141592653589793238462643383279502884197/2) * 3.141592653589793238462643383279502884197 * 2 ) as ALMOST_EXACT_COSINE
,cos( x - floor(x / pi() / 2) * pi() * 2 ) as TOTALLY_WRONG_COSINE
from test;

Output:

SOURCE_ANGLE_IN_RADIANS 9223372036854775807
ALMOST_EXACT_COSINE 0.8477880073480192
TOTALLY_WRONG_COSINE 0.2317799904467769

PS.

0xFF.
What about MOD() ? Why it was not implemented for full support of DECFLOAT datatype ?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 10, 2020

Commented by: @AlexPeshkoff

I slightly remember school trigonometry :)
To work well for all decfloat values pi should have number of digits comparable with max power of decfloat, i.e. abot 6000. Even for decimal/numeric max scale (127) existing in firebird high precision libraries are not enough.
Also I'm sure that if user needs to calculate trigonometric results for such huge values that means something is wrong, may be with his brain :D

0xff. MOD is also not supported for double precision, can you explain what means mod for *floating point* values?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 13, 2020

Modified by: @AlexPeshkoff

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: 4.0 RC 1 [ 10930 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants