-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55994/
-----------------------------------------------------------

(Updated Jan. 27, 2017, 7:21 p.m.)


Review request for hive, Mohit Sabharwal, Sahil Takiar, and Vihang Karajgaonkar.


Changes
-------

- Addressed feedback comments.
- Fixed the total += estimator.estimate(jobConf, scanOp, -1).getTotalLength(); 
line to use the cloned JobConf

Comments
- PlanUtils.configureJobPropertiesForStorageHandler() modifies the TableDesc 
properties. It is just one assignment that I think is atomic. And, this 
properties is not used anywhere on the code path that can caus e multi-thread 
conflicts. Please verify I'm correct.


Bugs: HIVE-15736
    https://issues.apache.org/jira/browse/HIVE-15736


Repository: hive-git


Description
-------

Added unit tests on TestUtilities to validate 
- Single and multiple threads 
- InputEstimator usage
- ContentSummaryInputFormat usage.

It also fixed an issue with the InputEstimator scenario where the values 
returned by the InputEstimator where overriden later by the correct filesystem 
calls.

An interesting thing (code commented while it is on review) is that when 
executing the InputEstimator code path the line commented seems are not needed. 
It might
be that the idea was to set some configurations to the jobConf, but the jobConf 
was never passed as parameter to the estimate method. Please help me verify 
this.


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 
68dd5e7247415dec1e353010ea34481c4f2fc6cd 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 
b2c586507483eb5588e07a9ceba3caf395b4d607 
  ql/src/test/org/apache/hadoop/hive/ql/exec/InputEstimatorTestClass.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java 
e444946e990d9adb90ce24837cfe4edcf5126d3a 

Diff: https://reviews.apache.org/r/55994/diff/


Testing
-------

Waiting for tests HiveQA


Thanks,

Sergio Pena

Reply via email to