----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/949/#review898 -----------------------------------------------------------
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java <https://reviews.apache.org/r/949/#comment1938> This may not be always successful. You may fail to create dirs for number of reasons. So, this needs to be handled gracefully. Transaction needs to rollback in such case and create database ddl needs to fail. For more info, look the first comment of Devaraj and also his attached partial patch. trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java <https://reviews.apache.org/r/949/#comment1941> As previously, mkdirs() can fail, so handle similarly as in createDatabase() trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java <https://reviews.apache.org/r/949/#comment1942> Please also add a test when a create database fails because a FS operation fails. In such a case no metadata should get created. One way to simulate that is to make location unwritable then try to create database on that location. - Ashutosh On 2011-06-23 09:55:50, Thiruvel Thirumoolan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/949/ > ----------------------------------------------------------- > > (Updated 2011-06-23 09:55:50) > > > Review request for hive, Ning Zhang and Amareshwari Sriramadasu. > > > Summary > ------- > > Usage: > > create database location 'path1'; > alter database location 'path2'; > > After 'alter', only newly created tables will be located under the new > location. Tables created before 'alter' will be under 'path1'. > > Notes: > ------ > 1. I have moved getDefaultDatabasePath() to HiveMetaStore and made it > private. There should only be one API to obtain the location of a database > and it has to accept 'Database' as an arg and hence the new method in > Warehouse 'getDatabasePath()' and similarly 'getTablePath()'. The usages of > older API also has been changed. Hope that should be fine. > 2. One could argue why have getDatabasePath() as location can be obtained by > db.getLocationUri(). I wanted to retain this method to do any additional > processing if necessary (getDns or whatever). > > > This addresses bug HIVE-1537. > https://issues.apache.org/jira/browse/HIVE-1537 > > > Diffs > ----- > > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java > 1138011 > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 1138011 > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 1138011 > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java > 1138011 > > trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java > 1138011 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1138011 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java 1138011 > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > 1138011 > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1138011 > > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/ImportSemanticAnalyzer.java > 1138011 > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 1138011 > > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java > 1138011 > trunk/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 1138011 > trunk/ql/src/test/queries/clientpositive/database_location.q PRE-CREATION > trunk/ql/src/test/results/clientpositive/database_location.q.out > PRE-CREATION > > Diff: https://reviews.apache.org/r/949/diff > > > Testing > ------- > > 1. Updated TestHiveMetaStore.java for testing the functionality - database > creation, alteration and table's locations as TestCliDriver outputs ignore > locations. > 2. Added database_location.q for testing the grammar primarily. > > Thanks, > Thiruvel > > > Thanks, > > Thiruvel > >
