Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-31 Thread Chaoyu Tang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55994/#review163784 --- Ship it! LGTM, +1 - Chaoyu Tang On Jan. 30, 2017, 8:48

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-30 Thread Sergio Pena
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55994/ --- (Updated Jan. 30, 2017, 8:48 p.m.) Review request for hive, Mohit Sabharwal,

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-30 Thread Sergio Pena
> On Jan. 28, 2017, 1:30 a.m., Sahil Takiar wrote: > > ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java, line 894 > > > > > > Not sure I understand this part. Passing in false invokes > >

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-30 Thread Peter Vary
> On Jan. 27, 2017, 3:37 p.m., Peter Vary wrote: > > 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

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-27 Thread Sahil Takiar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55994/#review163375 --- ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java (line

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-27 Thread Sergio Pena
> On Jan. 27, 2017, 3:37 p.m., Peter Vary wrote: > > 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

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-27 Thread Peter Vary
> On jan. 27, 2017, 3:37 du, Peter Vary wrote: > > 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

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-27 Thread Sergio Pena
--- 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,

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-27 Thread Sergio Pena
> On Jan. 27, 2017, 3:37 p.m., Peter Vary wrote: > > 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

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-27 Thread Sergio Pena
> On Jan. 27, 2017, 3:23 a.m., Sahil Takiar wrote: > > ql/src/test/org/apache/hadoop/hive/ql/exec/InputEstimatorTestClass.java, > > line 40 > > > > > > Is this necessary? It was the only way I thought on testing

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-27 Thread Peter Vary
--- 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

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-26 Thread Sahil Takiar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55994/#review163222 --- ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java (line

Re: Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-26 Thread Sergio Pena
--- 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,

Review Request 55994: HIVE-15736: Add unit tests to Utilities.getInputSummary() method for multi-threading cases

2017-01-26 Thread Sergio Pena
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55994/ --- Review request for hive, Mohit Sabharwal, Sahil Takiar, and Vihang Karajgaonkar.