presto icon indicating copy to clipboard operation
presto copied to clipboard

Fix error while accessing tables when using Glue

Open pratyakshsharma opened this issue 2 years ago • 24 comments

OpenCSVSerde deserializes columns from csv file into strings only. Currently the CSV support in Presto relies on metastore to return all data types as string. Glue metastore returns the original data type which results in error when trying to cast to string.

Fixes https://github.com/prestodb/presto/issues/15869.

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.

== NO RELEASE NOTE ==

pratyakshsharma avatar Sep 11 '22 14:09 pratyakshsharma

@imjalpreet can you follow up on this? I heard you have a review comment that you need to add here. Thanks!

ethanyzhang avatar Oct 26 '22 15:10 ethanyzhang

@imjalpreet can you check this and give your explicit approval if everything looks good? Thanks!

ethanyzhang avatar Nov 16 '22 18:11 ethanyzhang

@imjalpreet gentle ping!

pratyakshsharma avatar Dec 06 '22 22:12 pratyakshsharma

@imjalpreet @NikhilCollooru this should be good for another pass.

pratyakshsharma avatar Dec 21 '22 10:12 pratyakshsharma

@imjalpreet ping!

pratyakshsharma avatar Jan 03 '23 08:01 pratyakshsharma

@imjalpreet Please take a pass when you have cycles.

pratyakshsharma avatar Feb 01 '23 10:02 pratyakshsharma

@imjalpreet Thank you for your valuable inputs which helped me in getting this PR to a better shape and also understand the presto codebase a little better. I have addressed all your review comments now, please have a look.

pratyakshsharma avatar Mar 03 '23 12:03 pratyakshsharma

@prestodb/committers Please take a pass, this should be good to merge.

pratyakshsharma avatar Mar 08 '23 11:03 pratyakshsharma

@prestodb/committers ping!

pratyakshsharma avatar Mar 15 '23 21:03 pratyakshsharma

Hi @NikhilCollooru @rschlussel, can we take a look at this PR? Thanks!

ethanyzhang avatar Mar 21 '23 04:03 ethanyzhang

reposting my earlier question from the comment.

Looks like there will be an additional call to getTable inside loadPartitionsByName. But the table cache is not enabled by default. So it will be an increase in latency because of extra call. Is my understanding correct ?

NikhilCollooru avatar Mar 24 '23 03:03 NikhilCollooru

reposting my earlier question from the comment. Looks like there will be an additional call to getTable inside loadPartitionsByName. But the table cache is not enabled by default. So it will be an increase in latency because of extra call. Is my understanding correct ?

Replied on your comment itself. @NikhilCollooru

pratyakshsharma avatar Apr 04 '23 10:04 pratyakshsharma

@NikhilCollooru gentle ping!

cc @prestodb/committers

pratyakshsharma avatar Apr 11 '23 10:04 pratyakshsharma

@NikhilCollooru Pinging to see if you can review the latest changes anytime soon?

pratyakshsharma avatar May 29 '23 07:05 pratyakshsharma

@NikhilCollooru Please take a pass. I have fixed the test cases and addressed all comments.

pratyakshsharma avatar Jun 01 '23 10:06 pratyakshsharma

@NikhilCollooru gentle ping!

pratyakshsharma avatar Jun 05 '23 08:06 pratyakshsharma

https://github.com/prestodb/presto/pull/20228

pratyakshsharma avatar Jul 06 '23 11:07 pratyakshsharma

@NikhilCollooru please take a pass.

pratyakshsharma avatar Jul 06 '23 13:07 pratyakshsharma

@NikhilCollooru gentle ping!

pratyakshsharma avatar Jul 11 '23 15:07 pratyakshsharma

@NikhilCollooru This should be good for final pass. Please take a look.

pratyakshsharma avatar Jul 24 '23 18:07 pratyakshsharma

@NikhilCollooru ping.

pratyakshsharma avatar Aug 02 '23 17:08 pratyakshsharma

@NikhilCollooru @tdcmeehan Can we close this soon?

pratyakshsharma avatar Sep 06 '23 20:09 pratyakshsharma

@NikhilCollooru ping

pratyakshsharma avatar Sep 19 '23 09:09 pratyakshsharma

@NikhilCollooru I am going to take over this review. Please, if you have any feedback, let us know.

tdcmeehan avatar Oct 05 '23 19:10 tdcmeehan