> On Feb. 4, 2020, 3:49 p.m., Peter Vary wrote: > > Thanks for the patch! This will be very-very usefull. > > Some minor comments, questions...
Thanks a lot for the review!! > On Feb. 4, 2020, 3:49 p.m., Peter Vary wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorTestUtil.java > > Lines 55 (patched) > > <https://reviews.apache.org/r/71904/diff/3/?file=2210213#file2210213line55> > > > > Is this import used? You're right, it is not used. Removed it. > On Feb. 4, 2020, 3:49 p.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java > > Lines 843 (patched) > > <https://reviews.apache.org/r/71904/diff/3/?file=2210216#file2210216line845> > > > > Is inheritPerms still a working stuff? I kinda remember that it was > > removed from Hive some time ago... No, I think this log message was just a copy-paste error. Fixed it. > On Feb. 4, 2020, 3:49 p.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java > > Lines 1799 (patched) > > <https://reviews.apache.org/r/71904/diff/3/?file=2210218#file2210218line1799> > > > > Maybe slightly different log message, so we can easily ditinguish > > between this and the line below Fixed it. > On Feb. 4, 2020, 3:49 p.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > > Lines 7379 (patched) > > <https://reviews.apache.org/r/71904/diff/3/?file=2210231#file2210231line7379> > > > > We might want to make this feature configurable, to turn it on/off in > > case we missed some edge cases You are absolutely right. I introduced a config parameter so we can turn on/off this feature. > On Feb. 4, 2020, 3:49 p.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > > Lines 493-494 (patched) > > <https://reviews.apache.org/r/71904/diff/3/?file=2210235#file2210235line493> > > > > nit: Formatting? Really not important, just for the completensess shake > > :D Fixed it. > On Feb. 4, 2020, 3:49 p.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > > Lines 690-691 (patched) > > <https://reviews.apache.org/r/71904/diff/3/?file=2210235#file2210235line690> > > > > nit: Formatting? Fixed it. > On Feb. 4, 2020, 3:49 p.m., Peter Vary wrote: > > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java > > Lines 1246 (patched) > > <https://reviews.apache.org/r/71904/diff/3/?file=2210248#file2210248line1246> > > > > Is this table always exists? Shall we use "drop table if exists" > > instead? Fixed it. - Marta ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71904/#review219487 ----------------------------------------------------------- On Jan. 31, 2020, 4:12 p.m., Marta Kuczora wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71904/ > ----------------------------------------------------------- > > (Updated Jan. 31, 2020, 4:12 p.m.) > > > Review request for hive, Gopal V and Peter Vary. > > > Bugs: HIVE-21164 > https://issues.apache.org/jira/browse/HIVE-21164 > > > Repository: hive-git > > > Description > ------- > > Extended the original patch with saving the task attempt ids in the file > names and also fixed some bugs in the original patch. > With this fix, inserting into an ACID table would not use move task to place > the generated files into the final directory. It will inserts every files to > the final directory and then clean up the files which are not needed (like > written by failed task attempts). > Also fixed the replication tests which failed for the original patch as well. > > > Diffs > ----- > > > hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java > da677c7977 > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestAcidOnTez.java > 056cd27496 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/history/TestHiveHistory.java > 31d15fdef9 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorTestUtil.java > c2aa73b5f1 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java > 4c01311117 > ql/src/java/org/apache/hadoop/hive/ql/exec/AbstractFileMergeOperator.java > 9a3258115b > ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 9ad4e71482 > ql/src/java/org/apache/hadoop/hive/ql/exec/MoveTask.java 06e4ebee82 > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 6c67bc7dd8 > ql/src/java/org/apache/hadoop/hive/ql/io/AcidInputFormat.java bba3960102 > ql/src/java/org/apache/hadoop/hive/ql/io/AcidOutputFormat.java 1e8bb223f2 > ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 2f5ec5270c > ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java > 8980a6292a > ql/src/java/org/apache/hadoop/hive/ql/io/RecordUpdater.java 737e6774b7 > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 76984abd0a > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcOutputFormat.java > c4c56f8477 > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java > b8a0f0465c > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRecordUpdater.java > 398698ec06 > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java > 2543dc6fc4 > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 7f061d4a6b > ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java > 73ca658d9c > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 5fcc367cc9 > ql/src/java/org/apache/hadoop/hive/ql/parse/spark/GenSparkUtils.java > c102a69f8f > ql/src/java/org/apache/hadoop/hive/ql/plan/FileSinkDesc.java ecc7bdee4d > ql/src/java/org/apache/hadoop/hive/ql/plan/LoadTableDesc.java bed05819b5 > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > bb70db4524 > ql/src/java/org/apache/hadoop/hive/ql/util/UpgradeTool.java 58e6289583 > ql/src/test/org/apache/hadoop/hive/ql/TestTxnAddPartition.java c9cb6692df > ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java 842140815d > ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 88ca683173 > ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 908ceb43fc > ql/src/test/org/apache/hadoop/hive/ql/TestTxnConcatenate.java 8676e0db11 > ql/src/test/org/apache/hadoop/hive/ql/TestTxnExIm.java 66b2b2768b > ql/src/test/org/apache/hadoop/hive/ql/TestTxnLoadData.java bb55d9fd79 > ql/src/test/org/apache/hadoop/hive/ql/TestTxnNoBuckets.java ea6b1d9bec > ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java > af14e628b3 > ql/src/test/org/apache/hadoop/hive/ql/exec/TestExecDriver.java 83db48e758 > ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java > 2c4b69b2fe > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java > 48e9afc496 > ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/CompactorTest.java > cfd7290762 > ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestWorker.java > 70ae85c458 > ql/src/test/results/clientpositive/acid_subquery.q.out 1dc1775557 > ql/src/test/results/clientpositive/create_transactional_full_acid.q.out > e324d5ec43 > > ql/src/test/results/clientpositive/encrypted/encryption_insert_partition_dynamic.q.out > 61b0057adb > ql/src/test/results/clientpositive/llap/acid_no_buckets.q.out fbf4e481f1 > ql/src/test/results/clientpositive/llap/insert_overwrite.q.out fbc3326b39 > ql/src/test/results/clientpositive/llap/mm_all.q.out 226f2a9374 > ql/src/test/results/clientpositive/mm_all.q.out 143ebd69f9 > streaming/src/test/org/apache/hive/streaming/TestStreaming.java 35a220facd > > > Diff: https://reviews.apache.org/r/71904/diff/3/ > > > Testing > ------- > > Had to modify some tests because of the file name changes. Also added some > specific tests. > In the pre-commit run all tests passed successfully. > > > Thanks, > > Marta Kuczora > >
