HHH-14711 unsigned types causing out of range on MySQL/MariaDB

[peter1123581321]

MySQL fails for tinyint, smallint, integer and bigint, but works correctly for mediumint.
MariaDB fails for bigint, but works correctly for all others.
The MariaDB implementation of ResultSetMetaData#getColumnType() differs from the MySQL implementation.
MariaDB has a more accurate implementation concerning unsigned data types.

As MariaDbDialect extends MySQLDialect, the bugfix was implemented in MySQLDialect#resolveSqlTypeDescriptor().

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

---

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

[gavinking]

I would say that this issue is only worth fixing if we can fix it in a clean way.

Please see my comments.

[gavinking]

unsigned types are not, AFAIK, a standard or common feature of SQL databases, so this implementation doesn't belong as the default implementation on Dialect.

[gavinking]

Woah. This is ... a bit ugly. Surely there must be some more elegant way to do this?

Why not just directly switch on the columnTypeName, since you're depending on it anyway?

[gavinking]

Actually, @peter1123581321, is this a breaking change to existing native SQL queries which are expecting the narrower types?

[gavinking]

@peter1123581321 see my comment here: https://hibernate.atlassian.net/browse/HHH-14711?focusedCommentId=122112

[peter1123581321]

Many thanks Gavin for your comments on this PR.

1. I agree, the switch statement is ... a bit ugly. :D
   Unfortunately, as mentioned above, MariaDB handles unsigned data types differently than MySQL,
   that's the main reason why the fix is not very straight forward.
   I also thought about splitting the code up to both dialects, but that would propably lead to a bug some day when the Mysql implementation of getColumnType matches the more accurate one of MariaDB.

2. The more I think about it, the more I think that this should be fixed in the connectors. And as you said, ANSI SQL has no unsigned numeric types. So propably there is no real need to implement this workaround here.

3. I think I did not get your point of 2) in your comment on Jira. IMO it makes totally sense to map a unsigned datatype to the next larger one, e.g. tinyint to a smallint. Why should we use an even greater one?

4. Not sure if it can be considered as really breaking. If you define the column e.g. as usigned tinyint (0-255) with java type byte, but always only use the half range from (0-127), yes, in such cases it is breaking. Otherwise I think you would have got the out of range exception and changed your design.

Summarized, I also would suggest to close this bug.
If you agree, I would adopt the PR to only submit the tests for the cases for which the unsigned data types and the native queries work as expected (I did not have found any other tests that cover this situation).

[gavinking]

I think I did not get your point of 2) in your comment on Jira. IMO it makes totally sense to map an unsigned datatype to the next larger one, e.g. tinyint to a smallint. Why should we use an even greater one?

I'm not referring to what you're doing here, which is, well, OK.

I was referring to creating a database schema where you have just one bit of headroom for your numeric types. That's a bad idea, since it's incredibly rare for someone to have such a good estimate of the largest possible value for a column.

Not sure if it can be considered as really breaking. If you define the column e.g. as usigned tinyint (0-255) with java type byte, but always only use the half range from (0-127), yes, in such cases it is breaking.

And that's probably a pretty common scenario, since numeric columns are typically specified with a lot of headroom (i.e. more than one bit).