ISPN-14138 SQL Store fail to persist numbers with fractional portion #10308
Conversation
|
@gustavolira Do you think we can test this on the various DB providers? |
|
Hey @wburns and @gustavolira, does the test suite have tests that store and retrieve each kind of data type? Given this issue and the one with Booleans, it seems like it would be good to have! |
That is what I have expanded upon with the recent tests. To be honest my problem is I don't know what all the various types a user would use are :( But I can try to find others that we aren't testing, like I know float isn't explicitly tested, but should work. |
wburns
force-pushed
the
numeric_sql_store
branch
4 times, most recently
from
f5d1e98
to
eecba36
Compare
wburns
changed the title
|
Updated to make sure Numeric with decimals work, added support for Real and Decimal types. |
| @@ -110,6 +110,9 @@ int typeWeUse(int sqlType, String typeName) { | |||
| } else if (typeName.toUpperCase().startsWith("BOOL")) { | |||
| // Some databases store as int32 or something similar but have the typename as BOOLEAN or some derivation | |||
| return Types.BOOLEAN; | |||
| } else if (sqlType == Types.NUMERIC && scale == 0) { | |||
| // If scale is 0 we don't want to use float or double types | |||
| return Types.INTEGER; | |||
There was a problem hiding this comment.
Just to be on the safe side, should we use a BIGINT for this case or it is overkill?
There was a problem hiding this comment.
This keeps it inline with what we had before, as NUMERIC was always being assigned to an INTEGER (which is required for Oracle "boolean" types). The big difference is if the scale is not 0 we assign it to DOUBLE protostream type.
We could possibly assign it to BIGINT if the precision is higher though. I can try to prototype something like that. I had thought about it before but was trying to change as little as possible.
|
@wburns conflicts need to be resolved |
|
Yeah, I already resolved, was working on changing the types. But I am not having luck, I am going to say for this PR just leave it as is and we can enhance it further later. |
wburns
force-pushed
the
numeric_sql_store
branch
from
eecba36
to
03e4af6
Compare
|
Rebased. |
|
@wburns Couple of related failures in CI. |
Ah, yes, good catch.. Fixing. |
* Added support for Numeric * Added support for Decimal * Fixed Numeric to support decimals ** Numeric with 0 scale is still Integer
wburns
force-pushed
the
numeric_sql_store
branch
from
03e4af6
to
bd26131
Compare
|
Tests should be good now. |
|
Merged, thanks @wburns |
https://issues.redhat.com/browse/ISPN-14138