-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55994/#review163277
-----------------------------------------------------------
Hi Sergio,
Looking through the patch I did not find anything useful to enhance what you
did :)
I did run the yetus pre-commit stuff on it (author, checkstyle, javadoc,
findbugs, whitespace, asflicense), and it came up with the following nits:
./ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:1:/**: warning:
File length is 3,809 lines (max allowed is 2,000).
./ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:2098: public
static ContentSummary getInputSummary(final Context ctx, MapWork work,
PathFilter filter):3: warning: Method length is 179 lines (max allowed is 150).
./ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:2140:
new
ThreadFactoryBuilder().setDaemon(true).setNameFormat("Get-Input-Summary-%d").build());:
warning: Line is longer than 100 characters (found 102).
./ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:2192:
if (partDesc.getTableDesc() != null && partDesc.getTableDesc().getProperties()
!= null) {: warning: Line is longer than 100 characters (found 105).
./ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:2193:
metaTableStorage =
partDesc.getTableDesc().getProperties().getProperty(hive_metastoreConstants.META_TABLE_STORAGE,
null);: warning: Line is longer than 100 characters (found 139).
./ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:2196:
metaTableStorage =
partDesc.getProperties().getProperty(hive_metastoreConstants.META_TABLE_STORAGE,
metaTableStorage);: warning: Line is longer than 100 characters (found 136).
I do not think that we are able to keep files shorter than 2000 lines in Hive,
so I will remove that check, but AFAIK we try to keep the line length to 100.
Or should I change that too?
Thanks,
Peter
- Peter Vary
On Jan. 26, 2017, 8:09 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55994/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2017, 8:09 p.m.)
>
>
> Review request for hive, Mohit Sabharwal, Sahil Takiar, and Vihang
> Karajgaonkar.
>
>
> 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
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
> 68dd5e7247415dec1e353010ea34481c4f2fc6cd
> 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
>
>
