Re: Review Request 25178: Add DROP TABLE PURGE
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/ --- (Updated Sept. 2, 2014, 9:11 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 - 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
Re: Review Request 25178: Add DROP TABLE PURGE
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/ --- (Updated Sept. 2, 2014, 9:12 p.m.) Review request for hive and Xuefu Zhang. Changes --- put back the diff that somehow got lost Repository: hive-git Description --- Add PURGE option to DROP TABLE command to skip saving table data to the trash Diffs (updated) - 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 84ef5f9 metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 237166e metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreFsImpl.java cff0718 metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 143d1c7 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 6f225f3 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
Re: Review Request 25178: Add DROP TABLE PURGE
--- 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. Changes --- responded to review comments and uploaded new diff Repository: hive-git Description --- Add PURGE option to DROP TABLE command to skip saving table data to the trash Diffs (updated) - 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
Re: Review Request 25178: Add DROP TABLE PURGE
> On Sept. 4, 2014, 4:57 a.m., Lefty Leverenz wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java, > > line 822 > > <https://reviews.apache.org/r/25178/diff/1-2/?file=674238#file674238line822> > > > > You could add an explanation for UnsupportedOperationException but it > > seems self-explanatory (so I'm not opening an issue). Ok. I'll cleanup the various dropTable() docs with the next set of changes (when I get a code review). Thanks. - david --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/#review52279 ------- 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 > >
Re: Review Request 25178: Add DROP TABLE PURGE
> 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
>
>
Re: Review Request 25178: Add DROP TABLE PURGE
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/ --- (Updated Sept. 9, 2014, 6:51 p.m.) Review request for hive and Xuefu Zhang. Changes --- latest patch from HIVE-7100 includes documentation updates and responses to RB comments Repository: hive-git Description --- Add PURGE option to DROP TABLE command to skip saving table data to the trash Diffs (updated) - 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
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/ --- (Updated Sept. 12, 2014, 8:28 p.m.) Review request for hive and Xuefu Zhang. Changes --- Added JIRA ticket number to request Summary Summary (updated) - HIVE-7100 Add DROP TABLE PURGE 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
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/ --- (Updated Sept. 12, 2014, 8:28 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
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
> On Sept. 10, 2014, 6:40 p.m., Xuefu Zhang wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 1409 > > <https://reviews.apache.org/r/25178/diff/3/?file=684101#file684101line1409> > > > > Nit: should we just pass ifPurge as boolean to the method unless > > envContext is also used for something else. This seemingly makes the called > > method cleaner. I agree in principle, but if this is the way we want to do this, it will comlicate things a bit and require adding some more functions for the different signature both up and down the call tree, and will interact with the dropPartitions implementation. So I think the better approach is to leave it for the dropPartitions ticket. - david --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/#review52911 ------- On Sept. 12, 2014, 8:28 p.m., david seraf wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25178/ > --- > > (Updated Sept. 12, 2014, 8:28 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 > >
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
> 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. > > david seraf wrote: > I'm looking at it, but I think it may be a large enough change to justify > a separate ticket. It is agreed that this will be the subject of a separate ticket. - david --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/#review52353 --- On Sept. 12, 2014, 8:28 p.m., david seraf wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25178/ > --- > > (Updated Sept. 12, 2014, 8:28 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 > >
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/ --- (Updated Sept. 12, 2014, 9:51 p.m.) Review request for hive and Xuefu Zhang. Changes --- added ticket number to Bugs field Bugs: HIVE-7100 https://issues.apache.org/jira/browse/HIVE-7100 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
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
> On Sept. 13, 2014, 3:54 a.m., Xuefu Zhang wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 1409 > > <https://reviews.apache.org/r/25178/diff/3/?file=684101#file684101line1409> > > > > Maybe I wasn't clear, I was only talking about the two methods added in > > this patch: deletePartitionData(Path, EnvironmentContext) and > > deleteTableData(Path, EnvironmentContext). Instead of take 2nd param as > > this context, take a boolean. Callers will only need to pass a boolean. > > Also, both methods are new and private. I'm not sure if I undertand the > > complexity of the change. I misunderstood. I'm fine with changing those 2 methods. - david --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/#review53259 --- On Sept. 12, 2014, 9:51 p.m., david seraf wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25178/ > --- > > (Updated Sept. 12, 2014, 9:51 p.m.) > > > Review request for hive and Xuefu Zhang. > > > Bugs: HIVE-7100 > https://issues.apache.org/jira/browse/HIVE-7100 > > > 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 > >
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
> On Sept. 13, 2014, 3:54 a.m., Xuefu Zhang wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 1409 > > <https://reviews.apache.org/r/25178/diff/3/?file=684101#file684101line1409> > > > > Maybe I wasn't clear, I was only talking about the two methods added in > > this patch: deletePartitionData(Path, EnvironmentContext) and > > deleteTableData(Path, EnvironmentContext). Instead of take 2nd param as > > this context, take a boolean. Callers will only need to pass a boolean. > > Also, both methods are new and private. I'm not sure if I undertand the > > complexity of the change. > > david seraf wrote: > I misunderstood. I'm fine with changing those 2 methods. I looked at this in more detail and the problem is that dropTableData() is called from drop_table_with_environment_context(), which _is_ public and uses EnvironmentContext, so this change would have to propagate out to the interface class, which I'd rather not do just to make the code a little tidier. - david --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/#review53259 --- On Sept. 12, 2014, 9:51 p.m., david seraf wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25178/ > --- > > (Updated Sept. 12, 2014, 9:51 p.m.) > > > Review request for hive and Xuefu Zhang. > > > Bugs: HIVE-7100 > https://issues.apache.org/jira/browse/HIVE-7100 > > > 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 > >
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
> On Sept. 16, 2014, 10:21 p.m., Xuefu Zhang wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 1409 > > <https://reviews.apache.org/r/25178/diff/3/?file=684101#file684101line1409> > > > > Maybe I'm misunderstanding. the added dropTableData(Path, boolean) is > > called by some public API, such as drop_table_with_environment_context(), > > but we don't need to change the API. Instead, before calling > > dropTableData(), we just need to get a boolean value (ifPurge) from the > > context. The signature of the API doesn't need to change. I see it now. Sorry. Fixed in the next patch. - david --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/#review53612 ------- On Sept. 12, 2014, 9:51 p.m., david seraf wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25178/ > --- > > (Updated Sept. 12, 2014, 9:51 p.m.) > > > Review request for hive and Xuefu Zhang. > > > Bugs: HIVE-7100 > https://issues.apache.org/jira/browse/HIVE-7100 > > > 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 > >
Re: Review Request 25178: HIVE-7100 Add DROP TABLE PURGE
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25178/ --- (Updated Sept. 16, 2014, 11:52 p.m.) Review request for hive and Xuefu Zhang. Changes --- patch 10 fixes the last remaining review comments Bugs: HIVE-7100 https://issues.apache.org/jira/browse/HIVE-7100 Repository: hive-git Description --- Add PURGE option to DROP TABLE command to skip saving table data to the trash Diffs (updated) - hcatalog/core/src/test/java/org/apache/hive/hcatalog/mapreduce/TestHCatPartitionPublish.java c2a0f5f hcatalog/webhcat/svr/src/test/java/org/apache/hive/hcatalog/templeton/tool/TestTempletonUtils.java af952f2 metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 33745e4 metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 8765d53 metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreFsImpl.java cff0718 metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java e86a90a metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreFS.java a141793 metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java a32507d ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 56bcf1c ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java f40f5f7 ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java 6d18884 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 05cde3e ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 25cd3a5 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
