HHH-18150 Fix for Informix float and double precision in decimal digits

[VladoKuruc]

I propose using float precision 8 since the documentation smallfoat documentation states that the SMALLFLOAT data type stores single-precision floating-point numbers with approximately nine significant digits.

https://hibernate.atlassian.net/browse/HHH-18150

[gavinking]

Have you tested this?

[VladoKuruc]

Have you tested this?

Yes, with Informix 14 and Informix 11

[gavinking]

OK, great, LGTM, thanks.

[hibernate-github-bot[bot]]

Thanks for your pull request!

This pull request appears to follow the contribution rules.

› This message was automatically generated.

[beikov]

A much simpler solution would be to change the registration from CapacityDependentDdlType.builder( FLOAT, "float($p)", this ) to CapacityDependentDdlType.builder( FLOAT, "float", this ) since the documentation clearly states that the precision is ignored anyway.

[gavinking]

A much simpler solution would be to change the registration from CapacityDependentDdlType.builder( FLOAT, "float($p)", this ) to CapacityDependentDdlType.builder( FLOAT, "float", this ) since the documentation clearly states that the precision is ignored anyway.

I think the point is that smallfloat -> single precision, float -> double precision.

I don't think small float is the same type as float.

[beikov]

A much simpler solution would be to change the registration from CapacityDependentDdlType.builder( FLOAT, "float($p)", this ) to CapacityDependentDdlType.builder( FLOAT, "float", this ) since the documentation clearly states that the precision is ignored anyway.

I think the point is that smallfloat -> single precision, float -> double precision.

I don't think small float is the same type as float.

Correct, but I didn't doubt this, so not sure what you mean.

AFAIU, the users problem is that we currently pass binary digits as precision value when creating DDL for double precision and produce e.g. float(52). According to the docs, this is invalid, because the value range is 1 - 14 which I guess stands for decimal digits. Ultimately, it's not worth bothering though, because the docs say the DB ignores the value passed, and the DDL type float always means 8 byte floating point number, hence my suggestion to just remove the ($p) from the registration.

[beikov]

We could do the same simplification for e.g. PostgreSQL since the parametrizations of float are essentially just aliases as well according to its documentation.

[gavinking]

What I'm saying is that:

• we should produce smallfloat when the requested precision is InformixDialect.getFloatPrecision(), and
• float when it is InformixDialect.getDoublePrecision().

Otherwise we will be unnecessarily creating float columns when smallfloat columns would suffice.

[gavinking]

Actually perhaps you were merely suggesting to just change that one line of the fix, and leave everything else the same. If that's the case, I agree.

It sounded like you were suggesting that one-line change as an alternative simpler solution instead of the whole patch. But I probably misunderstood your comment.

[gavinking]

We could do the same simplification for e.g. PostgreSQL since the parametrizations of float are essentially just aliases as well according to its documentation.

Oh god, please no. I very deliberately moved us away from that mess of real, float, double that we used to generate, doing something different on every db, since real and float (with no explicit precision) have no fixed interpretation, to something that's uniform and consistent across all dialects.

OK, so that more uniform approach doesn't work so well on Informix, and that's fine. But it does work perfectly well on PostgreSQL and on most other databases, and is ANSI standard SQL.

[beikov]

It sounded like you were suggesting that one-line change as an alternative simpler solution instead of the whole patch. But I probably misunderstood your comment.

I am suggesting to that instead. The PR as it is right now will emit float(16) for double, which will also fail according to the docs, because the maximum allowed value for this database is 14. The thing is, why not just drop the value since it has no meaning anyway as long as it is in range?

[VladoKuruc]

The documentation states a range of 1-14, but the error message says "Maximum precision value for FLOAT data type must be between 1 and 16." I have verified that an error occurs when attempting to set the precision to 17 or higher.

[beikov]

Please rebase the PR, then we can merge it.

[VladoKuruc]

Please rebase the PR, then we can merge it.

I've done a rebase, can you merge it?

[gavinking]

Thanks, @VladoKuruc!