You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/01/30 07:59:39 UTC

[GitHub] [hive] baifachuan opened a new pull request #2987: HIVE-25912: Drop external table throw NPE if the the location to root path

baifachuan opened a new pull request #2987:
URL: https://github.com/apache/hive/pull/2987


   ### What changes were proposed in this pull request?
   
   modify the FileUtils.checkDeletePermission function add this check:
   
   ` 
   if(path.getParent() == null) {
     // no file/dir to be deleted, because of the path is root dir, hive table forbid set the location to root dir.
     return;
   }
   `
   If the path.getParent() we can be sure the location is ROOT path. no file/dir to be deleted
   
   
   ### Why are the changes needed?
   If I create an external table using the ROOT path, the table was created successfully, but when I drop the table throw the NPE.  So I can't drop the table forever.
   
   This is not a good phenomenon.
   
   ### Does this PR introduce _any_ user-facing change?
   no
   
   
   ### How was this patch tested?
   mvn test -Dtest=SomeTest --pl common
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] steveloughran commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1028164028


   Makes sense, but be aware that at lot of code doesn't like using / as a destination of work, as it is a special directory.
   
   * paths are wrong
   * rm / doesn't delete it
   
   even after this is fixed, there's a risk that jobs to / will fail for other reasons. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1032292606


   @steveloughran can you invite me to join the hive slack discuss group? or 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033651983


   > @baifachuan Have you checked if the problem affects also the master branch?
   
   I'm sure the master also has this problem.
   
   I test the master branch, have the same problem.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan removed a comment on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan removed a comment on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1032292606


   @steveloughran can you invite me to join the hive slack discuss group? or 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] pvary commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1032711493


   I would prefer to have a test for this, otherwise LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033417977


   > Maybe an overkill, but shall we create a test which makes sure that the method is not removed from the create table path?
   > 
   > Maybe something like this: https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java#L341
   
   Thanks for your reply. I think this feature only adds a verification for the storage location, its compatibility with other tests.
   
   I make sure to create fail when the location is ROOT path and make sure other tests pass.
   
   So I add two tests to verify the ROOT path and not the ROOT path. 
   
   TestTablesCreateDropAlterTruncate.java tests maybe like an E2E test.
   
   I will consider adding a test for this case.
   
   thx.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033594076


   @baifachuan Have you checked if the problem affects also the master branch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033314982


   > assertFalse
   
   Thanks for your suggestion, I have created a test for this feature.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan removed a comment on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan removed a comment on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033651983


   > @baifachuan Have you checked if the problem affects also the master branch?
   
   I'm sure the master also has this problem.
   
   I test the master branch, have the same problem.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan removed a comment on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan removed a comment on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033314982


   > assertFalse
   
   Thanks for your suggestion, I have created a test for this feature.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033315090


   > 
   
   Thanks for your suggestion, I have created a test for this feature.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1031004175


   > 
   
   Yes,  I agree with it, but I think we should check the path throw a fail exception if the path is illegal,  create the table successfully but can not drop the table, this is not a good experience.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan edited a comment on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan edited a comment on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1031046556


   I fixed the bug with check the hive storage path, if it is a ROOT path throw an exception, broken the create table behavior.
   
   ```
   DEBUG : Shutting down query CREATE EXTERNAL TABLE `fcbai1`(
   `inv_item_sk` int,
   `inv_warehouse_sk` int,
   `inv_quantity_on_hand` int)
   PARTITIONED BY (
   `inv_date_sk` int) STORED AS ORC
   LOCATION
   'hdfs://127.0.0.1:8020/'
   Error: Error while processing statement: FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask. InvalidObjectException(message:fcbai1 location must not be root path) (state=08S01,code=1)
   ```
   
   Ensure that creating tables and dropping tables behave the same.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan closed pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan closed pull request #2987:
URL: https://github.com/apache/hive/pull/2987


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033663696


   > @baifachuan Have you checked if the problem affects also the master branch?
   
   I'm sure the master also has this problem.
   
   I test the master branch, have the same problem.
   
   I update the error info to jira.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] pvary commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033403645


   Maybe an overkill, but shall we create a test which makes sure that the method is not removed from the create table path?
   
   Maybe something like this:
   https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java#L341


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1033677319


   I create a PR for the master branch, the link is: https://github.com/apache/hive/pull/3009


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] baifachuan commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
baifachuan commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1031046556


   I fixed the bug with check the hive storage path, if it is a ROOT path throw an exception, broken the create table.
   
   ```
   DEBUG : Shutting down query CREATE EXTERNAL TABLE `fcbai1`(
   `inv_item_sk` int,
   `inv_warehouse_sk` int,
   `inv_quantity_on_hand` int)
   PARTITIONED BY (
   `inv_date_sk` int) STORED AS ORC
   LOCATION
   'hdfs://127.0.0.1:8020/'
   Error: Error while processing statement: FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask. InvalidObjectException(message:fcbai1 location must not be root path) (state=08S01,code=1)
   ```
   
   Ensure that creating tables and dropping tables behave the same.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] steveloughran commented on pull request #2987: HIVE-25912: Drop external table throw NPE if the location set to ROOT path

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2987:
URL: https://github.com/apache/hive/pull/2987#issuecomment-1032525209


   LGTM; and as its hard to test not worried about that. does need review from hive developers though


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org