[jira] [Created] (HIVE-21526) JSONDropDatabaseMessage needs to have the full database object.

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-21526:
---

 Summary: JSONDropDatabaseMessage needs to have the full database 
object.
 Key: HIVE-21526
 URL: https://issues.apache.org/jira/browse/HIVE-21526
 Project: Hive
  Issue Type: Improvement
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali


The metastore notification event DROP_DATABASE does not provide full-thrift 
objects as of now.
We have added CREATION_TIME to databases in HIVE-21077, and metadata like this 
would be useful in notification processing. One of the use-cases is IMPALA-8338.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (HIVE-21205) Tests for replace flag in insert event messages in Metastore notifications.

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-21205:
---

 Summary: Tests for replace flag in insert event messages in 
Metastore notifications.
 Key: HIVE-21205
 URL: https://issues.apache.org/jira/browse/HIVE-21205
 Project: Hive
  Issue Type: Test
Reporter: Bharathkrishna Guruvayoor Murali


The replace flag is initially added in HIVE-16197. It would be good to have 
some tests in TestDbNotificationListener to validate if the flag is set as 
expected.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 69664: HIVE-21077 : Database and Catalogs should have creation time

[Bharathkrishna Guruvayoor Murali via Review Board]

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


Fix it, then Ship it!





standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift
Lines 346 (patched)
<https://reviews.apache.org/r/69664/#comment297546>

nit : you can add the same comment added in the DB part.
// creation time of catalog in seconds since epoch


- Bharathkrishna Guruvayoor Murali


On Jan. 9, 2019, 6:50 p.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69664/
> ---
> 
> (Updated Jan. 9, 2019, 6:50 p.m.)
> 
> 
> Review request for hive, Karthik Manamcheri, Naveen Gangam, and Peter Vary.
> 
> 
> Bugs: HIVE-21077
> https://issues.apache.org/jira/browse/HIVE-21077
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-21077 : Database and Catalogs should have creation time
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Catalog.java
>  3eb4dbd51110dd6e5d04c3bdacde2e5bdba09a7c 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Database.java
>  994797698a379e0b08604d73d2d6728a2fcee4df 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php
>  2953a8f327eabdee42dbc66e0c65f89d17add59a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py
>  f8b862862de4dde8dce3d0dc5f70eafb67b02d2c 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb
>  dfc5d7b294c1df8d5c6b0e7d676a77ba1181e076 
>   standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 
> 7d09a5c296a8ae924d61b200b4cb9135440fd9a0 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  a9398ae1e79404a15894aa42f451df5d18ed3e4c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
>  58dc6eefcb840d4dd70af7a47811fab1b5e696d9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  d43c0c1e70cffbebd39b05f89ec396227c58ac77 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/client/builder/DatabaseBuilder.java
>  f3d2182a04ab81417a4ba58d9340721513e8 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/model/MCatalog.java
>  e82cb4322f6e2ac7afeb5efcec7517a68c8b2dee 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/model/MDatabase.java
>  815b39c483b2233660310983d58194fb1ab2d107 
>   standalone-metastore/metastore-server/src/main/resources/package.jdo 
> caaec457194332a99d5cd57bef746e969dd38161 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  a3c4196dbff7e53be5317631b314983d16a99020 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  bcaebd18accf86846ae44a6498046514575fc069 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  5ea1b4450d8258e841bb4af7381ca6fb0ba1a827 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  edde08db9ef7ee01800c7cc3a04c813014abdd18 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  a59c7d7e933d25d8d5af611e5b6aa0c0c19b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  701acb00984c61f7511dcc48053890b154575d1f 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  b1980c5b83f16614845063516495188ebdd8c2a3 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  b9f63313251ab1fa6278b862ed9e07e62b234c04 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  9040005aa82b7a8cc5c01f257ecd47a7cc97e9b2 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  0c36069d071d4b60cc338ba729da5d22e08ca8ca 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  bb20d9f42a85510039714

[jira] [Created] (HIVE-21027) Add a configuration to include entire thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-21027:
---

 Summary: Add a configuration to include entire thrift objects in 
HMS notifications
 Key: HIVE-21027
 URL: https://issues.apache.org/jira/browse/HIVE-21027
 Project: Hive
  Issue Type: Improvement
  Components: Metastore
Affects Versions: 4.0.0
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali


Currently, we add the full thrift objects of Table / Partition in the HMS 
notification messages, starting from HIVE-15180.
We can have a configuration like NOTIFICATIONS_ADD_THRIFT_OBJECTS to do this 
under a flag.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Bharathkrishna Guruvayoor Murali via Review Board]

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



Hi, the patch looks good, I just had a quick question. Does this mean that all 
the alters happen in one transaction? Will it prevent concurrent operations for 
the whole time a large number of partitions are altered?

- Bharathkrishna Guruvayoor Murali


On Nov. 30, 2018, 11:31 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated Nov. 30, 2018, 11:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

[jira] [Created] (HIVE-20993) Update committer list

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-20993:
---

 Summary: Update committer list
 Key: HIVE-20993
 URL: https://issues.apache.org/jira/browse/HIVE-20993
 Project: Hive
  Issue Type: Task
Reporter: Bharathkrishna Guruvayoor Murali


Please update committer list:
Name: Bharath Krishna
Apache ID: bharos92
Organization: Cloudera



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Nov. 8, 2018, 5:16 p.m.)


Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang Karajgaonkar.


Changes
---

Adding awaitTermination and shutDownNow after cancelling the thread in close().


Repository: hive-git


Description
---

Improve record and memory usage logging in SparkRecordHandler


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
88dd12c05ade417aca4cdaece4448d31d4e1d65f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
 8880bb604e088755dcfb0bcb39689702fab0cb77 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java 
20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Nov. 7, 2018, 8:52 p.m.)


Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang Karajgaonkar.


Changes
---

Adding scheduledFuture.cancel with shutDown


Repository: hive-git


Description
---

Improve record and memory usage logging in SparkRecordHandler


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
88dd12c05ade417aca4cdaece4448d31d4e1d65f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
 8880bb604e088755dcfb0bcb39689702fab0cb77 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java 
20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Nov. 6, 2018, 5:08 a.m.)


Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang Karajgaonkar.


Changes
---

Adding shutDown and awaitTermination. Addressed the review comment.


Repository: hive-git


Description
---

Improve record and memory usage logging in SparkRecordHandler


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
88dd12c05ade417aca4cdaece4448d31d4e1d65f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
 8880bb604e088755dcfb0bcb39689702fab0cb77 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java 
20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Oct. 31, 2018, 5:29 p.m., Antal Sinkovits wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java
> > Line 62 (original), 83 (patched)
> > <https://reviews.apache.org/r/69107/diff/3/?file=2102836#file2102836line83>
> >
> > Probably these three rows can move to the MemoryInfoLogger class  (as 
> > static method), as from now on its his responsibility.
> > 
> > Something like MemoryInfoLogger.start() or schedule

It is used by the close method as well, so I will probably leave it there 
itself.


> On Oct. 31, 2018, 5:29 p.m., Antal Sinkovits wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java
> > Line 574 (original), 572 (patched)
> > <https://reviews.apache.org/r/69107/diff/3/?file=2102837#file2102837line574>
> >
> > Is this correct? In batch processing increasing it with only one?

I see that when I go to implementations of reducer.process(batch, 0);, it 
considers the batch as a single row. And there too they are incrementing 
runTimeRows++
So that is why it is probably done in such a way to increment row count by 1 
itself.


- Bharathkrishna


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


On Oct. 26, 2018, 5:13 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69107/
> ---
> 
> (Updated Oct. 26, 2018, 5:13 p.m.)
> 
> 
> Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Improve record and memory usage logging in SparkRecordHandler
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
> 88dd12c05ade417aca4cdaece4448d31d4e1d65f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
>  8880bb604e088755dcfb0bcb39689702fab0cb77 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
> cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java
>  20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 
> 
> 
> Diff: https://reviews.apache.org/r/69107/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Oct. 31, 2018, 11:15 p.m.)


Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang Karajgaonkar.


Changes
---

Added change for threadfactory to have daemon threads.
Addressed review comments. I am using shutDownNow instead of shutDown because 
we really do not need awaitTermination as shutDownNow will anyways interrupt 
the existing threads, and will not accept new tasks.
For awaitTermination, the main thread has to wait for a fixed amount of time 
which I feel is not really useful here, as we are not executing any critical 
task, we do not care about the logger thread as we will anyways log final count 
and memory usage in close method.


Repository: hive-git


Description
---

Improve record and memory usage logging in SparkRecordHandler


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
88dd12c05ade417aca4cdaece4448d31d4e1d65f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
 8880bb604e088755dcfb0bcb39689702fab0cb77 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java 
20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 


Diff: https://reviews.apache.org/r/69107/diff/4/

Changes: https://reviews.apache.org/r/69107/diff/3-4/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Oct. 26, 2018, 5:13 p.m.)


Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang Karajgaonkar.


Repository: hive-git


Description
---

Improve record and memory usage logging in SparkRecordHandler


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
88dd12c05ade417aca4cdaece4448d31d4e1d65f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
 8880bb604e088755dcfb0bcb39689702fab0cb77 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java 
20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

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




ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java
Line 67 (original), 67 (patched)
<https://reviews.apache.org/r/69107/#comment294627>

Creating this as a threadPool of size 1. I guess that is fine, as we know 
only one thread will be used at any point?



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java
Line 116 (original), 113 (patched)
<https://reviews.apache.org/r/69107/#comment294629>

Using shutDownNow instead of shutDown to cancel pending tasks


- Bharathkrishna Guruvayoor Murali


On Oct. 24, 2018, 8:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69107/
> ---
> 
> (Updated Oct. 24, 2018, 8:55 p.m.)
> 
> 
> Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Improve record and memory usage logging in SparkRecordHandler
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
> 88dd12c05ade417aca4cdaece4448d31d4e1d65f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
>  8880bb604e088755dcfb0bcb39689702fab0cb77 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
> cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java
>  20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 
> 
> 
> Diff: https://reviews.apache.org/r/69107/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Oct. 24, 2018, 8:51 a.m., Antal Sinkovits wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java
> > Lines 56 (patched)
> > <https://reviews.apache.org/r/69107/diff/1/?file=2101701#file2101701line56>
> >
> > Any reason why we use Timer?
> > 
> > From the timer java docs: 
> > 
> > "Java 5.0 introduced the java.util.concurrent package and one of the 
> > concurrency utilities therein is the ScheduledThreadPoolExecutor which is a 
> > thread pool for repeatedly executing tasks at a given rate or delay. It is 
> > effectively a more versatile replacement for the Timer/TimerTask 
> > combination, as it allows multiple service threads, accepts various time 
> > units, and doesn't require subclassing TimerTask (just implement Runnable). 
> > Configuring ScheduledThreadPoolExecutor with one thread makes it equivalent 
> > to Timer."

I used timer as there was only one thread, bbut I have changed it to 
ScheduledThreadPoolExecutor, as it looks more extensible


- Bharathkrishna


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


On Oct. 24, 2018, 8:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69107/
> ---
> 
> (Updated Oct. 24, 2018, 8:55 p.m.)
> 
> 
> Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Improve record and memory usage logging in SparkRecordHandler
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
> 88dd12c05ade417aca4cdaece4448d31d4e1d65f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
>  8880bb604e088755dcfb0bcb39689702fab0cb77 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
> cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java
>  20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 
> 
> 
> Diff: https://reviews.apache.org/r/69107/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Oct. 24, 2018, 8:55 p.m.)


Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang Karajgaonkar.


Changes
---

Using ThreadPoolExecutor
Addressed some review comments, and have added some comments which I want to be 
specifically reviewed.


Repository: hive-git


Description
---

Improve record and memory usage logging in SparkRecordHandler


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
88dd12c05ade417aca4cdaece4448d31d4e1d65f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
 8880bb604e088755dcfb0bcb39689702fab0cb77 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java 
20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Oct. 23, 2018, 7:50 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java
> > Line 50 (original), 53 (patched)
> > <https://reviews.apache.org/r/69107/diff/1/?file=2101701#file2101701line53>
> >
> > this need to be volatile since it is modified by the timer task

This variable is also used as 
logThresholdInterval = Math.min(maxLogThresholdInterval, 2 * 
logThresholdInterval);

Non-atomic operation. So should I make this variable atomic as well?


- Bharathkrishna


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


On Oct. 20, 2018, 7:13 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69107/
> ---
> 
> (Updated Oct. 20, 2018, 7:13 p.m.)
> 
> 
> Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Improve record and memory usage logging in SparkRecordHandler
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
> 88dd12c05ade417aca4cdaece4448d31d4e1d65f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
>  8880bb604e088755dcfb0bcb39689702fab0cb77 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
> cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java
>  20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 
> 
> 
> Diff: https://reviews.apache.org/r/69107/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Oct. 23, 2018, 7:50 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java
> > Line 49 (original), 52 (patched)
> > <https://reviews.apache.org/r/69107/diff/1/?file=2101701#file2101701line52>
> >
> > i think volatile long is sufficient here and is probably cheaper. 
> > atomics might be expensive when done per row

I first used volatile, but I replaced it with AtomicLong because the rowNumber 
needs to be incremented and rowNumber++ on a volatile variable is not 
considered a safe operation. What do you think about that?


- Bharathkrishna


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


On Oct. 20, 2018, 7:13 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69107/
> ---
> 
> (Updated Oct. 20, 2018, 7:13 p.m.)
> 
> 
> Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Improve record and memory usage logging in SparkRecordHandler
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
> 88dd12c05ade417aca4cdaece4448d31d4e1d65f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
>  8880bb604e088755dcfb0bcb39689702fab0cb77 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
> cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java
>  20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 
> 
> 
> Diff: https://reviews.apache.org/r/69107/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Review Request 69107: HIVE-20512

[Bharathkrishna Guruvayoor Murali via Review Board]

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

Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang Karajgaonkar.


Repository: hive-git


Description
---

Improve record and memory usage logging in SparkRecordHandler


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java 
88dd12c05ade417aca4cdaece4448d31d4e1d65f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java
 8880bb604e088755dcfb0bcb39689702fab0cb77 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java 
cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java 
20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68889: HIVE-20610 : TestDbNotificationListener should not use /tmp directory

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Oct. 3, 2018, 4 p.m.)


Review request for hive, Alexander Kolbasov and Andrew Sherman.


Changes
---

Fixing checkstyle errros.


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


Repository: hive-git


Description
---

Adding java.io.tmpdir as tmp directory instead of /tmp


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 82429e36a575918d92a9b22bedcd63788ec51c5f 


Diff: https://reviews.apache.org/r/68889/diff/4/

Changes: https://reviews.apache.org/r/68889/diff/3-4/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68889: HIVE-20610 : TestDbNotificationListener should not use /tmp directory

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Oct. 2, 2018, 11:51 p.m., Alexander Kolbasov wrote:
> > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
> > Lines 123 (patched)
> > <https://reviews.apache.org/r/68889/diff/2/?file=2093709#file2093709line123>
> >
> > Style: class fields usually follow normal camelCase.
> > Do we need a subdirectory for tests?
> > It is better to use joining with Path or File, e.g.
> > 
> > Path currentPath = Paths.get(System.getProperty("java.io.tmpdir"));
> > Path filePath = Paths.get(currentPath.toString(), "testDbNotif");

Fixed style issue.
Now using Paths.get to create the path.
Just added subDirectory just in case not to delete the java.io.tmpdir when it 
is being used somewhere (if at all that's possible).


- Bharathkrishna


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


On Oct. 3, 2018, 5:09 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68889/
> ---
> 
> (Updated Oct. 3, 2018, 5:09 a.m.)
> 
> 
> Review request for hive, Alexander Kolbasov and Andrew Sherman.
> 
> 
> Bugs: HIVE-20610
> https://issues.apache.org/jira/browse/HIVE-20610
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding java.io.tmpdir as tmp directory instead of /tmp
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  82429e36a575918d92a9b22bedcd63788ec51c5f 
> 
> 
> Diff: https://reviews.apache.org/r/68889/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68889: HIVE-20610 : TestDbNotificationListener should not use /tmp directory

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Oct. 3, 2018, 5:09 a.m.)


Review request for hive, Alexander Kolbasov and Andrew Sherman.


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


Repository: hive-git


Description
---

Adding java.io.tmpdir as tmp directory instead of /tmp


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 82429e36a575918d92a9b22bedcd63788ec51c5f 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68889: HIVE-20610 : TestDbNotificationListener should not use /tmp directory

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Oct. 1, 2018, 3:43 p.m., Andrew Sherman wrote:
> > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
> > Lines 123 (patched)
> > <https://reviews.apache.org/r/68889/diff/1/?file=2093130#file2093130line123>
> >
> > General comment: don't try to do too much in static initializers in 
> > server code. Just like in HIVE-20545 you have to consider what will happen 
> > if there is a failure during initialization, and the result is always ugly. 
> > In this case it looks safe but IT MADE ME THINK which is bad.

I made it final, no longer static


> On Oct. 1, 2018, 3:43 p.m., Andrew Sherman wrote:
> > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
> > Line 802 (original), 804 (patched)
> > <https://reviews.apache.org/r/68889/diff/1/?file=2093130#file2093130line804>
> >
> > This looks ugly to me. I think the string concatenation operator + 
> > should be separated on both sides by spaces. I think that is what is most 
> > commonly used on Hive - I'll leave it to you to check. But the usage is 
> > here is different from that in the static initializer code and that 
> > inconsistency is ugly too. IMHO You should teach Intellij to do your 
> > formatting and then let it decide this stuff

Corrected checkstyle


- Bharathkrishna


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


On Oct. 2, 2018, 9:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68889/
> ---
> 
> (Updated Oct. 2, 2018, 9:55 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov and Andrew Sherman.
> 
> 
> Bugs: HIVE-20610
> https://issues.apache.org/jira/browse/HIVE-20610
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding java.io.tmpdir as tmp directory instead of /tmp
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  82429e36a575918d92a9b22bedcd63788ec51c5f 
> 
> 
> Diff: https://reviews.apache.org/r/68889/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68889: HIVE-20610 : TestDbNotificationListener should not use /tmp directory

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Oct. 2, 2018, 9:55 p.m.)


Review request for hive, Alexander Kolbasov and Andrew Sherman.


Changes
---

Making some checkstyle corrections. Also, adding TEST_TEMP_DIR as just final 
and not static.


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


Repository: hive-git


Description
---

Adding java.io.tmpdir as tmp directory instead of /tmp


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 82429e36a575918d92a9b22bedcd63788ec51c5f 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 918 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091788#file2091788line918>
> >
> > You already have this check when you call this function.
> 
> Bharathkrishna Guruvayoor Murali wrote:
> I thought this could be used separately as a utility method, if the user 
> has just one predicate. Should I just keep the other method which accepts a 
> list of predicates and make this one private?

Closing this issue. Maintaining both methods as they can be used as separate 
utilities if needed.


- Bharathkrishna


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


On Oct. 2, 2018, 5:19 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Oct. 2, 2018, 5:19 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
>  7ff168f7931f91fe17f7d38df848ba2eed33c463 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Oct. 2, 2018, 5:19 a.m.)


Review request for hive and Alexander Kolbasov.


Changes
---

I added init() method to MessageFactory, and overriding it in 
JSONMessageFactory to initialize the static variables. Although the exception 
traces look mostly similar, I guess this adds a bit more clarity and 
information about the exception if the regex is invalid.


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


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f81292b0db54f4eb82468191fda38f9a0d4 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c6b10a4f9494e49a42282cf90027ad7 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
 7ff168f7931f91fe17f7d38df848ba2eed33c463 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b053205f48226da442ce65fcc2d7f6e76763 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
 30de1c4cfa1cf019186b10583a06da0bf5491634 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
> > Line 297 (original), 310 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091789#file2091789line310>
> >
> > This is used by the notifications that (we think) we understand, but it 
> > is also used by JSONAcidWriteMessage. So what happens if someone uses your 
> > new mechanism to reduce the size of messages, but affects 
> > JSONAcidWriteMessage? In other words there could be multile uses for 
> > notifications in a complex system, and this mechanism affects them all.

For now, I feel this change is reasonable and when there is a specific 
requirement that it should not filter parameters from other types of 
messages(if any), it can be taken up later on as an improvement.


- Bharathkrishna


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


On Sept. 30, 2018, 6:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 30, 2018, 6:55 p.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Review Request 68889: HIVE-20610 : TestDbNotificationListener should not use /tmp directory

[Bharathkrishna Guruvayoor Murali via Review Board]

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

Review request for hive, Alexander Kolbasov and Andrew Sherman.


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


Repository: hive-git


Description
---

Adding java.io.tmpdir as tmp directory instead of /tmp


Diffs
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 82429e36a5 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Sept. 30, 2018, 6:55 p.m.)


Review request for hive and Alexander Kolbasov.


Changes
---

corrected the typo.


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


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f81292b0db54f4eb82468191fda38f9a0d4 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c6b10a4f9494e49a42282cf90027ad7 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b053205f48226da442ce65fcc2d7f6e76763 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
 30de1c4cfa1cf019186b10583a06da0bf5491634 


Diff: https://reviews.apache.org/r/68827/diff/4/

Changes: https://reviews.apache.org/r/68827/diff/3-4/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Sept. 28, 2018, 4:59 p.m.)


Review request for hive and Alexander Kolbasov.


Changes
---

Added unit-tests which uses MetaStoreConf To make sure it works with default 
values


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


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f81292b0db54f4eb82468191fda38f9a0d4 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c6b10a4f9494e49a42282cf90027ad7 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b053205f48226da442ce65fcc2d7f6e76763 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
 30de1c4cfa1cf019186b10583a06da0bf5491634 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
> > Lines 66 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091790#file2091790line66>
> >
> > add test with the default param map from MetastoreConf

An empty exclude pattern will remove all elements from the map! But when it is 
done through configuration, the default value returns an empty array, like new 
String[0].
Hence, the predicates become an empty List. What can I do if needed to add a 
test for this.


- Bharathkrishna


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


On Sept. 28, 2018, 10:17 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 10:17 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Sept. 28, 2018, 10:17 a.m.)


Review request for hive and Alexander Kolbasov.


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


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f81292b0db54f4eb82468191fda38f9a0d4 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c6b10a4f9494e49a42282cf90027ad7 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b053205f48226da442ce65fcc2d7f6e76763 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
 30de1c4cfa1cf019186b10583a06da0bf5491634 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
> > Lines 44 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091790#file2091790line44>
> >
> > It would be better to use hamcrest asserts and just check that you map 
> > matches your expected map with a single assert.

Fixed.


- Bharathkrishna


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


On Sept. 28, 2018, 9:38 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 9:38 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f8129 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b05320 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 917 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091788#file2091788line917>
> >
> > I think a better name would be something like truncateMapByKey.
> > 
> > Can you guarantee that it always receives non-null map? Then you don't 
> > need to check for null.
> > 
> > Since you don't care about values, can this be a generic map ` > T>`?

Since this is a utility method, I wanted to check for null as it could be 
called from other classes in future.
Making the Map


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 918 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091788#file2091788line918>
> >
> > You already have this check when you call this function.

I thought this could be used separately as a utility method, if the user has 
just one predicate. Should I just keep the other method which accepts a list of 
predicates and make this one private?


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 931 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091788#file2091788line931>
> >
> > Naming - may be something like 'truncateMapByKeys'?

filter sounds better to me than truncate! I changed method name to 
filterMapKeys. Let me know if it is fine.


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
> > Line 297 (original), 310 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091789#file2091789line310>
> >
> > The default exclude filter is empty. When someone sets this variable 
> > they should understand what they are doing.

I added a better description for variable in MetastoreConf. Please let know if 
that is sufficient.


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
> > Lines 37 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091790#file2091790line37>
> >
> > Can you add a test with an empty exclude pattern?

An empty exclude pattern will remove all elements from the map! But when it is 
done through configuration, the default value returns an empty array, like new 
String[0].
Hence, the predicates become an empty List. What can I do if needed to add a 
test for this.


- Bharathkrishna


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


On Sept. 28, 2018, 9:38 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 9:38 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f8129 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b05320 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 913 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091788#file2091788line913>
> >
> > Good doumentation!
> > Are you 100% sure that this Map is never sused by anyone else? What 
> > about future code?

By future code, do you mean for someone who might use this in future?
Currently, I have added a note in CreateTableObjJson to warn that the Map could 
be filtered based on regexes provided.


> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 917 (patched)
> > <https://reviews.apache.org/r/68827/diff/1/?file=2091788#file2091788line917>
> >
> > make private

I thought this could be used separately as a utility method, if the user has 
just one predicate. Should I just keep the other method which accepts a list of 
predicates and make this one private?


- Bharathkrishna


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


On Sept. 28, 2018, 9:38 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 9:38 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f8129 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b05320 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated Sept. 28, 2018, 9:38 a.m.)


Review request for hive and Alexander Kolbasov.


Summary (updated)
-

HIVE-20545 : Exclude large-sized parameters from serialization of Table and 
Partition thrift objects in HMS notifications


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


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f8129 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b05320 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
 PRE-CREATION 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68828: HIVE-20601 : EnvironmentContext null in ALTER_PARTITION event in DbNotificationListener

[Bharathkrishna Guruvayoor Murali via Review Board]

> On Sept. 26, 2018, 7:24 p.m., Alexander Kolbasov wrote:
> > Are there any other cases where environment context isn't passed to the 
> > listener?

In other cases, it is being passed to listener


> On Sept. 26, 2018, 7:24 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
> > Line 746 (original), 746 (patched)
> > <https://reviews.apache.org/r/68828/diff/1/?file=2091792#file2091792line746>
> >
> > Looks like formatting is off here

I think formatting is correct here, no bugs according to checkstyle


- Bharathkrishna


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


On Sept. 24, 2018, 8:42 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68828/
> ---
> 
> (Updated Sept. 24, 2018, 8:42 p.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> It will be useful to have the environmentContext passed to 
> DbNotificationListener in this case, to know if the alter happened due to a 
> stat change.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
>  f52ff91a8f2e7710801dcadc4a83ce454992a66a 
> 
> 
> Diff: https://reviews.apache.org/r/68828/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Review Request 68828: HIVE-20601 : EnvironmentContext null in ALTER_PARTITION event in DbNotificationListener

[Bharathkrishna Guruvayoor Murali via Review Board]

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

Review request for hive and Alexander Kolbasov.


Repository: hive-git


Description
---

It will be useful to have the environmentContext passed to 
DbNotificationListener in this case, to know if the alter happened due to a 
stat change.


Diffs
-

  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
 f52ff91a8f2e7710801dcadc4a83ce454992a66a 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Review Request 68827: Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali via Review Board]

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

Review request for hive and Alexander Kolbasov.


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f8129 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b05320 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
 PRE-CREATION 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

[jira] [Created] (HIVE-20610) TestDbNotificationListener should not use /tmp directory

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-20610:
---

 Summary: TestDbNotificationListener should not use /tmp directory
 Key: HIVE-20610
 URL: https://issues.apache.org/jira/browse/HIVE-20610
 Project: Hive
  Issue Type: Bug
Affects Versions: 3.0.0, 4.0.0
Reporter: Bharathkrishna Guruvayoor Murali


Using /tmp directory creates exceptions for tests like dropTable :
{code:java}
2018-09-19T06:42:04,818  INFO [main] metastore.HiveMetaStore: 0: drop_table : 
tbl=hive.default.droptbl
2018-09-19T06:42:04,819  INFO [main] HiveMetaStore.audit: ugi=hiveptest 
ip=unknown-ip-addr  cmd=drop_table : tbl=hive.default.droptbl   
2018-09-19T06:42:05,072  WARN [main] fs.FileUtil: Failed to delete file or dir 
[/tmp/.ICE-unix]: it still exists.
2018-09-19T06:42:05,072  WARN [main] fs.FileUtil: Failed to delete file or dir 
[/tmp/.XIM-unix]: it still exists.
2018-09-19T06:42:05,072  WARN [main] fs.FileUtil: Failed to delete file or dir 
[/tmp/.X11-unix]: it still exists.
2018-09-19T06:42:05,072  WARN [main] fs.FileUtil: Failed to delete file or dir 
[/tmp/hsperfdata_root]: it still exists.
2018-09-19T06:42:05,072  WARN [main] fs.FileUtil: Failed to delete file or dir 
[/tmp/.font-unix]: it still exists.
2018-09-19T06:42:05,072  WARN [main] fs.FileUtil: Failed to delete file or dir 
[/tmp/.Test-unix]: it still exists.
2018-09-19T06:42:05,072 ERROR [main] utils.FileUtils: Failed to delete file:/tmp
2018-09-19T06:42:05,072 ERROR [main] utils.MetaStoreUtils: Got exception: 
org.apache.hadoop.hive.metastore.api.MetaException Unable to delete directory: 
file:/tmp
org.apache.hadoop.hive.metastore.api.MetaException: Unable to delete directory: 
file:/tmp
at 
org.apache.hadoop.hive.metastore.HiveMetaStoreFsImpl.deleteDir(HiveMetaStoreFsImpl.java:45)
 [hive-standalone-metastore-server-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.metastore.Warehouse.deleteDir(Warehouse.java:365) 
[hive-standalone-metastore-common-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.metastore.Warehouse.deleteDir(Warehouse.java:353) 
[hive-standalone-metastore-common-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.deleteTableData(HiveMetaStore.java:2562)
 [hive-standalone-metastore-server-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.drop_table_core(HiveMetaStore.java:2523)
 [hive-standalone-metastore-server-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.drop_table_with_environment_context(HiveMetaStore.java:2685)
 [hive-standalone-metastore-server-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) 
~[?:1.8.0_102]
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) 
~[?:1.8.0_102]
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 ~[?:1.8.0_102]
at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_102]
at 
org.apache.hadoop.hive.metastore.RetryingHMSHandler.invokeInternal(RetryingHMSHandler.java:147)
 [hive-standalone-metastore-server-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.metastore.RetryingHMSHandler.invoke(RetryingHMSHandler.java:108)
 [hive-standalone-metastore-server-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at com.sun.proxy.$Proxy33.drop_table_with_environment_context(Unknown 
Source) [?:?]
at 
org.apache.hadoop.hive.metastore.HiveMetaStoreClient.drop_table_with_environment_context(HiveMetaStoreClient.java:3204)
 [hive-standalone-metastore-common-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.metastore.HiveMetaStoreClient.dropTable(HiveMetaStoreClient.java:1492)
 [hive-standalone-metastore-common-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.metastore.HiveMetaStoreClient.dropTable(HiveMetaStoreClient.java:1432)
 [hive-standalone-metastore-common-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hive.hcatalog.listener.TestDbNotificationListener.dropTable(TestDbNotificationListener.java:522)
 [test-classes/:?]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) 
~[?:1.8.0_102]{code}
 

 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (HIVE-20601) EnvironmentContext null in ALTER_PARTITION event in DbNotificationListener

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-20601:
---

 Summary: EnvironmentContext null in ALTER_PARTITION event in 
DbNotificationListener
 Key: HIVE-20601
 URL: https://issues.apache.org/jira/browse/HIVE-20601
 Project: Hive
  Issue Type: Improvement
  Components: Metastore
Affects Versions: 3.0.0, 4.0.0
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali


Cause : EnvironmentContext not passed here:

[https://github.com/apache/hive/blob/36c33ca066c99dfdb21223a711c0c3f33c85b943/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java#L726]

 

It will be useful to have the environmentContext passed to 
DbNotificationListener in this case, to know if the alter happened due to a 
stat change.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (HIVE-20545) Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-20545:
---

 Summary: Exclude large-sized parameters from serialization of 
Table and Partition thrift objects in HMS notifications
 Key: HIVE-20545
 URL: https://issues.apache.org/jira/browse/HIVE-20545
 Project: Hive
  Issue Type: Improvement
Reporter: Bharathkrishna Guruvayoor Murali


Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 67815: HIVE-19733 : RemoteSparkJobStatus#getSparkStageProgress inefficient implementation

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated July 19, 2018, 7:21 p.m.)


Review request for hive, Peter Vary and Sahil Takiar.


Changes
---

I added separate exception messages for getSparkStagesInfo, I hope this would 
help distinguish exceptions from the getJobInfoJob call.


Repository: hive-git


Description
---

Adding GetSparkStagesInfoJob which gets the jobInfo and stageInfos in a single 
job


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java 
06d0ed3c18f80df84c7d06e695c722e20957b0eb 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/RemoteSparkJobStatus.java
 832832b325d7eb0e43fb34a4306d7ffa43ceaa78 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67815: HIVE-19733 : RemoteSparkJobStatus#getSparkStageProgress inefficient implementation

[Bharathkrishna Guruvayoor Murali via Review Board]

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




ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/RemoteSparkJobStatus.java
Line 194 (original), 197 (patched)
<https://reviews.apache.org/r/67815/#comment288719>

So should I make the exception messages generic to both the methods that 
can call getSparkJobInfo() ?


- Bharathkrishna Guruvayoor Murali


On July 5, 2018, 7:34 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67815/
> ---
> 
> (Updated July 5, 2018, 7:34 p.m.)
> 
> 
> Review request for hive, Peter Vary and Sahil Takiar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding GetSparkStagesInfoJob which gets the jobInfo and stageInfos in a 
> single job
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/RemoteSparkJobStatus.java
>  832832b325d7eb0e43fb34a4306d7ffa43ceaa78 
> 
> 
> Diff: https://reviews.apache.org/r/67815/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67815: HIVE-19733 : RemoteSparkJobStatus#getSparkStageProgress inefficient implementation

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated July 5, 2018, 7:34 p.m.)


Review request for hive, Peter Vary and Sahil Takiar.


Repository: hive-git


Description
---

Adding GetSparkStagesInfoJob which gets the jobInfo and stageInfos in a single 
job


Diffs (updated)
-

  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/RemoteSparkJobStatus.java
 832832b325d7eb0e43fb34a4306d7ffa43ceaa78 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Review Request 67815: HIVE-19733 : RemoteSparkJobStatus#getSparkStageProgress inefficient implementation

[Bharathkrishna Guruvayoor Murali via Review Board]

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

Review request for hive, Peter Vary and Sahil Takiar.


Repository: hive-git


Description
---

Adding GetSparkStagesInfoJob which gets the jobInfo and stageInfos in a single 
job


Diffs
-

  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/RemoteSparkJobStatus.java
 832832b325d7eb0e43fb34a4306d7ffa43ceaa78 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67636: HIVE-19176 : Add HoS support to progress bar on Beeline client.

[Bharathkrishna Guruvayoor Murali via Review Board]

> On June 21, 2018, 7:47 p.m., Sahil Takiar wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
> > Lines 96 (patched)
> > <https://reviews.apache.org/r/67636/diff/1-2/?file=2042207#file2042207line96>
> >
> > why does testOutput contain both formats?
> > 
> > also there is a unnecessary pair of "()" here

One is the progress bar and other one is the report string.
Please note that for the test, I am using :
  updateFunction = new RenderStrategy.InPlaceUpdateFunction(monitor);
  
So it will just print the progress bar and report string to the output stream.

Should I add any tests specifically from beeline side to test the progress bar?


> On June 21, 2018, 7:47 p.m., Sahil Takiar wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/67636/diff/2/?file=2044174#file2044174line108>
> >
> > why is this necessary?

The test TestSparkTask#testRemoteSparkCancel was failing, as at some point it 
calls SparkJobMonitor#updateFunction and SessionState.get().isHiveServerQuery() 
was getting NPE.


- Bharathkrishna


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


On June 22, 2018, 8:01 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> ---
> 
> (Updated June 22, 2018, 8:01 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This logic is similar to the RenderStrategy used in Tez to print the progress 
> bar.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> ad8d1a7f1cca3a763bb7c07335998ab7d39d7598 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java
>  2a6c33bfd4824c96e7004cd1ecce48c62c97d685 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
>  004b50ba95934280cf302055a46a5d984b421e07 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 3531ac25a9959aacd5766a9a42316890c68a1cd5 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 
> 368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
>  e66354f0869738bd3cf0eb831c13fa6af1eda256 
>   service/src/java/org/apache/hive/service/ServiceUtils.java 
> 226e43244df10c22143b91f92ef312e56739d036 
>   
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> 68fe8d8aa143fafbfc611253ce3a12065016a537 
> 
> 
> Diff: https://reviews.apache.org/r/67636/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67636: HIVE-19176 : Add HoS support to progress bar on Beeline client.

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated June 22, 2018, 8:01 p.m.)


Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.


Repository: hive-git


Description
---

This logic is similar to the RenderStrategy used in Tez to print the progress 
bar.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0 
  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
ad8d1a7f1cca3a763bb7c07335998ab7d39d7598 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java
 2a6c33bfd4824c96e7004cd1ecce48c62c97d685 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
 004b50ba95934280cf302055a46a5d984b421e07 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
3531ac25a9959aacd5766a9a42316890c68a1cd5 
  ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 
368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
 e66354f0869738bd3cf0eb831c13fa6af1eda256 
  service/src/java/org/apache/hive/service/ServiceUtils.java 
226e43244df10c22143b91f92ef312e56739d036 
  
service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java
 PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
68fe8d8aa143fafbfc611253ce3a12065016a537 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67636: HIVE-19176 : Add HoS support to progress bar on Beeline client.

[Bharathkrishna Guruvayoor Murali via Review Board]

> On June 19, 2018, 8:53 p.m., Sahil Takiar wrote:
> > is there a way to add some unit tests for this?

I added the progress bar to unit test.
Do you mean to write some test from beeline side, or is this good enough?


- Bharathkrishna


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


On June 21, 2018, 3:50 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> ---
> 
> (Updated June 21, 2018, 3:50 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This logic is similar to the RenderStrategy used in Tez to print the progress 
> bar.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> ad8d1a7f1cca3a763bb7c07335998ab7d39d7598 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java
>  2a6c33bfd4824c96e7004cd1ecce48c62c97d685 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
>  004b50ba95934280cf302055a46a5d984b421e07 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 3531ac25a9959aacd5766a9a42316890c68a1cd5 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 
> 368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
>  e66354f0869738bd3cf0eb831c13fa6af1eda256 
>   service/src/java/org/apache/hive/service/ServiceUtils.java 
> 226e43244df10c22143b91f92ef312e56739d036 
>   
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> 68fe8d8aa143fafbfc611253ce3a12065016a537 
> 
> 
> Diff: https://reviews.apache.org/r/67636/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67636: HIVE-19176 : Add HoS support to progress bar on Beeline client.

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated June 21, 2018, 3:50 p.m.)


Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.


Changes
---

Fixed failing test in TestSparkTask.
Added progress bar to the unit test.
Added javadocs.


Repository: hive-git


Description
---

This logic is similar to the RenderStrategy used in Tez to print the progress 
bar.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0 
  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
ad8d1a7f1cca3a763bb7c07335998ab7d39d7598 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java
 2a6c33bfd4824c96e7004cd1ecce48c62c97d685 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
 004b50ba95934280cf302055a46a5d984b421e07 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
3531ac25a9959aacd5766a9a42316890c68a1cd5 
  ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 
368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
 e66354f0869738bd3cf0eb831c13fa6af1eda256 
  service/src/java/org/apache/hive/service/ServiceUtils.java 
226e43244df10c22143b91f92ef312e56739d036 
  
service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java
 PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
68fe8d8aa143fafbfc611253ce3a12065016a537 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67636: HIVE-19176 : Add HoS support to progress bar on Beeline client.

[Bharathkrishna Guruvayoor Murali via Review Board]

> On June 19, 2018, 8:53 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
> > Lines 43 (patched)
> > <https://reviews.apache.org/r/67636/diff/1/?file=2042205#file2042205line43>
> >
> > how much of this code is new coded vs. copied from other classes?

The methods getReport, isSameAsPreviousProgress, printStatus are copied from 
existing.


- Bharathkrishna


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


On June 18, 2018, 8:41 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> ---
> 
> (Updated June 18, 2018, 8:41 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This logic is similar to the RenderStrategy used in Tez to print the progress 
> bar.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 933bda4ad01a6f7878019a7b4c971a0c39068ae2 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> ad8d1a7f1cca3a763bb7c07335998ab7d39d7598 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java
>  2a6c33bfd4824c96e7004cd1ecce48c62c97d685 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
>  004b50ba95934280cf302055a46a5d984b421e07 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 3531ac25a9959aacd5766a9a42316890c68a1cd5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
>  e66354f0869738bd3cf0eb831c13fa6af1eda256 
>   service/src/java/org/apache/hive/service/ServiceUtils.java 
> 226e43244df10c22143b91f92ef312e56739d036 
>   
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> 68fe8d8aa143fafbfc611253ce3a12065016a537 
> 
> 
> Diff: https://reviews.apache.org/r/67636/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67636: HIVE-19176 : Add HoS support to progress bar on Beeline client.

[Bharathkrishna Guruvayoor Murali via Review Board]

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




ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
Lines 43 (patched)
<https://reviews.apache.org/r/67636/#comment287903>

The methods getReport, isSameAsPreviousProgress, printStatus are copied 
from existing.


- Bharathkrishna Guruvayoor Murali


On June 18, 2018, 8:41 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> ---
> 
> (Updated June 18, 2018, 8:41 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This logic is similar to the RenderStrategy used in Tez to print the progress 
> bar.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 933bda4ad01a6f7878019a7b4c971a0c39068ae2 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> ad8d1a7f1cca3a763bb7c07335998ab7d39d7598 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java
>  2a6c33bfd4824c96e7004cd1ecce48c62c97d685 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
>  004b50ba95934280cf302055a46a5d984b421e07 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 3531ac25a9959aacd5766a9a42316890c68a1cd5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
>  e66354f0869738bd3cf0eb831c13fa6af1eda256 
>   service/src/java/org/apache/hive/service/ServiceUtils.java 
> 226e43244df10c22143b91f92ef312e56739d036 
>   
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> 68fe8d8aa143fafbfc611253ce3a12065016a537 
> 
> 
> Diff: https://reviews.apache.org/r/67636/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

[jira] [Created] (HIVE-19942) Hive Notification: All events for indexes should have table name

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-19942:
---

 Summary: Hive Notification: All events for indexes should have 
table name
 Key: HIVE-19942
 URL: https://issues.apache.org/jira/browse/HIVE-19942
 Project: Hive
  Issue Type: Improvement
  Components: Hive
Affects Versions: 2.3.2
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali


All the events for indexes: Create Index, Alter Index, Drop Index have the 
TBL_NAME as null. The TBL_NAME should be populated with the table on which the 
index is created.

This makes it easier to decide whether to process the event or not without 
needing to parse the json message (which is a slower process).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 67636: HIVE-19176 : Add HoS support to progress bar on Beeline client.

[Bharathkrishna Guruvayoor Murali via Review Board]

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




jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Lines 379 (patched)
<https://reviews.apache.org/r/67636/#comment287785>

Added this condition because in the do-while loop, when this executes after 
the results are printed, the results get overwritten by progress bar. So if 
operation is complete, no need to update.


- Bharathkrishna Guruvayoor Murali


On June 18, 2018, 8:41 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> ---
> 
> (Updated June 18, 2018, 8:41 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This logic is similar to the RenderStrategy used in Tez to print the progress 
> bar.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 933bda4ad01a6f7878019a7b4c971a0c39068ae2 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
> ad8d1a7f1cca3a763bb7c07335998ab7d39d7598 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java
>  2a6c33bfd4824c96e7004cd1ecce48c62c97d685 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
>  004b50ba95934280cf302055a46a5d984b421e07 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 3531ac25a9959aacd5766a9a42316890c68a1cd5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
>  e66354f0869738bd3cf0eb831c13fa6af1eda256 
>   service/src/java/org/apache/hive/service/ServiceUtils.java 
> 226e43244df10c22143b91f92ef312e56739d036 
>   
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> 68fe8d8aa143fafbfc611253ce3a12065016a537 
> 
> 
> Diff: https://reviews.apache.org/r/67636/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Review Request 67636: HIVE-19176 : Add HoS support to progress bar on Beeline client.

[Bharathkrishna Guruvayoor Murali via Review Board]

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

Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.


Repository: hive-git


Description
---

This logic is similar to the RenderStrategy used in Tez to print the progress 
bar.


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
933bda4ad01a6f7878019a7b4c971a0c39068ae2 
  jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 
ad8d1a7f1cca3a763bb7c07335998ab7d39d7598 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java
 2a6c33bfd4824c96e7004cd1ecce48c62c97d685 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
 004b50ba95934280cf302055a46a5d984b421e07 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
3531ac25a9959aacd5766a9a42316890c68a1cd5 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
 e66354f0869738bd3cf0eb831c13fa6af1eda256 
  service/src/java/org/apache/hive/service/ServiceUtils.java 
226e43244df10c22143b91f92ef312e56739d036 
  
service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java
 PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
68fe8d8aa143fafbfc611253ce3a12065016a537 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

[jira] [Created] (HIVE-19925) NPE in SparkTask#printConsoleMetrics

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-19925:
---

 Summary: NPE in SparkTask#printConsoleMetrics
 Key: HIVE-19925
 URL: https://issues.apache.org/jira/browse/HIVE-19925
 Project: Hive
  Issue Type: Improvement
Reporter: Bharathkrishna Guruvayoor Murali


When running a join query with HOS, as :
{code:java}
SELECT a.id FROM sample a JOIN sample b ON (a.id=b.id);{code}
Got the following exception :


{code:java}
Error while processing statement: FAILED: Execution Error, return code 1 from 
org.apache.hadoop.hive.ql.exec.spark.SparkTask. java.lang.NullPointerException
at 
org.apache.hadoop.hive.ql.exec.spark.SparkTask.printConsoleMetrics(SparkTask.java:229)
at org.apache.hadoop.hive.ql.exec.spark.SparkTask.execute(SparkTask.java:166)
at org.apache.hadoop.hive.ql.exec.Task.executeTask(Task.java:205)
at org.apache.hadoop.hive.ql.exec.TaskRunner.runSequential(TaskRunner.java:97)
at org.apache.hadoop.hive.ql.Driver.launchTask(Driver.java:2678)
at org.apache.hadoop.hive.ql.Driver.execute(Driver.java:2330)
at org.apache.hadoop.hive.ql.Driver.runInternal(Driver.java:2001)
at org.apache.hadoop.hive.ql.Driver.run(Driver.java:1701)
at org.apache.hadoop.hive.ql.Driver.run(Driver.java:1695)
at org.apache.hadoop.hive.ql.reexec.ReExecDriver.run(ReExecDriver.java:157)
at 
org.apache.hive.service.cli.operation.SQLOperation.runQuery(SQLOperation.java:224)
at 
org.apache.hive.service.cli.operation.SQLOperation.access$600(SQLOperation.java:87)
at 
org.apache.hive.service.cli.operation.SQLOperation$BackgroundWork$1.run(SQLOperation.java:315)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
at 
org.apache.hive.service.cli.operation.SQLOperation$BackgroundWork.run(SQLOperation.java:328)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748) (state=08S01,code=1)
{code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated June 7, 2018, 10:43 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
---

Making changes to be in sync with HIVE-19508 so that it does not cause merge 
conflicts with master


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


Repository: hive-git


Description
---

Refactor inplace progress code in Hive-on-spark progress monitor to use 
ProgressMonitor instance


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
e78b1cd6637c46070378c25a372916817fe99a59 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
 PRE-CREATION 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated June 4, 2018, 6:34 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
---

Correcting issues reported by checkstyle and findbugs checks.


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


Repository: hive-git


Description
---

Refactor inplace progress code in Hive-on-spark progress monitor to use 
ProgressMonitor instance


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
7afd8864075aa0d9708274eea8839c662324c732 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/67263/diff/4/

Changes: https://reviews.apache.org/r/67263/diff/3-4/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

[jira] [Created] (HIVE-19766) Show the number of rows inserted when execution engine is Spark

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-19766:
---

 Summary: Show the number of rows inserted when execution engine is 
Spark
 Key: HIVE-19766
 URL: https://issues.apache.org/jira/browse/HIVE-19766
 Project: Hive
  Issue Type: Improvement
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali


Currently when insert query is run, the beeline output shows No rows affected.

The logic to show the number of rows inserted is now present when execution 
engine is MR.

This Jira is to make this work with Spark.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Review Request 67415: HIVE-19525 : Spark task logs print PLAN PATH excessive number of times

[Bharathkrishna Guruvayoor Murali via Review Board]

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

Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Changing path log level to debug.
The code to check if base work is already present in the map is placed before 
the logic to get kryo object and classloader.


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 
406bea011da83aee55f385029a4df1af94400e4c 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 304 (original), 231 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line306>
> >
> >     whats the point of this class?
> 
> Bharathkrishna Guruvayoor Murali wrote:
> I used this class to follow same pattern as in tez. I will add the logic 
> similar to RenderStrategy used in tez while adding beeline progress bar, so 
> this should be useful.
> 
> Sahil Takiar wrote:
> Ok, can this be a private class? Is it used outside `SparkJobMonitor`?
> 
> Bharathkrishna Guruvayoor Murali wrote:
> Not used outside SparkJobMonitor. Making private

I actually removed this class, because I was using the static class in a 
"non-static" kind of way. Just donot want to have code with wrong pattern even 
though it will bee changed soon.
I will add it when it is actually required, ie. later when I need to introduce 
progress bar in beeline.


- Bharathkrishna


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


On June 1, 2018, 4:58 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> ---
> 
> (Updated June 1, 2018, 4:58 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
> https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use 
> ProgressMonitor instance
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 7afd8864075aa0d9708274eea8839c662324c732 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated June 1, 2018, 4:58 a.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Refactor inplace progress code in Hive-on-spark progress monitor to use 
ProgressMonitor instance


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
7afd8864075aa0d9708274eea8839c662324c732 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
 PRE-CREATION 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 70 (original), 67 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line72>
> >
> > is this still used?
> 
> Bharathkrishna Guruvayoor Murali wrote:
> Will remove this if we do not need to pass the headers , footers etc.. to 
> ProgressMonitor.
> (ie. if the progress bar format shown in below comment is acceptable).
> 
> Sahil Takiar wrote:
> So what changes about the progress bar format if you do pass the headers 
> and footers?

Nothing changes, except the bit of dashed lines at the end (), which we are 
considering fine for now as mentioned in below comment.
So removing this variable definition.


> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 304 (original), 231 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line306>
> >
> > whats the point of this class?
> 
> Bharathkrishna Guruvayoor Murali wrote:
> I used this class to follow same pattern as in tez. I will add the logic 
> similar to RenderStrategy used in tez while adding beeline progress bar, so 
> this should be useful.
> 
> Sahil Takiar wrote:
> Ok, can this be a private class? Is it used outside `SparkJobMonitor`?

Not used outside SparkJobMonitor. Making private


- Bharathkrishna


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


On May 29, 2018, 10:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> ---
> 
> (Updated May 29, 2018, 10:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
> https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use 
> ProgressMonitor instance
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 7afd8864075aa0d9708274eea8839c662324c732 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated May 29, 2018, 10:53 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
---

Making changes wrt review comments


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


Repository: hive-git


Description
---

Refactor inplace progress code in Hive-on-spark progress monitor to use 
ProgressMonitor instance


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
7afd8864075aa0d9708274eea8839c662324c732 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
 PRE-CREATION 


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

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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > are there any logic changes, or is most of the code just copied into the 
> > new class?

Most of the code is copied to new class. Mostly I have the doubts mentioned in 
above comments.


- Bharathkrishna


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


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> ---
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
> https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use 
> ProgressMonitor instance
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 7afd8864075aa0d9708274eea8839c662324c732 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 70 (original), 67 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line72>
> >
> > is this still used?

Will remove this if we do not need to pass the headers , footers etc.. to 
ProgressMonitor.
(ie. if the progress bar format shown in below comment is acceptable).


> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 304 (original), 231 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line306>
> >
> > whats the point of this class?

I used this class to follow same pattern as in tez. I will add the logic 
similar to RenderStrategy used in tez while adding beeline progress bar, so 
this should be useful.


> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
> > Lines 27 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027541#file2027541line27>
> >
> > whats the impact of having an extra argument? does the formatting 
> > change at all?

It changes a bit,like this:

--
  STAGES   ATTEMPTSTATUS  TOTAL  COMPLETED  RUNNING  PENDING  
FAILED
--
Stage-1  0  FINISHED  1  100
   0
--
STAGES: 01/01[==>>] 100%  ELAPSED TIME: 1.01 s
--

Notice the bit of extra space at the end. But other than that, it looks pretty 
much same.


- Bharathkrishna


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


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> ---
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
> https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use 
> ProgressMonitor instance
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 7afd8864075aa0d9708274eea8839c662324c732 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

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




ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 27 (patched)
<https://reviews.apache.org/r/67263/#comment285929>

Using an empty String as last argument, because the standard InplaceUpdate 
header expects one more argument. Same in rows() method too.

Should I instead pass the headerFormat and rowFormat also as a parameter, 
by adding methods in ProgressMonitor interface? Then I will need to update Tez 
code, thrift code to have response in this form. Can avoid that if this is 
acceptable.



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 77 (patched)
<https://reviews.apache.org/r/67263/#comment285930>

These states are not used as of now, but I guess it will be needed when we 
add progress bar in beeline.
(with states similar to TezProgressMonitorStatusMapper).
Are these executionStatuses good enough.



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 102 (patched)
<https://reviews.apache.org/r/67263/#comment285928>

Using just an int counter here, instead of adding completed stages to a 
HashSet as done in existing implementation, because we just need the count.


- Bharathkrishna Guruvayoor Murali


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> ---
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
> https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use 
> ProgressMonitor instance
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
> 7afd8864075aa0d9708274eea8839c662324c732 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Review Request 67263: HIVE-19602

[Bharathkrishna Guruvayoor Murali via Review Board]

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

Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Refactor inplace progress code in Hive-on-spark progress monitor to use 
ProgressMonitor instance


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 
7afd8864075aa0d9708274eea8839c662324c732 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
 PRE-CREATION 


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


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

[jira] [Created] (HIVE-19602) Refactor inplace progress code in Hive-on-spark progress monitor to use InplaceUpdate

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-19602:
---

 Summary: Refactor inplace progress code in Hive-on-spark progress 
monitor to use InplaceUpdate
 Key: HIVE-19602
 URL: https://issues.apache.org/jira/browse/HIVE-19602
 Project: Hive
  Issue Type: Bug
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali


We can refactor the HOS inplace progress monitor code to use InplaceUpdate.

This would be similar to :

[https://github.com/apache/hive/blob/0b6bea89f74b607299ad944b37e4b62c711aaa69/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/RenderStrategy.java#L181]



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated May 16, 2018, 5:48 p.m.)


Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.


Changes
---

Updated the q file and q.out file.
Changes to exception handling logic.


Repository: hive-git


Description
---

Adding support to retain the time part (HH:mm:ss) for add_months UDF when the 
input is given as timestamp format.


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
dae4b97b4a17e98122431e5fda655fd9f873fdb5 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java 
af9b6c43c7dafc69c4944eab02894786af306f35 
  ql/src/test/queries/clientpositive/udf_add_months.q 
0b8b444fd1657117dec18c2b4e7173767617 
  ql/src/test/results/clientpositive/udf_add_months.q.out 
5ba720ae85d30c0da7f94f377d6b324bce850907 


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

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


Testing
---

Added unit tests.


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 16, 2018, 11:32 a.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java
> > Lines 85 (patched)
> > <https://reviews.apache.org/r/67073/diff/2/?file=2023748#file2023748line85>
> >
> > I would prefer to thow an exception here instead of working with the 
> > default version. Better for the user to see the error faster, and not only 
> > after a long query run, when the output is not formatted as expected. What 
> > do you think?

Sounds good.
I did it that way because I thought we can use default format if user provides 
invalid value, but it makes sense as the user anyways wants to provide a 
different format and we can let them know earlier if it is invalid.
Removed exception catch so that it will be thrown if format is invalid.


> On May 16, 2018, 11:32 a.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java
> > Lines 143 (patched)
> > <https://reviews.apache.org/r/67073/diff/2/?file=2023748#file2023748line143>
> >
> > Why is this null check needed?

Just added it as it was present in UDFDateFormat.
But I think we won't have a null or invalid formatter or null Date in the line
String res = formatter.format(newDate);

So removing the null check


- Bharathkrishna


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


On May 16, 2018, 5:48 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67073/
> ---
> 
> (Updated May 16, 2018, 5:48 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding support to retain the time part (HH:mm:ss) for add_months UDF when the 
> input is given as timestamp format.
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
> dae4b97b4a17e98122431e5fda655fd9f873fdb5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
>  af9b6c43c7dafc69c4944eab02894786af306f35 
>   ql/src/test/queries/clientpositive/udf_add_months.q 
> 0b8b444fd1657117dec18c2b4e7173767617 
>   ql/src/test/results/clientpositive/udf_add_months.q.out 
> 5ba720ae85d30c0da7f94f377d6b324bce850907 
> 
> 
> Diff: https://reviews.apache.org/r/67073/diff/3/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive

[Bharathkrishna Guruvayoor Murali via Review Board]

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

(Updated May 15, 2018, 11:18 p.m.)


Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.


Changes
---

Changing the patch with new logic.
Adding an additional optional parameter dateFormat, which can provide output 
format that user expects.
The default format is kept as the existing one, -MM-dd. 
Hence, the patch is backward compatible, and also capable of handling different 
input and output formats as required by the user.

Also, now the code format is consistent with the other similar UDFs 
MonthsBetween and GenericUDFDateFormat.

Added unit tests to check various cases.

Thanks Peter for helping me reach a conclusion on what logic I need to use.

I have updated the UDF description. Will update the docs once the patch is 
approved.


Repository: hive-git


Description
---

Adding support to retain the time part (HH:mm:ss) for add_months UDF when the 
input is given as timestamp format.


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
dae4b97b4a17e98122431e5fda655fd9f873fdb5 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java 
af9b6c43c7dafc69c4944eab02894786af306f35 


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

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


Testing
---

Added unit tests.


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 11, 2018, 8:51 p.m., Bharathkrishna Guruvayoor Murali wrote:
> > Should I handle String inputs like 2018-05-10 12:20:10 to retain the time 
> > part.
> > I did not do this because the inputs can have several formats if it is 
> > String (whereas timestamp is only in one format and it was easier to 
> > handle).
> > 
> > Should I add an extra parameter in the UDF that accepts an optional 
> > DateFormat string, where user can pass the format they want the output to 
> > be in. If the parser fails, I can fallback to the default format.
> > What are your thoughts on this.
> 
> Peter Vary wrote:
> I think using the approach we see in GenericUDFDateFormat we will have 
> consistent behaviour across different UDF-s.
> What do you think?
> 
> Bharathkrishna Guruvayoor Murali wrote:
> So the problem that I see is that, String format can have different types 
> of strings, like -MM-dd HH:mm:ss or -MM-dd HH:mm:ss.fff, so it is 
> difficult to provide the output in the same format as input.
> Also, String format can have just -MM-dd (without time), in this 
> case, we should be having logic to not have time in output, otherwise it will 
> be of form -MM-dd 00:00:00
> 
> The GenericUDFDateFormat uses a dateformat value, which helps convert the 
> String to given format, so there are no ambiguities about output format.
> That is why I wanted to add extra parameter in the UDF for output 
> DateFormat.

Or with the current patch, if the user wants to retain time part for String 
values, they can do:
 select add_months(select cast(date_format('1997-02-28 10:30:00','-MM-dd 
HH:mm:ss') as timestamp),4) ;
 
This would cast to timestamp and always return standard output format.


- Bharathkrishna


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


On May 10, 2018, 9:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67073/
> ---
> 
> (Updated May 10, 2018, 9:55 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding support to retain the time part (HH:mm:ss) for add_months UDF when the 
> input is given as timestamp format.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> 65f3b9401916abdfa52fbf75d115ba6b61758fb0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
> dae4b97b4a17e98122431e5fda655fd9f873fdb5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
>  af9b6c43c7dafc69c4944eab02894786af306f35 
> 
> 
> Diff: https://reviews.apache.org/r/67073/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 11, 2018, 8:51 p.m., Bharathkrishna Guruvayoor Murali wrote:
> > Should I handle String inputs like 2018-05-10 12:20:10 to retain the time 
> > part.
> > I did not do this because the inputs can have several formats if it is 
> > String (whereas timestamp is only in one format and it was easier to 
> > handle).
> > 
> > Should I add an extra parameter in the UDF that accepts an optional 
> > DateFormat string, where user can pass the format they want the output to 
> > be in. If the parser fails, I can fallback to the default format.
> > What are your thoughts on this.
> 
> Peter Vary wrote:
> I think using the approach we see in GenericUDFDateFormat we will have 
> consistent behaviour across different UDF-s.
> What do you think?

So the problem that I see is that, String format can have different types of 
strings, like -MM-dd HH:mm:ss or -MM-dd HH:mm:ss.fff, so it is 
difficult to provide the output in the same format as input.
Also, String format can have just -MM-dd (without time), in this case, we 
should be having logic to not have time in output, otherwise it will be of form 
-MM-dd 00:00:00

The GenericUDFDateFormat uses a dateformat value, which helps convert the 
String to given format, so there are no ambiguities about output format.
That is why I wanted to add extra parameter in the UDF for output DateFormat.


- Bharathkrishna


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


On May 10, 2018, 9:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67073/
> ---
> 
> (Updated May 10, 2018, 9:55 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding support to retain the time part (HH:mm:ss) for add_months UDF when the 
> input is given as timestamp format.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> 65f3b9401916abdfa52fbf75d115ba6b61758fb0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
> dae4b97b4a17e98122431e5fda655fd9f873fdb5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
>  af9b6c43c7dafc69c4944eab02894786af306f35 
> 
> 
> Diff: https://reviews.apache.org/r/67073/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

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

[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

[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 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive

[Bharathkrishna Guruvayoor Murali via Review Board]

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



Should I handle String inputs like 2018-05-10 12:20:10 to retain the time part.
I did not do this because the inputs can have several formats if it is String 
(whereas timestamp is only in one format and it was easier to handle).

Should I add an extra parameter in the UDF that accepts an optional DateFormat 
string, where user can pass the format they want the output to be in. If the 
parser fails, I can fallback to the default format.
What are your thoughts on this.

- Bharathkrishna Guruvayoor Murali


On May 10, 2018, 9:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67073/
> ---
> 
> (Updated May 10, 2018, 9:55 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding support to retain the time part (HH:mm:ss) for add_months UDF when the 
> input is given as timestamp format.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> 65f3b9401916abdfa52fbf75d115ba6b61758fb0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
> dae4b97b4a17e98122431e5fda655fd9f873fdb5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
>  af9b6c43c7dafc69c4944eab02894786af306f35 
> 
> 
> Diff: https://reviews.apache.org/r/67073/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 11, 2018, 10:08 a.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java
> > Lines 74 (patched)
> > <https://reviews.apache.org/r/67073/diff/1/?file=2019552#file2019552line74>
> >
> > Do we think this is a better approach then the one used by 
> > GenericUDFMonthsBetween, and GenericUDFDateFormat? Are we sure that we know 
> > the exact primitive categories we can convert? My guess that we originally 
> > accepted string as an input... What happens then?

Do you mean that we need to retain the time part also in the case when input 
type is String?
My patch would only retain time part if input type is timestamp.


- Bharathkrishna


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


On May 10, 2018, 9:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67073/
> ---
> 
> (Updated May 10, 2018, 9:55 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding support to retain the time part (HH:mm:ss) for add_months UDF when the 
> input is given as timestamp format.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> 65f3b9401916abdfa52fbf75d115ba6b61758fb0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
> dae4b97b4a17e98122431e5fda655fd9f873fdb5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
>  af9b6c43c7dafc69c4944eab02894786af306f35 
> 
> 
> Diff: https://reviews.apache.org/r/67073/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive

[Bharathkrishna Guruvayoor Murali via Review Board]

> On May 11, 2018, 3:22 p.m., Sahil Takiar wrote:
> > common/src/java/org/apache/hive/common/util/DateUtils.java
> > Lines 39 (patched)
> > <https://reviews.apache.org/r/67073/diff/1/?file=2019551#file2019551line39>
> >
> > why does this need to be `ThreadLocal`? there doesn't seem to be 
> > anything thread specific to this object

The class DateUtils is mentioned to be thread-safe class (in javadoc comment). 
and the existing date format is created as a threadlocal.
So I thought this one too should be a threadlocal to keep it thread-safe.


- Bharathkrishna


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


On May 10, 2018, 9:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67073/
> ---
> 
> (Updated May 10, 2018, 9:55 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding support to retain the time part (HH:mm:ss) for add_months UDF when the 
> input is given as timestamp format.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> 65f3b9401916abdfa52fbf75d115ba6b61758fb0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
> dae4b97b4a17e98122431e5fda655fd9f873fdb5 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
>  af9b6c43c7dafc69c4944eab02894786af306f35 
> 
> 
> Diff: https://reviews.apache.org/r/67073/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

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

[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

[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

[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)
> > <https://reviews.apache.org/r/66290/diff/5-6/?file=2014506#file2014506line437>
> >
> > 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

[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

[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

[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

[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)
> > <https://reviews.apache.org/r/66290/diff/1/?file=1988690#file1988690line712>
> >
> > 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/servic

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

[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

[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

[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

[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)
> > <https://reviews.apache.org/r/66290/diff/1/?file=1988690#file1988690line712>
> >
> > 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

[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)
> > <https://reviews.apache.org/r/66290/diff/2/?file=2005792#file2005792line711>
> >
> > 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)
> > <https://reviews.apache.org/r/66290/diff/2/?file=2005797#file2005797line431>
> >
> > 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 
> 52cc3a

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

[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

[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)
> > <https://reviews.apache.org/r/66290/diff/1/?file=1988690#file1988690line712>
> >
> > 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)
> > <https://reviews.apache.org/r/66290/diff/1/?file=1988691#file1988691line2213>
> >
> > 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)
> > <https://reviews.apache.org/r/66290/diff/1/?file=1988706#file1988706line186>
> >
> > 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 
>

[jira] [Created] (HIVE-19133) HS2 WebUI phase-wise performance metrics not showing correctly

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-19133:
---

 Summary: HS2 WebUI phase-wise performance metrics not showing 
correctly
 Key: HIVE-19133
 URL: https://issues.apache.org/jira/browse/HIVE-19133
 Project: Hive
  Issue Type: Bug
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali
 Attachments: WebUI-compile time query metrics.png

The query specific WebUI metrics (go to drilldown -> performance logging) are 
not showing up in the correct phase and are often mixed up.
Attaching screenshot.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Need edit permission to Hive Confluence page

[Bharathkrishna Guruvayoor Murali]

Hi Lefty,

Thanks for pointing me to the wiki.
My username is: bharos92


On Thu, Apr 5, 2018 at 8:55 PM, Lefty Leverenz <leftylever...@gmail.com>
wrote:

> Bharath, first you need to get a Confluence username as described here:
>
> About This Wiki -- How to get permission to edit
> <https://cwiki.apache.org/confluence/display/Hive/AboutThisWiki#AboutThisWiki-Howtogetpermissiontoedit>
>
>
> (It's okay to use the dev@hive list instead of user@hive, just tell us
> your username in this thread.)
>
> -- Lefty
>
>
> On Thu, Apr 5, 2018 at 2:57 PM Bharathkrishna Guruvayoor Murali <
> bhar...@cloudera.com> wrote:
>
>> Hi,
>>
>> I need edit permission to Confluence page.
>> Currently I need to make a doc update related to HIVE-16944.
>>
>> I would like to update SchemaTool wiki page
>> <https://cwiki.apache.org/confluence/display/Hive/Hive+Schema+Tool> to
>> include the parameters driver and URL which are currently missing in the
>> documentation.
>>
>> Someone who has the rights please add me.
>>
>> Thanks,
>> Bharath
>>
>

Need edit permission to Hive Confluence page

[Bharathkrishna Guruvayoor Murali]

Hi,

I need edit permission to Confluence page.
Currently I need to make a doc update related to HIVE-16944.

I would like to update SchemaTool wiki page
 to
include the parameters driver and URL which are currently missing in the
documentation.

Someone who has the rights please add me.

Thanks,
Bharath

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

[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

[jira] [Created] (HIVE-18981) Hive beeline ignores throwable

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-18981:
---

 Summary: Hive beeline ignores throwable
 Key: HIVE-18981
 URL: https://issues.apache.org/jira/browse/HIVE-18981
 Project: Hive
  Issue Type: Bug
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali


Beeline ignores the exception in catch block here:

[https://github.com/apache/hive/blob/f40d94476718df6de42caef3b18811f024fe8713/beeline/src/java/org/apache/hive/beeline/BeeLine.java#L1017

]

here:

https://github.com/apache/hive/blob/f40d94476718df6de42caef3b18811f024fe8713/beeline/src/java/org/apache/hive/beeline/BeeLine.java#L1055



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (HIVE-18972) beeline command suggestion to kill job deprecated

[Bharathkrishna Guruvayoor Murali (JIRA)]

Bharathkrishna Guruvayoor Murali created HIVE-18972:
---

 Summary: beeline command suggestion to kill job deprecated
 Key: HIVE-18972
 URL: https://issues.apache.org/jira/browse/HIVE-18972
 Project: Hive
  Issue Type: Bug
Reporter: Bharathkrishna Guruvayoor Murali
Assignee: Bharathkrishna Guruvayoor Murali


When I run a beeline command that uses YARN:
{code}
INFO : The url to track the job: 
http://vd0514.halxg.cloudera.com:8088/proxy/application_1488996234407_0010/
INFO : Starting Job = job_1488996234407_0010, Tracking URL = 
http://vd0514.halxg.cloudera.com:8088/proxy/application_1488996234407_0010/
INFO : Kill Command = 
/opt/cloudera/parcels/CDH-5.11.0-1.cdh5.11.0.p0.9/lib/hadoop/bin/hadoop job 
-kill job_1488996234407_0010
{code}
If I then try to kill the job using that command:
{code}
[systest@vd0514 ~]$ 
/opt/cloudera/parcels/CDH-5.11.0-1.cdh5.11.0.p0.9/lib/hadoop/bin/hadoop job 
-kill job_1488996234407_0010
DEPRECATED: Use of this script to execute mapred command is deprecated.
Instead use the mapred command for it.
{code}
 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)