> On 2011-06-24 06:32:41, Amareshwari Sriramadasu wrote: > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java, > > line 159 > > <https://reviews.apache.org/r/949/diff/1/?file=21562#file21562line159> > > > > I don't think we can just remove these public apis. Shall we deprecate > > them now and remove in next version?
Dependence on dbname and not Database: If dbname is an arg, a Database object is required to get location and that will add a dependency to a RawStore instance (either creating it internally or as argument). The current separation of functionality seem to be breached and I did not want to do that. Let me know what you think. Making methods deprecated: 0. I assumed this API is not used outside Hive and addressed internal usages. HCatalog will be inheriting this patch and that should work for them as well. 1. Retaining the API without change would return wrong results for databases created with a location. 2. Changing the API with change would add dependency to RawStore, which again I don't want. Considering all these, I removed it. Let me know if I am wrong. > On 2011-06-24 06:32:41, Amareshwari Sriramadasu wrote: > > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g, line 613 > > <https://reviews.apache.org/r/949/diff/1/?file=21567#file21567line613> > > > > Similar to TOK_ALTERTABLE_*, we can still have > > TOK_ALTERDATABASE_DBPROPERTIES for properties and > > TOK_ALTERDATABASE_LOCATION for location. What do you think? I thought about it, I intended to keep it compact. The amount of work being done for each ALTERTABLE_* is high but its pretty small for database (at-least for now). But it does not hurt to separate out. - Thiruvel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/949/#review905 ----------------------------------------------------------- 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 > >
