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

Document unsigned numeric type MySQL/MariaDB to Trino mapping #17181

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

willshen
Copy link
Contributor

@willshen willshen commented Apr 21, 2023

Description

Document how unsigned numeric types from MySQL and MariaDB are mapped to Trino.

Additional context and related issues

As discussed in #14679

private static Optional<ColumnMapping> getUnsignedMapping(JdbcTypeHandle typeHandle)
{
if (typeHandle.getJdbcTypeName().isEmpty()) {
return Optional.empty();
}
String typeName = typeHandle.getJdbcTypeName().get();
if (typeName.equalsIgnoreCase("tinyint unsigned")) {
return Optional.of(smallintColumnMapping());
}
if (typeName.equalsIgnoreCase("smallint unsigned")) {
return Optional.of(integerColumnMapping());
}
if (typeName.equalsIgnoreCase("int unsigned")) {
return Optional.of(bigintColumnMapping());
}
if (typeName.equalsIgnoreCase("bigint unsigned")) {
return Optional.of(decimalColumnMapping(createDecimalType(20)));
}

switch (jdbcTypeName.toLowerCase(ENGLISH)) {
case "tinyint unsigned":
return Optional.of(smallintColumnMapping());
case "smallint unsigned":
return Optional.of(integerColumnMapping());
case "int unsigned":
return Optional.of(bigintColumnMapping());
case "bigint unsigned":
return Optional.of(decimalColumnMapping(createDecimalType(20)));

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Apr 21, 2023
@willshen willshen requested a review from mosabua April 21, 2023 19:53
@willshen
Copy link
Contributor Author

FYI @mosabua I followed the precedent set in https://github.com/trinodb/trino/blob/master/docs/src/main/sphinx/connector/memsql.rst#type-mapping to create a new row after BIGINT for BIGINT UNSIGNED.

@mosabua
Copy link
Member

mosabua commented Apr 21, 2023

Thank you so much. Can you please confirm that this is technically accurate @ebyhr.

Looking at getUnsignedMapping (see

private static Optional<ColumnMapping> getUnsignedMapping(JdbcTypeHandle typeHandle)
) we should maybe also document the unsigned variants of tinyint, smallint and int. At least for MariaDB ..

Just checked.. MySql is the same logic but different code structure..

@mosabua mosabua added the docs label Apr 21, 2023
@willshen willshen changed the title Document unsigned bigint MySQL/MariaDB to Trino mapping Document unsigned numeric type MySQL/MariaDB to Trino mapping Apr 21, 2023
@mosabua
Copy link
Member

mosabua commented Apr 21, 2023

That was fast @willshen .. looks good to me. Once @ebyhr or another engineer confirm .. this looks good to go from my point of view.

@ebyhr ebyhr force-pushed the unsigned-bigint-doc branch from df358cc to a93051f Compare April 21, 2023 23:14
@ebyhr ebyhr merged commit fe5573f into trinodb:master Apr 21, 2023
@github-actions github-actions bot added this to the 415 milestone Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants