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
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
--- 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: 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: 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
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
--- 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
