Skip to content

Make type name to be Optional in JdbcTypeHandle#523

Merged
kokosing merged 1 commit intotrinodb:masterfrom
kokosing:origin/master/112_optional_type_name
Apr 24, 2019
Merged

Make type name to be Optional in JdbcTypeHandle#523
kokosing merged 1 commit intotrinodb:masterfrom
kokosing:origin/master/112_optional_type_name

Conversation

@kokosing
Copy link
Member

Make type name to be Optional in JdbcTypeHandle

In some RDMBS there are types that do not have names.

@cla-bot cla-bot bot added the cla-signed label Mar 22, 2019
@kokosing kokosing changed the title Make type name to be Optional in JdbcTypeHandle [WIP] Make type name to be Optional in JdbcTypeHandle Mar 22, 2019
Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code doesn't compile

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.0:compile (default-compile) on project presto-postgresql: Compilation failure: Compilation failure: 
[ERROR] /home/travis/build/prestosql/presto/presto-postgresql/src/main/java/io/prestosql/plugin/postgresql/PostgreSqlClient.java:[141,16] incompatible types: java.util.Optional<java.lang.String> cannot be converted to int
[ERROR] /home/travis/build/prestosql/presto/presto-postgresql/src/main/java/io/prestosql/plugin/postgresql/PostgreSqlClient.java:[148,84] incompatible types: java.util.Optional<java.lang.String> cannot be converted to java.lang.String

@kokosing kokosing force-pushed the origin/master/112_optional_type_name branch from 0fb71de to 2e17a55 Compare March 26, 2019 11:37
@kokosing kokosing changed the title [WIP] Make type name to be Optional in JdbcTypeHandle Make type name to be Optional in JdbcTypeHandle Mar 27, 2019
@kokosing
Copy link
Member Author

@findepi This is ready for a review.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go to super.toPrestoType.
However, since we know postgresql types are properly named we can simply

String jdbcTypeName = typeHandle.getJdbcTypeName().orElseThrow

then you don't need if (!typeHandle.getJdbcTypeName().isPresent()) at all

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super.toPrestoType does not use getJdbcTypeName

@kokosing kokosing force-pushed the origin/master/112_optional_type_name branch from 2e17a55 to af1fb81 Compare April 9, 2019 08:33
Copy link
Member Author

@kokosing kokosing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CA

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super.toPrestoType does not use getJdbcTypeName

@kokosing kokosing force-pushed the origin/master/112_optional_type_name branch from 8ef6a8f to e26f960 Compare April 23, 2019 07:20
In some RDMBS there are types that do not have names.
@kokosing kokosing force-pushed the origin/master/112_optional_type_name branch from d5e3b10 to ce00ad0 Compare April 23, 2019 10:46
@kokosing kokosing merged commit 694a68c into trinodb:master Apr 24, 2019
@kokosing kokosing deleted the origin/master/112_optional_type_name branch April 24, 2019 07:14
v-jizhang added a commit to v-jizhang/presto that referenced this pull request Aug 15, 2023
Cherry-pick of trinodb/trino#523

Co-authored-by: Grzegorz Kokosiński <grzegorz@starburstdata.com>
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.

2 participants