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.

Sign up for GitHub

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

Jump to bottom

Remove unsupported types in JDBC connectors #12231

Closed
wants to merge 2 commits into from
Closed

Remove unsupported types in JDBC connectors #12231

wants to merge 2 commits into from

Conversation

findepi
Copy link
Contributor

@findepi findepi commented Jan 15, 2019

Extracted from #12151

@findepi findepi requested a review from kokosing January 15, 2019 19:40
@findepi
Copy link
Contributor Author

findepi commented Jan 15, 2019

cc @electrum

@electrum
Copy link
Contributor

#11561 adds full DDL support for JDBC connectors. With that in mind, maybe remove everything except the "note" part of the write path commit message (which is sufficient justification by itself to remove these).

@findepi
Remove unsupported types on write path in JDBC connectors
9a9dede 
These types are not supported in
`com.facebook.presto.plugin.jdbc.JdbcPageSink#appendColumn`, so cannot
be written. Attempt to support these types must be well test-covered on
per-database basis, so they should not be supported in a generic way
anyway.
@findepi
Remove unsupported types on read path in JDBC connectors
cd54d57 
No JDBC-based connector supports TIMESTAMP WITH TIME ZONE or TIME WITH
TIME ZONE, so no need to support predicate push down for these types.
@findepi
Copy link
Contributor Author

findepi commented Jan 16, 2019

With that in mind, maybe remove everything except the "note" part of the write path commit message (which is sufficient justification by itself to remove these).

@electrum i changed the commit message to:

Remove unsupported types on write path in JDBC connectors

These types are not supported in
`com.facebook.presto.plugin.jdbc.JdbcPageSink#appendColumn`, so cannot
be written. Attempt to support these types must be well test-covered on
per-database basis, so they should not be supported in a generic way
anyway.

@electrum @kokosing thanks for review!

@findepi findepi closed this Jan 24, 2019
@findepi findepi deleted the findepi/master/remove-unsupported-types-on-read-path-in-jdbc-connectors-1e7099 branch January 24, 2019 15:51
@findepi findepi mentioned this pull request Jan 24, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kokosing kokosing kokosing approved these changes

@electrum electrum electrum approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@findepi @electrum @kokosing @facebook-github-bot