> On 2011-06-23 16:49:59, Ashutosh Chauhan wrote: > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 591 > > <https://reviews.apache.org/r/949/diff/1/?file=21560#file21560line591> > > > > 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. > > Thiruvel Thirumoolan wrote: > I requested Devaraj offline to handle it in a separate JIRA. I am not > sure about other methods having the same issue. That said, I introduced the > same bug with alter_database. Will fix it for create and alter databases.
Actually, problem exists in create Database even now without your patch. So, you are not making it any worse. I am fine if you prefer to address it in a followup jira. About alter database, I am not sure if there is any real usecase for it. Having a database spread across multiple locations is not a regular semantics. First concern is clean rollback semantics. Another is what about drop database in such scenarios, which directories are deleted when you drop a database, current one or all or one you specify in drop database ddl? You potentially need to persist all the locations of database in objectstore for deletion or for other purposes, which means a list of locationUri instead of a single string. Given all these, you might want to defer alter database to a new jira. Apart from better understanding of the usecases and semantics for alter database, doing it in two different jira will make this patch smaller and thus easier to get committed. - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/949/#review898 ----------------------------------------------------------- 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 > >
