> On Sept. 4, 2014, 9:39 p.m., Mithun Radhakrishnan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 1409 > > <https://reviews.apache.org/r/25178/diff/2/?file=674619#file674619line1409> > > > > Shouldn't you be passing the environment context here? ifPurge must > > apply to both table and partition data.
Indeed. Fixed in the next patch. > On Sept. 4, 2014, 9:39 p.m., Mithun Radhakrishnan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java, > > line 828 > > <https://reviews.apache.org/r/25178/diff/2/?file=674620#file674620line828> > > > > Fix whitespace for this line, and the if{} block. Fixed in the next patch. > On Sept. 4, 2014, 9:39 p.m., Mithun Radhakrishnan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java, > > lines 824-826 > > <https://reviews.apache.org/r/25178/diff/2/?file=674620#file674620line824> > > > > I wish this could be made more uniform. The ifPurge is being passed > > into the old `dropTable()` in an envContext, but `ignoreUnknown`, > > `deleteData`, etc. are not. I understand your predicament in changing the > > signature of an existing public interface method. I see your point, but I'm not so sure. The ignoreUnknownTab arg doesn't belong in envContext anyway, since it doesn't propagate that far down. If deleteData is removed from this API, it should also be removed from dropDatabase(), dropIndex() and dropPartition(), which do propagate down into the lower classes and maybe justifies a separate ticket. > On Sept. 4, 2014, 9:39 p.m., Mithun Radhakrishnan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java, line 1748 > > <https://reviews.apache.org/r/25178/diff/2/?file=674626#file674626line1748> > > > > We're going to need this too, for the solution to be complete. We'll > > need a new `dropPartitions()` overload that takes an ifPurge flag. This > > method should forward to the new method. I'm looking at it, but I think it may be a large enough change to justify a separate ticket. - david ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/#review52353 ----------------------------------------------------------- On Sept. 2, 2014, 11:41 p.m., david seraf wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25178/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2014, 11:41 p.m.) > > > Review request for hive and Xuefu Zhang. > > > Repository: hive-git > > > Description > ------- > > Add PURGE option to DROP TABLE command to skip saving table data to the trash > > > Diffs > ----- > > > hcatalog/core/src/test/java/org/apache/hive/hcatalog/mapreduce/TestHCatPartitionPublish.java > be7134f > > hcatalog/webhcat/svr/src/test/java/org/apache/hive/hcatalog/templeton/tool/TestTempletonUtils.java > af952f2 > > itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2.java > da51a55 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 9489949 > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > a94a7a3 > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreFsImpl.java > cff0718 > metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java > cbdba30 > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreFS.java > a141793 > metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 613b709 > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cd017d8 > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java e387b8f > > ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java > 4cf98d8 > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > f31a409 > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 32db0c7 > ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java ba30e1f > ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 406aae9 > ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveRemote.java 1a5ba87 > ql/src/test/queries/clientpositive/drop_table_purge.q PRE-CREATION > ql/src/test/results/clientpositive/drop_table_purge.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/25178/diff/ > > > Testing > ------- > > added code test and added QL test. Tests passed in CI, but other, unrelated > tests failed. > > > Thanks, > > david seraf > >
