presto
presto copied to clipboard
Fix error while accessing tables when using Glue
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 ==
@imjalpreet can you follow up on this? I heard you have a review comment that you need to add here. Thanks!
@imjalpreet can you check this and give your explicit approval if everything looks good? Thanks!
@imjalpreet gentle ping!
@imjalpreet @NikhilCollooru this should be good for another pass.
@imjalpreet ping!
@imjalpreet Please take a pass when you have cycles.
@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.
@prestodb/committers Please take a pass, this should be good to merge.
@prestodb/committers ping!
Hi @NikhilCollooru @rschlussel, can we take a look at this PR? Thanks!
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 ?
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
@NikhilCollooru gentle ping!
cc @prestodb/committers
@NikhilCollooru Pinging to see if you can review the latest changes anytime soon?
@NikhilCollooru Please take a pass. I have fixed the test cases and addressed all comments.
@NikhilCollooru gentle ping!
https://github.com/prestodb/presto/pull/20228
@NikhilCollooru please take a pass.
@NikhilCollooru gentle ping!
@NikhilCollooru This should be good for final pass. Please take a look.
@NikhilCollooru ping.
@NikhilCollooru @tdcmeehan Can we close this soon?
@NikhilCollooru ping
@NikhilCollooru I am going to take over this review. Please, if you have any feedback, let us know.