presto icon indicating copy to clipboard operation
presto copied to clipboard

Removed unsupported data types on read/write paths in jdbc connectors

Open pratyakshsharma opened this issue 2 years ago • 6 comments

Cherrypick of https://github.com/prestodb/presto/pull/12231/commits Test plan - (Please fill in how you tested your changes)

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines. Don't forget to follow our attribution guidelines for any code copied from other projects.

Fill in the release notes towards the bottom of the PR description. See Release Notes Guidelines for details.

== RELEASE NOTES ==

General Changes
* QueryBuilder and BaseJdbcClient see the changes
* 

If release note is NOT required, use:

== NO RELEASE NOTE ==

pratyakshsharma avatar Jul 01 '22 10:07 pratyakshsharma

@NikhilCollooru Can you please take a pass.

pratyakshsharma avatar Jul 14 '22 08:07 pratyakshsharma

I am on parental leave. May be @highker can help review.

NikhilCollooru avatar Jul 14 '22 08:07 NikhilCollooru

There is no harm to keep it around right?

highker avatar Jul 15 '22 00:07 highker

We actually have dependency of this client to fall back from TIMESTAMP_WITH_TIME_ZONE to TIMESTAMP (or TIME_WITH_TIME_ZONE to TIME). Deleting these two types could cause regression.

highker avatar Jul 16 '22 06:07 highker

We actually have dependency of this client to fall back from TIMESTAMP_WITH_TIME_ZONE to TIMESTAMP (or TIME_WITH_TIME_ZONE to TIME). Deleting these two types could cause regression.

@highker Where do we have this dependency? Can you point me to the relevant files?

pratyakshsharma avatar Aug 17 '22 10:08 pratyakshsharma

@highker ping.

pratyakshsharma avatar Sep 14 '22 12:09 pratyakshsharma

Anyone who extends the JDBC connector may have a dependency on those types. Why delete them?

tdcmeehan avatar Oct 21 '22 20:10 tdcmeehan

Got you, closing this PR. Thank you for reviewing.

pratyakshsharma avatar Oct 27 '22 13:10 pratyakshsharma