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 2021/07/15 05:44:47 UTC

[GitHub] [hive] ujc714 opened a new pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

ujc714 opened a new pull request #2478:
URL: https://github.com/apache/hive/pull/2478


   ### What changes were proposed in this pull request?
   Use a default directory for MANAGEDLOCATION if it's not assigned in CREATE DATABASE query.
   
   ### Why are the changes needed?
   HMS doesn't create MANAGEDLOCATION directory if it's NULL. If we run a CTAS query immediately after the CREATE DATABASE query and the staging directory is not under the MANAGEDLOCATION directory, the CTAS query will fail in MOVE task.
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   
   ### How was this patch tested?
   mvn test -Dtest=TestMiniTezCliDriver -Dqfile=create_database.q


-- 
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] abstractdog commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r674573000



##########
File path: itests/src/test/resources/testconfiguration.properties
##########
@@ -7,6 +7,7 @@ minimr.query.files=\
 # Queries ran by both MiniLlapLocal and MiniTez
 minitez.query.files.shared=\
   compressed_skip_header_footer_aggr.q,\
+  create_database.q,\

Review comment:
       let's use TestMiniLlapLocalCliDriver if the patch has nothing specific to tez container mode




-- 
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] ujc714 commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
ujc714 commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r714608422



##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExclusiveReplica.java
##########
@@ -710,7 +710,6 @@ private void verifyCustomDBLocations(String srcDb, List<String> listOfTables, St
               replicatedDbName.toLowerCase()  + ".db").toUri().getPath());
     } else {
       Assert.assertNotEquals(managedCustLocOnSrc,  null);
-      Assert.assertEquals(replDatabase.getManagedLocationUri(),  null);

Review comment:
       @pkumarsinha Made the change as you suggested :) 




-- 
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] ujc714 commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
ujc714 commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r682339546



##########
File path: ql/src/test/results/clientpositive/llap/alter_change_db_location.q.out
##########
@@ -11,7 +11,7 @@ PREHOOK: Input: database:newdb
 POSTHOOK: query: describe database extended newDB
 POSTHOOK: type: DESCDATABASE
 POSTHOOK: Input: database:newdb
-newdb		location/in/test		hive_test_user	USER			
+#### A masked pattern was here ####

Review comment:
       The managedlocation is like "file:/home/robbie/hive/itests/qtest/target/localfs/warehouse/newdb.db" which matches "file:" in QOutProcessor.maskIfContains. 




-- 
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] ujc714 commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
ujc714 commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r674586948



##########
File path: itests/src/test/resources/testconfiguration.properties
##########
@@ -7,6 +7,7 @@ minimr.query.files=\
 # Queries ran by both MiniLlapLocal and MiniTez
 minitez.query.files.shared=\
   compressed_skip_header_footer_aggr.q,\
+  create_database.q,\

Review comment:
       Got it. Good to know :)




-- 
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] ujc714 commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
ujc714 commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r714450436



##########
File path: iceberg/iceberg-handler/src/test/results/positive/truncate_force_iceberg_table.q.out
##########
@@ -85,7 +85,7 @@ Retention:          	0
 #### A masked pattern was here ####
 Table Type:         	EXTERNAL_TABLE      	 
 Table Parameters:	 	 
-	COLUMN_STATS_ACCURATE	{\"BASIC_STATS\":\"true\"}
+	COLUMN_STATS_ACCURATE	{\"BASIC_STATS\":\"true\",\"COLUMN_STATS\":{\"id\":\"true\",\"value\":\"true\"}}

Review comment:
       You are right. The two iceberg qfiles are updated in the master branch. I rebased and recommitted the changes again.




-- 
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] pkumarsinha commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r714488480



##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExclusiveReplica.java
##########
@@ -710,7 +710,6 @@ private void verifyCustomDBLocations(String srcDb, List<String> listOfTables, St
               replicatedDbName.toLowerCase()  + ".db").toUri().getPath());
     } else {
       Assert.assertNotEquals(managedCustLocOnSrc,  null);
-      Assert.assertEquals(replDatabase.getManagedLocationUri(),  null);

Review comment:
       @ujc714  Rather than removing, I think we should assert that replDatabase.getManagedLocationUri() is in default warehouse location, i.e it should be equal to new Path(replica.warehouseRoot, replicatedDbName.toLowerCase()  + ".db")




-- 
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] ujc714 commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
ujc714 commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r694851572



##########
File path: ql/src/test/results/clientpositive/llap/alter_change_db_location.q.out
##########
@@ -11,7 +11,7 @@ PREHOOK: Input: database:newdb
 POSTHOOK: query: describe database extended newDB
 POSTHOOK: type: DESCDATABASE
 POSTHOOK: Input: database:newdb
-newdb		location/in/test		hive_test_user	USER			
+#### A masked pattern was here ####

Review comment:
       Actually the original output is like:
   newdb		location/in/test	file:/home/robbie/hive/itests/qtest/target/localfs/warehouse/newdb.db	hive_test_user	USER
   
   The managedlocation is not empty. Because of the pattern "file:/", QOutProcessor masks the whole line.	




-- 
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] abstractdog commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r713687287



##########
File path: iceberg/iceberg-handler/src/test/results/positive/truncate_force_iceberg_table.q.out
##########
@@ -85,7 +85,7 @@ Retention:          	0
 #### A masked pattern was here ####
 Table Type:         	EXTERNAL_TABLE      	 
 Table Parameters:	 	 
-	COLUMN_STATS_ACCURATE	{\"BASIC_STATS\":\"true\"}
+	COLUMN_STATS_ACCURATE	{\"BASIC_STATS\":\"true\",\"COLUMN_STATS\":{\"id\":\"true\",\"value\":\"true\"}}

Review comment:
       is this because of the patch? looks unrelated




-- 
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] abstractdog merged pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
abstractdog merged pull request #2478:
URL: https://github.com/apache/hive/pull/2478


   


-- 
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] ujc714 commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
ujc714 commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r675321115



##########
File path: itests/src/test/resources/testconfiguration.properties
##########
@@ -7,6 +7,7 @@ minimr.query.files=\
 # Queries ran by both MiniLlapLocal and MiniTez
 minitez.query.files.shared=\
   compressed_skip_header_footer_aggr.q,\
+  create_database.q,\

Review comment:
       Removed the Tez tests and squashed the commits.




-- 
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] abstractdog commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r693817683



##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExclusiveReplica.java
##########
@@ -710,7 +710,6 @@ private void verifyCustomDBLocations(String srcDb, List<String> listOfTables, St
               replicatedDbName.toLowerCase()  + ".db").toUri().getPath());
     } else {
       Assert.assertNotEquals(managedCustLocOnSrc,  null);
-      Assert.assertEquals(replDatabase.getManagedLocationUri(),  null);

Review comment:
       @pkumarsinha: could you please confirm if removing this assertion won't hide any problems with replication?
   this patch takes care of setting managed location uri if it's not set by create database




-- 
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] ujc714 commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
ujc714 commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r714608422



##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExclusiveReplica.java
##########
@@ -710,7 +710,6 @@ private void verifyCustomDBLocations(String srcDb, List<String> listOfTables, St
               replicatedDbName.toLowerCase()  + ".db").toUri().getPath());
     } else {
       Assert.assertNotEquals(managedCustLocOnSrc,  null);
-      Assert.assertEquals(replDatabase.getManagedLocationUri(),  null);

Review comment:
       Made the change as you suggested :) 




-- 
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] abstractdog commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r693810698



##########
File path: ql/src/test/results/clientpositive/llap/alter_change_db_location.q.out
##########
@@ -11,7 +11,7 @@ PREHOOK: Input: database:newdb
 POSTHOOK: query: describe database extended newDB
 POSTHOOK: type: DESCDATABASE
 POSTHOOK: Input: database:newdb
-newdb		location/in/test		hive_test_user	USER			
+#### A masked pattern was here ####

Review comment:
       I see, I created a followup ticket for this: HIVE-25473 (in order to get useful info back)




-- 
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] abstractdog commented on pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

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


   change looks good to me, all tests passed +1


-- 
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] abstractdog commented on a change in pull request #2478: HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire…

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #2478:
URL: https://github.com/apache/hive/pull/2478#discussion_r681759845



##########
File path: ql/src/test/results/clientpositive/llap/alter_change_db_location.q.out
##########
@@ -11,7 +11,7 @@ PREHOOK: Input: database:newdb
 POSTHOOK: query: describe database extended newDB
 POSTHOOK: type: DESCDATABASE
 POSTHOOK: Input: database:newdb
-newdb		location/in/test		hive_test_user	USER			
+#### A masked pattern was here ####

Review comment:
       we need to double-check in what way the output changed which triggered a q.out masking




-- 
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