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

Conversion from zero numeric literals to DECFLOAT results in incorrect value [CORE5696] #5962

Closed
firebird-automations opened this issue Jan 6, 2018 · 14 comments

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: @mrotteveel

Depends on CORE6060

Zero numeric literals do not correctly convert to DECFLOAT (test with inserting a value into a DECFLOAT(16) column).

literal => resulting decfloat value || expected decfloat value
-0 => 0 || -0
-0E300 => -0E-16 || -0E+300
-0E+300 => -0E-16 || -0E+300
-0.0E+300 => -0E-16 || -0E+300
0E300 => -0E-16 || 0E+300
0E+300 => -0E-16 || 0E+300
0E-300 => 0E-16 || 0E-300

Using the equivalent string and casting that to DECFLOAT (decfloat(16) in my tests) does result in the correct value (except for the -0.0E+300 case).

However conversion from string to decfloat also has its problems:

'0.0E300' => 0E299 || 0E300 (or 0.0E300)

Commits: 59ebe83 5d70d4a FirebirdSQL/jaybird@0982555

@firebird-automations
Copy link
Collaborator Author

Modified by: @mrotteveel

description: Zero numeric literals do not correctly convert to DECFLOAT (test with inserting a value into a DECFLOAT(16) column).

literal => resulting decfloat value || expected decfloat value
-0 => 0 || -0
-0E300 => -0E-16 || -0E+300
-0E+300 => -0E-16 || -0E+300
-0.0E+300 => -0E-16 || -0E+300
0E300 => -0E-16 || 0E+300
0E+300 => -0E-16 || 0E+300

Using the equivalent string and casting that to DECFLOAT (decfloat(16) in my tests) does result in the correct value.

However conversion from string to decfloat also has its problems:

'0.0E300' => 0E299 || 0E300 (or 0.0E300)

=>

Zero numeric literals do not correctly convert to DECFLOAT (test with inserting a value into a DECFLOAT(16) column).

literal => resulting decfloat value || expected decfloat value
-0 => 0 || -0
-0E300 => -0E-16 || -0E+300
-0E+300 => -0E-16 || -0E+300
-0.0E+300 => -0E-16 || -0E+300
0E300 => -0E-16 || 0E+300
0E+300 => -0E-16 || 0E+300
0E-300 => 0E-16 || 0E-300

Using the equivalent string and casting that to DECFLOAT (decfloat(16) in my tests) does result in the correct value (except for the -0.0E+300 case).

However conversion from string to decfloat also has its problems:

'0.0E300' => 0E299 || 0E300 (or 0.0E300)

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

assignee: Alexander Peshkov [ alexpeshkoff ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

Link: This issue depends on CORE6060 [ CORE6060 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

Mark, I must say that for 0.0E300 canonical form is 0E299, i.e. this result appears right to me.

@firebird-automations
Copy link
Collaborator Author

Commented by: @mrotteveel

@alex, looking at http://speleotrove.com/decimal/daconvs.html#reftonum you're indeed right that 0.0E300 should yield 0E299, however that wasn't the main problem reported in this ticket. The main problem was conversions like 0E300 yielding -0E-16.

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

Mark I do not close the tickets till your confirmation that everything is OK.

Re your question (how can literals work OK for both double prec & decfloat) - I always keep suspicious literals as initial string till I can decide about type of target, only after it literal is converted to appropriate numeric value.

I kindly ask you to check - may be there are some more wrong cases, both regarding format of literal and use fo it in SQL code.

@firebird-automations
Copy link
Collaborator Author

Commented by: @mrotteveel

It works for DECFLOAT(16), but for DECFLOAT(34), values seem to be restricted to the DECFLOAT(16) limits.

For example:
literal => resulting decfloat34 value || expected decfloat34 value
0E+370 => 0E+369 || 0E+370
0E-399 => 0E-398 || 0E-399
0E+6111 => 0E+369 || 0E+6111
0E-6167 => 0E-369 || 0E-6176
1E-6176 => 0E-369 || 1E-6176 (or 1.000000000000000000000000000000000E-6176 or 1000000000000000000000000000000000E-6143)
1E+6111=> Overflow || 1E+6111
1E+6144 => Overflow || 1E+6144 (or 1.000000000000000000000000000000000E+6144)
1.234567890123456789012345678901234E0 => 1.234567890123457 || 1.234567890123456789012345678901234

Using a string literal to populate the value will work as expected.

@firebird-automations
Copy link
Collaborator Author

Commented by: @mrotteveel

This change also broke existing tests for Jaybird (DecfloatSupportTest.simpleValuesSelect()), which perform an insert into a DECFLOAT(34) with the all-nine maximum value 9999999999999999999999999999999999E+6111 (or 9.999999999999999999999999999999999E+6144)

insert into decfloattest(id, decfloat16, decfloat34) values (3, 9999999999999999E+369, 9999999999999999999999999999999999E+6111)

This now yields a "Decimal float overflow. The exponent of a result is greater than the magnitude allowed.".

Changing the literal to a string ('9999999999999999999999999999999999E+6111') allows to test to work.

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

That's fixed (trivial copy/paste error). Thanks for testing, and once again wait for your confirmation.

@firebird-automations
Copy link
Collaborator Author

Commented by: @mrotteveel

Thanks, I'll test it this weekend.

@firebird-automations
Copy link
Collaborator Author

Commented by: @mrotteveel

Re-tested on Firebird-4.0.0.1556-0_x64 and works ok for DECFLOAT(34) now as well.

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

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

QA Status: No test => Done successfully

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

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

resolution: Fixed [ 1 ]

Fix Version: 4.0 Beta 2 [ 10888 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Closed [ 6 ]

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

No branches or pull requests

2 participants