Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-15 Thread Peter Vary via Review Board

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


Ship it!




+1 pending tests

- Peter Vary


On May 14, 2018, 8:07 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 14, 2018, 8:07 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 52799b30c39af2f192c4ae22ce7d68b403014183 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-14 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

(Updated May 14, 2018, 8:07 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
---

Adding java8 method addExact() instead of copying it.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs (updated)
-

  beeline/src/main/resources/BeeLine.properties 
c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
b217259553be472863cd33bb2259aa700e6c3528 
  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
52799b30c39af2f192c4ae22ce7d68b403014183 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
01a5b4c9c328cb034a613a1539cea2584e122fb4 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
fcdc9967f12a454a9d3f31031e2261f264479118 
  ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
  ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
28f376f8c4c2151383286e754447d1349050ef4e 
  ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
96819f4e1c446f6de423f99c7697d548ff5dbe06 
  ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


Diff: https://reviews.apache.org/r/66290/diff/10/

Changes: https://reviews.apache.org/r/66290/diff/9-10/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-14 Thread Sahil Takiar

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



I think you are safe to use the Java 8 specific method rather than copying it.

- Sahil Takiar


On May 12, 2018, 12:30 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 12, 2018, 12:30 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 52799b30c39af2f192c4ae22ce7d68b403014183 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-14 Thread Peter Vary via Review Board


> On May 11, 2018, 10:32 a.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java
> > Lines 2334 (patched)
> > 
> >
> > Is it possible to end up with overflowed long?

You can use Java8 method. No need to copy the implementation.
Thanks,
Peter


- Peter


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


On May 12, 2018, 12:30 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 12, 2018, 12:30 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 52799b30c39af2f192c4ae22ce7d68b403014183 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-11 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

(Updated May 12, 2018, 12:30 a.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
---

I added check overflow logic in Driver.java
I have a doubt here, I used addExact() method from Java8 to check overflows.
Since I am unsure that I can use java8 specific method, I just copied the 
method to Driver.java
Is this fine?
Also corrected other issues pointed out by Peter.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs (updated)
-

  beeline/src/main/resources/BeeLine.properties 
c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
b217259553be472863cd33bb2259aa700e6c3528 
  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
52799b30c39af2f192c4ae22ce7d68b403014183 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
01a5b4c9c328cb034a613a1539cea2584e122fb4 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
fcdc9967f12a454a9d3f31031e2261f264479118 
  ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
  ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
28f376f8c4c2151383286e754447d1349050ef4e 
  ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
96819f4e1c446f6de423f99c7697d548ff5dbe06 
  ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


Diff: https://reviews.apache.org/r/66290/diff/9/

Changes: https://reviews.apache.org/r/66290/diff/8-9/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-11 Thread Peter Vary via Review Board

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



All of my other comments are fixed.
Thanks,
Peter


jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Lines 706-709 (original), 706-709 (patched)


Is the comment text changed at all?



ql/src/java/org/apache/hadoop/hive/ql/Driver.java
Lines 2334 (patched)


Is it possible to end up with overflowed long?



ql/src/java/org/apache/hadoop/hive/ql/Driver.java
Lines 2334 (patched)


Is it possible to end up with overflowed long?



ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java
Lines 433 (patched)


Why not reuse ss variable instead of SessionState.get() again?


- Peter Vary


On May 10, 2018, 10:51 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 10, 2018, 10:51 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 52799b30c39af2f192c4ae22ce7d68b403014183 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-10 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

(Updated May 10, 2018, 10:51 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
---

Updating patch after fixing the mentioned issues.
Sahil, can you kindly review that all the open issues are now addressed so that 
I haven't missed anything.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs (updated)
-

  beeline/src/main/resources/BeeLine.properties 
c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
b217259553be472863cd33bb2259aa700e6c3528 
  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
52799b30c39af2f192c4ae22ce7d68b403014183 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
01a5b4c9c328cb034a613a1539cea2584e122fb4 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
fcdc9967f12a454a9d3f31031e2261f264479118 
  ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
  ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
28f376f8c4c2151383286e754447d1349050ef4e 
  ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
96819f4e1c446f6de423f99c7697d548ff5dbe06 
  ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


Diff: https://reviews.apache.org/r/66290/diff/8/

Changes: https://reviews.apache.org/r/66290/diff/7-8/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-10 Thread Sahil Takiar


> On May 7, 2018, 6:07 p.m., Bharathkrishna Guruvayoor Murali wrote:
> > Added new version of patch.
> > Adding the result as "Unknown rows affected" for return value -1 from 
> > beeline.
> > Fixing test failures, and modifying tests to accommodate the change.
> > Further changes in this version are:
> >   - Using the waitForOperationToComplete method itself in 
> > HiveStatement#getUpdateCount, because in executeAsync mode it fails 
> > otherwise.
> >   - I converted the while loop to do-while in 
> > HiveStatement#waitForOperationToComplete, because otherwise some cases the 
> > response is never initialized.

why are the changes to the while loop in `HiveStatement` necessary?


- Sahil


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


On May 9, 2018, 8:46 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 9, 2018, 8:46 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 52799b30c39af2f192c4ae22ce7d68b403014183 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-10 Thread Sahil Takiar

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




ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
Lines 134 (patched)


remove `transient`, `static` fields can't be serialized anyway



service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java
Lines 23 (patched)


don't think these imports need to be re-ordered?


- Sahil Takiar


On May 9, 2018, 8:46 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 9, 2018, 8:46 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 52799b30c39af2f192c4ae22ce7d68b403014183 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-09 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

(Updated May 9, 2018, 8:46 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
---

Reverted the checkstyle/formatting only related changes.
Responded to the review comments.

Now no formatting related changes are present in the patch.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs (updated)
-

  beeline/src/main/resources/BeeLine.properties 
c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
b217259553be472863cd33bb2259aa700e6c3528 
  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
52799b30c39af2f192c4ae22ce7d68b403014183 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
01a5b4c9c328cb034a613a1539cea2584e122fb4 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
fcdc9967f12a454a9d3f31031e2261f264479118 
  ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
  ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
28f376f8c4c2151383286e754447d1349050ef4e 
  ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
96819f4e1c446f6de423f99c7697d548ff5dbe06 
  ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


Diff: https://reviews.apache.org/r/66290/diff/7/

Changes: https://reviews.apache.org/r/66290/diff/6-7/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-09 Thread Bharathkrishna Guruvayoor Murali via Review Board


> On May 8, 2018, 10:42 a.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java
> > Lines 430-439 (patched)
> > 
> >
> > Why did you moved this inside the if statement?

Moved this inside to not have null pointer exceptions.
Noticed in some tests that sessionstate could be null. Also added the null 
check for the counter along with that.


- Bharathkrishna


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


On May 7, 2018, 5:58 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 7, 2018, 5:58 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 9f4e6f2e53b43839fefe1d2522a75a95d393544f 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-08 Thread Peter Vary via Review Board

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



Almost there. Mostly just formatting questions, so you will have easier time to 
backport the change.
Only 1 probably relevant question in the end


jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 68 (original), 68 (patched)


nit: Do we need this change? Please remove formatting only changes. It will 
be easier to backport the change if only contains relevant changes



jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 272 (original), 271 (patched)


nit: Do we need this change?



jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 300 (original), 298 (patched)


nit: Do we need this change?



jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 347 (original), 344 (patched)


nit: Do we need this change?



jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 927 (original), 928 (patched)


nit: Do we need this change?



jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Lines 934-936 (original), 935-937 (patched)


nit: Do we need this change?



jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 941 (original), 942 (patched)


nit: Do we need this change?



ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
Lines 86 (patched)


nit: Do we need this change?



ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
Lines 90 (patched)


nit: Do we need this change?



ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
Lines 133 (patched)


nit: Do we need this change?



ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java
Lines 430-439 (patched)


Why did you moved this inside the if statement?


- Peter Vary


On May 7, 2018, 5:58 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 7, 2018, 5:58 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 9f4e6f2e53b43839fefe1d2522a75a95d393544f 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   

Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-07 Thread Bharathkrishna Guruvayoor Murali via Review Board

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



Added new version of patch.
Adding the result as "Unknown rows affected" for return value -1 from beeline.
Fixing test failures, and modifying tests to accommodate the change.
Further changes in this version are:
  - Using the waitForOperationToComplete method itself in 
HiveStatement#getUpdateCount, because in executeAsync mode it fails otherwise.
  - I converted the while loop to do-while in 
HiveStatement#waitForOperationToComplete, because otherwise some cases the 
response is never initialized.

- Bharathkrishna Guruvayoor Murali


On May 7, 2018, 5:58 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated May 7, 2018, 5:58 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   beeline/src/main/resources/BeeLine.properties 
> c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> b217259553be472863cd33bb2259aa700e6c3528 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 9f4e6f2e53b43839fefe1d2522a75a95d393544f 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> 01a5b4c9c328cb034a613a1539cea2584e122fb4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
> 18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
>   ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
> 28f376f8c4c2151383286e754447d1349050ef4e 
>   ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
> 96819f4e1c446f6de423f99c7697d548ff5dbe06 
>   ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
> d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-07 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

(Updated May 7, 2018, 5:58 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs (updated)
-

  beeline/src/main/resources/BeeLine.properties 
c41b3ed637e04d8d2d9800ad5e9284264f7e4055 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
b217259553be472863cd33bb2259aa700e6c3528 
  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
9f4e6f2e53b43839fefe1d2522a75a95d393544f 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
01a5b4c9c328cb034a613a1539cea2584e122fb4 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
fcdc9967f12a454a9d3f31031e2261f264479118 
  ql/src/test/results/clientpositive/llap/dp_counter_mm.q.out 
18f4c69a191bde3cae2d5efac5ef20fd0b1a9f0c 
  ql/src/test/results/clientpositive/llap/dp_counter_non_mm.q.out 
28f376f8c4c2151383286e754447d1349050ef4e 
  ql/src/test/results/clientpositive/llap/orc_ppd_basic.q.out 
96819f4e1c446f6de423f99c7697d548ff5dbe06 
  ql/src/test/results/clientpositive/llap/tez_input_counters.q.out 
d2fcdaa1bfba03e1f0e4191c8d056b05f334443d 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


Diff: https://reviews.apache.org/r/66290/diff/6/

Changes: https://reviews.apache.org/r/66290/diff/5-6/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-05-01 Thread Sahil Takiar

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


Fix it, then Ship it!




One minor comment, otherwise LGTM.


jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Lines 713 (patched)


Should we modify beeline so if it gets a value of `-1` it says something 
like "Unknown number of rows affected", or just don't print anything at all. 
Right now it would print "No rows affected..." if an overflow happens, which 
would be odd.


- Sahil Takiar


On April 27, 2018, 11:05 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 27, 2018, 11:05 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 41ad002abf3d2a6969ef0d1d48f7db22e096bb47 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-27 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

(Updated April 27, 2018, 11:05 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs (updated)
-

  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
41ad002abf3d2a6969ef0d1d48f7db22e096bb47 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
c084fa054cb771bfdb033d244935713e3c7eb874 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
fcdc9967f12a454a9d3f31031e2261f264479118 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


Diff: https://reviews.apache.org/r/66290/diff/5/

Changes: https://reviews.apache.org/r/66290/diff/4-5/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-26 Thread Sahil Takiar

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




jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 712 (original), 712 (patched)


how should we be handling overflows here? in Hive especially, the number of 
rows we write can be very high, larger than `INTEGER.MAX_VALUE`

perhaps we should emit a warning if an overflow occurs, and just return 
`INTEGER.MAX_VALUE`. we probably don't want the job to fail just because we 
can't report an accurrate count



ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java
Lines 41 (patched)


its still `numModifiedRows` right?



ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
Lines 132 (patched)


Maybe change to `TOTAL_TABLE_ROWS_WRITTEN` the current name seems too wordy.


- Sahil Takiar


On April 23, 2018, 9:56 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 23, 2018, 9:56 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 9cb2ff101581d22965b447e82601970d909daefd 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-25 Thread Bharathkrishna Guruvayoor Murali via Review Board


> On March 27, 2018, 12:53 p.m., Peter Vary wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
> > Line 712 (original), 712 (patched)
> > 
> >
> > Is it possible to behave differently, when we have information about 
> > the number of rows, and when we do not know anything? The returned number 
> > will be 0 in this case, which might cause interesting behavior I guess :)
> 
> Bharathkrishna Guruvayoor Murali wrote:
> This is called only when resultSet is not present (like in insert 
> queries). If it returns zero, it will display No rows affected, which is like 
> the current behavior.
> 
> Peter Vary wrote:
> So the code handles -1, and 0 in the same way?
> Previously we returned -1 indicating we do not have info about the 
> affected rows. No we always will return 0 if we do not have the exact info, 
> like when running HoS
> 
> Bharathkrishna Guruvayoor Murali wrote:
> I have changed the default value in QueryState for numModifiedRows to -1 
> from 0.
> Currently it shows the same message on beeline for both -1 and 0, but I 
> guess it is good to have default as -1 in case we have any use-case that 
> needs to distinguish these two cases.

Please ignore my previous message, I think my change of having default value of 
-1 for numModifiedRows in QueryState won't have an effect, because it will be 
overwritten as 0.
I will keep the default value of numModifiedRows as 0 itself. 
I do not think it is a problem because even the present behavior when it 
returns -1, it shows no rows affected, hence I think current behavior won't be 
affected.


- Bharathkrishna


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


On April 23, 2018, 9:56 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 23, 2018, 9:56 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 9cb2ff101581d22965b447e82601970d909daefd 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: 

Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-23 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

(Updated April 23, 2018, 9:56 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs (updated)
-

  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
9cb2ff101581d22965b447e82601970d909daefd 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
c084fa054cb771bfdb033d244935713e3c7eb874 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
fcdc9967f12a454a9d3f31031e2261f264479118 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


Diff: https://reviews.apache.org/r/66290/diff/3/

Changes: https://reviews.apache.org/r/66290/diff/2-3/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-23 Thread Bharathkrishna Guruvayoor Murali via Review Board


- Bharathkrishna


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


On April 18, 2018, 11:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 18, 2018, 11:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> a88453c97835db847d74b4b4c3ef318d4d6c0ce5 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-23 Thread Bharathkrishna Guruvayoor Murali via Review Board


- Bharathkrishna


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


On April 18, 2018, 11:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 18, 2018, 11:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> a88453c97835db847d74b4b4c3ef318d4d6c0ce5 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-23 Thread Bharathkrishna Guruvayoor Murali via Review Board


> On March 27, 2018, 12:53 p.m., Peter Vary wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
> > Line 712 (original), 712 (patched)
> > 
> >
> > Is it possible to behave differently, when we have information about 
> > the number of rows, and when we do not know anything? The returned number 
> > will be 0 in this case, which might cause interesting behavior I guess :)
> 
> Bharathkrishna Guruvayoor Murali wrote:
> This is called only when resultSet is not present (like in insert 
> queries). If it returns zero, it will display No rows affected, which is like 
> the current behavior.
> 
> Peter Vary wrote:
> So the code handles -1, and 0 in the same way?
> Previously we returned -1 indicating we do not have info about the 
> affected rows. No we always will return 0 if we do not have the exact info, 
> like when running HoS

I have changed the default value in QueryState for numModifiedRows to -1 from 0.
Currently it shows the same message on beeline for both -1 and 0, but I guess 
it is good to have default as -1 in case we have any use-case that needs to 
distinguish these two cases.


- Bharathkrishna


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


On April 18, 2018, 11:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 18, 2018, 11:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> a88453c97835db847d74b4b4c3ef318d4d6c0ce5 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-23 Thread Bharathkrishna Guruvayoor Murali via Review Board


> On April 20, 2018, 4:34 p.m., Sahil Takiar wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
> > Line 711 (original), 711 (patched)
> > 
> >
> > why change the method call? don't both methods return the same thing?
> > 
> > also its more like `numModifiedRows`, right?

Changed numRows to numModifiedRows.

The return value from the method waitForOperationToComplete() was never used 
here.
This method gets called once from execute(), and the boolean 
isOperationComplete will already be set to true.
Hence, when we call this method again, it returns the TGetOperationStatusResp 
as null.
So I think using the alternate method suffices the requirement here.


> On April 20, 2018, 4:34 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java
> > Lines 431 (patched)
> > 
> >
> > When you look at the counters in the log, is there anything like 
> > `RECORDS_OUT_[table-name]` do we know how that gets populated?

I am populating the value for the counter I defined in the same way as the 
above mentioned counter is populated. I agree that it is like a duplicate for 
this counter.
But to get the value of the RECORDS_OUT_{ID}_{table_name} counter, I need to 
get the destTableId and tableName as present in the FileSinkOperator.
I could not find any straight-forward way to get these values. Hence, I thought 
of creating a counter with fixed name so that it can be used from places 
outside the Operator context.


- Bharathkrishna


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


On April 18, 2018, 11:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 18, 2018, 11:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> a88453c97835db847d74b4b4c3ef318d4d6c0ce5 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: 

Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-23 Thread Peter Vary via Review Board


> On March 27, 2018, 12:53 p.m., Peter Vary wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
> > Line 712 (original), 712 (patched)
> > 
> >
> > Is it possible to behave differently, when we have information about 
> > the number of rows, and when we do not know anything? The returned number 
> > will be 0 in this case, which might cause interesting behavior I guess :)
> 
> Bharathkrishna Guruvayoor Murali wrote:
> This is called only when resultSet is not present (like in insert 
> queries). If it returns zero, it will display No rows affected, which is like 
> the current behavior.

So the code handles -1, and 0 in the same way?
Previously we returned -1 indicating we do not have info about the affected 
rows. No we always will return 0 if we do not have the exact info, like when 
running HoS


> On March 27, 2018, 12:53 p.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java
> > Lines 2213 (patched)
> > 
> >
> > I think this information is only available when the execution engine is 
> > MR. Do we have information available when using Spark as an execution 
> > enginge?
> 
> Bharathkrishna Guruvayoor Murali wrote:
> Currently in this JIRA, I am focussing on updating the count for MR as 
> execution engine. I will create follow-up JIRA to have this working in Spark 
> as well, once I have this fix in place.

Ok. Do it on a followup jira..


- Peter


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


On April 18, 2018, 11:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 18, 2018, 11:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> a88453c97835db847d74b4b4c3ef318d4d6c0ce5 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor 

Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-20 Thread Sahil Takiar

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




jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 711 (original), 711 (patched)


why change the method call? don't both methods return the same thing?

also its more like `numModifiedRows`, right?



ql/src/java/org/apache/hadoop/hive/ql/QueryState.java
Lines 52 (patched)


it's not really `numRows` its more like `numModfiedRows` right?



ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java
Lines 431 (patched)


When you look at the counters in the log, is there anything like 
`RECORDS_OUT_[table-name]` do we know how that gets populated?


- Sahil Takiar


On April 18, 2018, 11:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated April 18, 2018, 11:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> a88453c97835db847d74b4b4c3ef318d4d6c0ce5 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> fcdc9967f12a454a9d3f31031e2261f264479118 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 
> 3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> c64c99120ad21ee98af81ec6659a2722e3e1d1c7 
> 
> 
> Diff: https://reviews.apache.org/r/66290/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-18 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

(Updated April 18, 2018, 11:53 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs (updated)
-

  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
a88453c97835db847d74b4b4c3ef318d4d6c0ce5 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
c084fa054cb771bfdb033d244935713e3c7eb874 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
fcdc9967f12a454a9d3f31031e2261f264479118 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali



Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-04-18 Thread Bharathkrishna Guruvayoor Murali via Review Board


> On March 27, 2018, 12:53 p.m., Peter Vary wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
> > Line 712 (original), 712 (patched)
> > 
> >
> > Is it possible to behave differently, when we have information about 
> > the number of rows, and when we do not know anything? The returned number 
> > will be 0 in this case, which might cause interesting behavior I guess :)

This is called only when resultSet is not present (like in insert queries). If 
it returns zero, it will display No rows affected, which is like the current 
behavior.


> On March 27, 2018, 12:53 p.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java
> > Lines 2213 (patched)
> > 
> >
> > I think this information is only available when the execution engine is 
> > MR. Do we have information available when using Spark as an execution 
> > enginge?

Currently in this JIRA, I am focussing on updating the count for MR as 
execution engine. I will create follow-up JIRA to have this working in Spark as 
well, once I have this fix in place.


> On March 27, 2018, 12:53 p.m., Peter Vary wrote:
> > service/src/java/org/apache/hive/service/cli/operation/Operation.java
> > Line 186 (original), 186 (patched)
> > 
> >
> > I am not really confortable leaking the QueryState object here. It 
> > contains other sensitve informations too. It might be better to have a 
> > method to return the number of rows instead. What do you think?

That is right, changing this to return only the numRows so we can avoid 
exposing entire queryState object.


- Bharathkrishna


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


On March 26, 2018, 9:35 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated March 26, 2018, 9:35 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 75f928b69d3d7b206564216d24be450848a1fe8a 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java 
> c28ef99621e67a5b16bf02a1112df2ec993c4f79 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> eb3a11a8815e35dee825edb7d3246c8ecef6b0a7 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
> 0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
> 60183dae9e9927bd09a9676e49eeb4aea2401737 
>   service/src/java/org/apache/hive/service/cli/CLIService.java 
> c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
>   service/src/java/org/apache/hive/service/cli/OperationStatus.java 
> 52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
>   

Re: Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-03-27 Thread Peter Vary via Review Board

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



Hi Bharathkrishna,

This is a long missing feature. Thanks for your patch!
I have some questions below.

Thanks,
Peter


jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Line 712 (original), 712 (patched)


Is it possible to behave differently, when we have information about the 
number of rows, and when we do not know anything? The returned number will be 0 
in this case, which might cause interesting behavior I guess :)



ql/src/java/org/apache/hadoop/hive/ql/Driver.java
Lines 2213 (patched)


I think this information is only available when the execution engine is MR. 
Do we have information available when using Spark as an execution enginge?



ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
Lines 67 (patched)


nit: Do we need this import?



ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
Lines 592 (patched)


We should not use arbitary string values as identifiers. It would be good 
to define this as a public static final variable I think...



ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java
Lines 41 (patched)


nit: Unneccessary change



ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java
Lines 426 (patched)


We should not use arbitary string values as identifiers. It would be good 
to define this as a public static final variable I think...



service/src/java/org/apache/hive/service/cli/operation/Operation.java
Line 186 (original), 186 (patched)


I am not really confortable leaking the QueryState object here. It contains 
other sensitve informations too. It might be better to have a method to return 
the number of rows instead. What do you think?



service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java
Lines 23 (patched)


nit: Do not use * based imports


- Peter Vary


On March 26, 2018, 9:35 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66290/
> ---
> 
> (Updated March 26, 2018, 9:35 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-14388
> https://issues.apache.org/jira/browse/HIVE-14388
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently, when you run insert command on beeline, it returns a message 
> saying "No rows affected .."
> A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"
> 
> Added the numRows parameter as part of QueryState.
> Adding the numRows to the response as well to display in beeline.
> 
> Getting the count in FileSinkOperator and setting it in statsMap, when it 
> operates only on table specific rows for the particular operation. (so that 
> we can get only the insert to table count and avoid counting non-table 
> specific file-sink operations happening during query execution).
> 
> 
> Diffs
> -
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> 06542cee02e5dc4696f2621bb45cc4f24c67dfda 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 75f928b69d3d7b206564216d24be450848a1fe8a 
>   ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
> cf9c2273159c0d779ea90ad029613678fb0967a6 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> c084fa054cb771bfdb033d244935713e3c7eb874 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java 
> c28ef99621e67a5b16bf02a1112df2ec993c4f79 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> eb3a11a8815e35dee825edb7d3246c8ecef6b0a7 
>   service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
> 4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
> b2b62c71492b844f4439367364c5c81aa62f3908 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
>  15e8220eb3eb12b72c7b64029410dced33bc0d72 
>   service-rpc/src/gen/thrift/gen-php/Types.php 
> abb7c1ff3a2c8b72dc97689758266b675880e32b 
>   

Review Request 66290: HIVE-14388 : Add number of rows inserted message after insert command in Beeline

2018-03-26 Thread Bharathkrishna Guruvayoor Murali via Review Board

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

Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Currently, when you run insert command on beeline, it returns a message saying 
"No rows affected .."
A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)"

Added the numRows parameter as part of QueryState.
Adding the numRows to the response as well to display in beeline.

Getting the count in FileSinkOperator and setting it in statsMap, when it 
operates only on table specific rows for the particular operation. (so that we 
can get only the insert to table count and avoid counting non-table specific 
file-sink operations happening during query execution).


Diffs
-

  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
06542cee02e5dc4696f2621bb45cc4f24c67dfda 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
75f928b69d3d7b206564216d24be450848a1fe8a 
  ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 
cf9c2273159c0d779ea90ad029613678fb0967a6 
  ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
706c9ffa48b9c3b4a6fdaae78bab1d39c3d0efda 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
c084fa054cb771bfdb033d244935713e3c7eb874 
  ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java 
c28ef99621e67a5b16bf02a1112df2ec993c4f79 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
eb3a11a8815e35dee825edb7d3246c8ecef6b0a7 
  service-rpc/if/TCLIService.thrift 30f8af7f3e6e0598b410498782900ac27971aef0 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 
4321ad6d3c966d30f7a69552f91804cf2f1ba6c4 
  service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 
b2b62c71492b844f4439367364c5c81aa62f3908 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TGetOperationStatusResp.java
 15e8220eb3eb12b72c7b64029410dced33bc0d72 
  service-rpc/src/gen/thrift/gen-php/Types.php 
abb7c1ff3a2c8b72dc97689758266b675880e32b 
  service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py 
0f8fd0745be0f4ed9e96b7bbe0f092d03649bcdf 
  service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 
60183dae9e9927bd09a9676e49eeb4aea2401737 
  service/src/java/org/apache/hive/service/cli/CLIService.java 
c9914ba9bf8653cbcbca7d6612e98a64058c0fcc 
  service/src/java/org/apache/hive/service/cli/OperationStatus.java 
52cc3ae4f26b990b3e4edb52d9de85b3cc25f269 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 
3706c72abc77ac8bd77947cc1c5d084ddf965e9f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
c64c99120ad21ee98af81ec6659a2722e3e1d1c7 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali