----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72532/#review220956 -----------------------------------------------------------
LGTM, a few minor suggestions. (Non-binding) ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java Line 1411 (original) <https://reviews.apache.org/r/72532/#comment309692> Are these originals not needed, or collected elsewhere? ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java Lines 1438 (patched) <https://reviews.apache.org/r/72532/#comment309688> might want to add a check + error message before the while loop in case someone added a file that has no delta or base dir in its path. you never know what people are capable of :) ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java Lines 1441 (patched) <https://reviews.apache.org/r/72532/#comment309685> nit: typo ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java Lines 1442 (patched) <https://reviews.apache.org/r/72532/#comment309686> nit: typo ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java Line 1937 (original), 1880 (patched) <https://reviews.apache.org/r/72532/#comment309689> Does this need to be public? - Karen Coppage On June 4, 2020, 7:35 a.m., Peter Varga wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72532/ > ----------------------------------------------------------- > > (Updated June 4, 2020, 7:35 a.m.) > > > Review request for hive, Karen Coppage, Marta Kuczora, and Peter Vary. > > > Repository: hive-git > > > Description > ------- > > since HIVE-21225 there are two redundant implementation of the > AcidUtils.getAcidState. > > The previous implementation (without the recursive listing) can be removed. > > Also the performance can be improved, by removing unnecessary fileStatus > calls. > > > Diffs > ----- > > > hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java > 569de706df > > hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/mutate/StreamingAssert.java > 86f762e97c > ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java bf332bc0b8 > ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java ca234cfb37 > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1059cb227f > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java > 16c915959c > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java > 598220b0c4 > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 5fa3d9ad42 > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > 018c73376f > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java > fa2ede3738 > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java > d83a50f555 > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java > 5e11d8d2d8 > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java > 1bdec7df2d > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java a96cf1e731 > ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 366282a30f > ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java 9e6d47ebc5 > ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java > 12a15a16eb > ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestOrcRawRecordMerger.java > f63c40a7b5 > streaming/src/test/org/apache/hive/streaming/TestStreaming.java 6101caac66 > > > Diff: https://reviews.apache.org/r/72532/diff/2/ > > > Testing > ------- > > > Thanks, > > Peter Varga > >
