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

Reply via email to