Re: Review Request 62810: HIVE-17300 WebUI query plan graphs

[Barna Zsombor Klara]

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


Ship it!




Ship It!

- Barna Zsombor Klara


On Oct. 30, 2017, 4:23 p.m., Peter Vary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62810/
> ---
> 
> (Updated Oct. 30, 2017, 4:23 p.m.)
> 
> 
> Review request for hive, Karen Coppage, Xuefu Zhang, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-17300
> https://issues.apache.org/jira/browse/HIVE-17300
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Moving the review here, since could not change Karen's original one
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c7 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6631a6e 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6c6ad92 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java 132bec6 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> 2d2eafd 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 41a1ef1 
>   service/src/jamon/org/apache/hive/tmpl/QueryProfileTmpl.jamon ff7476e 
>   service/src/resources/hive-webapps/static/css/query-plan-graph.css 
> PRE-CREATION 
>   service/src/resources/hive-webapps/static/js/query-plan-graph.js 
> PRE-CREATION 
>   service/src/resources/hive-webapps/static/js/vis.min.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62810/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Vary
> 
>

Re: Review Request 62810: HIVE-17300 WebUI query plan graphs

[Peter Vary via Review Board]

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

(Updated Oct. 30, 2017, 4:23 p.m.)


Review request for hive, Karen Coppage, Xuefu Zhang, and Xuefu Zhang.


Changes
---

Addressing Zsombor's comments


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


Repository: hive-git


Description
---

Moving the review here, since could not change Karen's original one


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c7 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6631a6e 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6c6ad92 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514 
  ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java 132bec6 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
2d2eafd 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 41a1ef1 
  service/src/jamon/org/apache/hive/tmpl/QueryProfileTmpl.jamon ff7476e 
  service/src/resources/hive-webapps/static/css/query-plan-graph.css 
PRE-CREATION 
  service/src/resources/hive-webapps/static/js/query-plan-graph.js PRE-CREATION 
  service/src/resources/hive-webapps/static/js/vis.min.js PRE-CREATION 


Diff: https://reviews.apache.org/r/62810/diff/2/

Changes: https://reviews.apache.org/r/62810/diff/1-2/


Testing
---


Thanks,

Peter Vary

Re: Review Request 62810: HIVE-17300 WebUI query plan graphs

[Barna Zsombor Klara]

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



Thank you for the patch Peter (and Karen). I have 3 minor comments if you fixed 
those, then we can ship it.


common/src/java/org/apache/hadoop/hive/common/LogUtils.java
Lines 239-240 (patched)


Can you please check that this cast is always correct? An if with an 
instance of check should be enough.



ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java
Lines 159 (patched)


I would prefer an iterator with a type parameter. This way we can avoid the 
explicit cast 2 lines below.



ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java
Lines 162 (patched)


Same as before, please use a typed iterator if possible.


- Barna Zsombor Klara


On Oct. 6, 2017, 3:37 p.m., Peter Vary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62810/
> ---
> 
> (Updated Oct. 6, 2017, 3:37 p.m.)
> 
> 
> Review request for hive, Karen Coppage, Xuefu Zhang, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-17300
> https://issues.apache.org/jira/browse/HIVE-17300
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Moving the review here, since could not change Karen's original one
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c7 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d2afc2c 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1943c6d 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java bf6cb91 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> 3c07197 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 41a1ef1 
>   service/src/jamon/org/apache/hive/tmpl/QueryProfileTmpl.jamon ff7476e 
>   service/src/resources/hive-webapps/static/css/query-plan-graph.css 
> PRE-CREATION 
>   service/src/resources/hive-webapps/static/js/query-plan-graph.js 
> PRE-CREATION 
>   service/src/resources/hive-webapps/static/js/vis.min.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62810/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Vary
> 
>

Review Request 62810: HIVE-17300 WebUI query plan graphs

[Peter Vary]

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

Review request for hive, Karen Coppage, Xuefu Zhang, and Xuefu Zhang.


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


Repository: hive-git


Description
---

Moving the review here, since could not change Karen's original one


Diffs
-

  common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c7 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d2afc2c 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1943c6d 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514 
  ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java bf6cb91 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
3c07197 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 41a1ef1 
  service/src/jamon/org/apache/hive/tmpl/QueryProfileTmpl.jamon ff7476e 
  service/src/resources/hive-webapps/static/css/query-plan-graph.css 
PRE-CREATION 
  service/src/resources/hive-webapps/static/js/query-plan-graph.js PRE-CREATION 
  service/src/resources/hive-webapps/static/js/vis.min.js PRE-CREATION 


Diff: https://reviews.apache.org/r/62810/diff/1/


Testing
---


Thanks,

Peter Vary