> On máj. 18, 2020, 12:51 du, Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > > Lines 305 (patched) > > <https://reviews.apache.org/r/72281/diff/1/?file=2216347#file2216347line305> > > > > Migth want to add asserts here to check non-null argument > > Karen Coppage wrote: > I think if the StorageDescriptor is null, a NPE *should* be thrown > because that would be a huge problem, "non-null" is in the JavaDoc contract, > this method is used once, and will make it a private method in the next > patch.
Since it become private, it is even better! :) Normally for public methods I perfer using, so if someone tries to use this then easier to identify what went wrong. Like this: ``` assert sd != null : "Non-null sd is required" ``` - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72281/#review220805 ----------------------------------------------------------- On máj. 19, 2020, 5:58 de, Karen Coppage wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72281/ > ----------------------------------------------------------- > > (Updated máj. 19, 2020, 5:58 de) > > > Review request for hive, Laszlo Pinter and Peter Vary. > > > Bugs: HIVE-22971 > https://issues.apache.org/jira/browse/HIVE-22971 > > > Repository: hive-git > > > Description > ------- > > File rename is expensive for object stores, so MM (insert-only) compaction > should skip that step when committing and write directly to base_x_cZ or > delta_x_y_cZ. > > This also fixes the issue that for MM QB compaction the temp tables were > stored under the table directory, and these temp dirs were never cleaned up. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java f5ad3a882b7 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java > b9db1d1bb98 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java > 89920ccebf4 > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > 9410a963518 > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MajorQueryCompactor.java > c70d4f33a80 > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java > 4d0e5f703e7 > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java > 724a4375b75 > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java > 1cd95f80155 > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java > 7f3ccfa04ed > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactorFactory.java > 6542eef58af > > > Diff: https://reviews.apache.org/r/72281/diff/2/ > > > Testing > ------- > > > Thanks, > > Karen Coppage > >