-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Enable ctas with unmanaged hive table #15990
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.
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
base: master
Are you sure you want to change the base?
Enable ctas with unmanaged hive table #15990
Conversation
|
@rguillome This is cherry pick of trinodb/trino#3755 and trinodb/trino#2669. Please add co-author. Thanks |
presto-hive/src/main/java/com/facebook/presto/hive/HiveLocationService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Hi @v-jizhang This PR is still waiting approval and I just saw another PR with the same purpose (#16147). |
da9508b to
6aa52d9
Compare
Hi @rguillome, we happened to address this recently in our distro so I raised a PR not knowing there's already some WIP on this. I will close the other PR. |
|
Thanks @ashishtadose ! |
|
@rguillome Please squash the 6 commits in the PR (some of them are merge commits) |
6aa52d9 to
644df66
Compare
|
Hi @ajaygeorge |
dcde93f to
5c4b803
Compare
|
Hi @ajaygeorge, Please let me know what's why Facebook integration is failing (I thought I have seen succeed a day before...) and what's missing/wrong about this PR. |
|
@rguillome can you try triggering the build again by doing an git amend and push with no changes. |
5c4b803 to
a08d4ba
Compare
|
Hi @ajaygeorge , I made the push force 5 days ago with no more luck. Regards, |
|
@cocozianu can you please take a look at why the facebook integration build is not working. Looks like a new build is not getting fired for this PR. |
|
Hi @ajaygeorge and @cocozianu There is still a facebook integration error on this PR. Please let me know if I can change something to help going through this step. Regards, |
|
Hi @rguillome it's cleared now, sorry for the inconvenience. |
|
Hi @cocozianu : what's the next step for this PR, how can it be handled to be merged ? Regards, |
Please up ! |
|
Hi @ajaygeorge and @cocozianu Is there any chance this PR would be integrated soon ? |
|
@aweisberg Do you think we can merge for the next release if it looks good to you as well? |
|
Hi there, Happy new year to Presto's team 🎉 Is there a chance that PR would be merge ? Time is passing and I would like to avoid maintaining a fork with work to integrate incoming changes in my current production version of PrestoSQL Please let me know what do you plan for it Regards |
|
This doesn't bring in all of the tests from Trino so maybe we should go with #16147? |
As mentionned in the PR first comment, it was deliberate since a CTAS is a table creation from query results and not writing to an existing table but perhaps it's better to stick to trino's view. I will close this one and go to the #16147 |
1026084 to
8de0e9c
Compare
Hi @aweisberg, As the other PR #16147 introducing CTAS has not changed since your last comment, I managed to make this one compliant with the missings tests (there is one test more than the #16147 in the product-test module). Rebase to the current master has been made too. |
Co-authored-by: alexjo2144 <[email protected]>
8de0e9c to
b80eeec
Compare
|
Hey @yingsu00, could you please help with the final revive and get this merged? Thanks! |
|
Hi, any news from this one please ? |
|
@yingsu00 Friendly ping. Please review this PR. Thank you! |
|
Hi, |
Test plan - TU to create an table as a select with an external location with presto-hive plugin and a manual TI to test the creation under EMR, using a Glue catalog and seeing the results in S3 and Athena
This PR allow the creation of an unmanaged table (with external location specified) as a select result (CTAS) with Hive Plugin
Co-authored-by: alexjo2144 [email protected]
Fix:
#15946
Based on
trino PR #2669
trino PR #3755