Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance

[Sergio Pena via Review Board]

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




standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 4549-4555 (original), 4560-4569 (patched)
<https://reviews.apache.org/r/69642/#comment297044>

Isn't simpler to have a constructor that accepts the (catName, dbName, 
tblName, this) instead of using a Supplier? It will reduce code and make the 
code more readable.


- Sergio Pena


On Jan. 3, 2019, 1:40 a.m., Karthik Manamcheri wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> ---
> 
> (Updated Jan. 3, 2019, 1:40 a.m.)
> 
> 
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen 
> Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve 
> get_partition performance
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  a9398ae1e7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java
>  beec72bc12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
>  7429d18226 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
>  fe64a91b56 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java
>  4d7f7c1220 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
>  a338bd4032 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69642/diff/4/
> 
> 
> Testing
> ---
> 
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup 
> performance.
> 
> 
> Thanks,
> 
> Karthik Manamcheri
> 
>

Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

[Sergio Pena via Review Board]

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




standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
Lines 1837 (patched)
<https://reviews.apache.org/r/69585/#comment296625>

Is this the only place where is called? Btw, the patch is meant fo the 
server-side only, we could enhance the client-side another patch to keep this 
simpler.



standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
Lines 220-221 (patched)
<https://reviews.apache.org/r/69585/#comment296623>

These two variables are not necessary. To disable the filter, the admin can 
just remove the filter hooks from the configuration and voilà, filter disabled. 
This applies to both sides, server-side and client-side.



standalone-metastore/metastore-server/pom.xml
Lines 242-246 (patched)
<https://reviews.apache.org/r/69585/#comment296624>

Why is this dep necessary? I wonder if this conflicts with the standalone 
solution where all the Hive dependences were removed.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 1405 (patched)
<https://reviews.apache.org/r/69585/#comment296626>

I thought we were going to use the PreReadEvent instead. Also, I'm 
concerned the implementation of filterDatabase does not throw 
NoSuchObjectException, and instead returns null. We should check both cases to 
guarantee get_database and others methods will deny the access to this database.

I think we should use the PreReadEvent which is meant for authorization 
hooks.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 2934 (patched)
<https://reviews.apache.org/r/69585/#comment296627>

What is this extra space?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 2910-2911 (original), 2943-2946 (patched)
<https://reviews.apache.org/r/69585/#comment296628>

Why is this change needed? I don't see anything new except splitting it in 
two lines. This will stay in the git history.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Line 2931 (original), 2966-2970 (patched)
<https://reviews.apache.org/r/69585/#comment296629>

Notice the firePreEvent before the filter. If we use that for authorization 
checks, then the filter is not required here.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 3050-3053 (patched)
<https://reviews.apache.org/r/69585/#comment296630>

Same question, why splitting the lines where if this patch doesn't need it?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 3016-3017 (original), 3059-3062 (patched)
<https://reviews.apache.org/r/69585/#comment296631>

Same question, why splitting the lines where if this patch doesn't need it?


- Sergio Pena


On Dec. 19, 2018, 10:50 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 19, 2018, 10:50 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   standalone-metastore/metastore-server/pom.xml 
> 895abfc423f00b121ee63e40904f5b3e57aea8ed 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/3/
&g

Re: Review Request 67186: HIVE-19585: Add UNKNOWN to PrincipalType

[Sergio Pena via Review Board]

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




standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PrincipalType.java
Lines 18 (patched)
<https://reviews.apache.org/r/67186/#comment285539>

This file is auto-generated by Thrift. To add a new field, you need to edit 
the hive_metastore.thrift file and generate the new thrift files.

Btw, this might add a behavior to all the authorization commands like: 
ALTER TABLE ... SET OWNER UNKNOWN 

Do we want to support that? Do we need the UNKNOWN on the PrincipalType?


- Sergio Pena


On May 17, 2018, 3:19 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67186/
> ---
> 
> (Updated May 17, 2018, 3:19 p.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> We need to include type UNKNOWN to PrincipalType to match with 
> HivePrincipal.HivePrincipalType.UKNOWN
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PrincipalType.java
>  82eb8fd700 
> 
> 
> Diff: https://reviews.apache.org/r/67186/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: [VOTE] Stricter commit guidelines

[Sergio Pena]

+1

On Tue, May 15, 2018 at 11:05 AM, Gunther Hagleitner <
ghagleit...@hortonworks.com> wrote:

> +1
> 
> From: Sankar Hariappan 
> Sent: Tuesday, May 15, 2018 9:03 AM
> To: dev@hive.apache.org
> Subject: Re: [VOTE] Stricter commit guidelines
>
> +1
>
>
> On 15/05/18, 9:30 PM, "Sahil Takiar"  wrote:
>
> >+1
> >
> >On Tue, May 15, 2018 at 10:56 AM, Owen O'Malley 
> >wrote:
> >
> >> +1
> >>
> >> On Tue, May 15, 2018 at 8:55 AM, Peter Vary  wrote:
> >>
> >> > +1 - Hoping for something like this for a long while! Thanks for
> taking
> >> > this up all!
> >> >
> >> > > On May 15, 2018, at 5:44 PM, Jesus Camacho Rodriguez <
> >> > jcama...@apache.org> wrote:
> >> > >
> >> > > Forgot to mention the length of the vote in original message.
> >> > >
> >> > > Let's leave the vote open for a shorter period than usual, for
> instance
> >> > 48 hours, i.e., till Wednesday 10pm PST. Situation can only get worse
> >> than
> >> > it is now if we do not take action for a longer period.
> >> > >
> >> > > As Alan suggested, vote passes if there is a lazy majority (at
> least 3
> >> > votes, more +1s than -1s).
> >> > >
> >> > > Thanks,
> >> > > Jesús
> >> > >
> >> > >
> >> > > On 5/15/18, 8:37 AM, "Andrew Sherman" 
> wrote:
> >> > >
> >> > >+1
> >> > >
> >> > >On Tue, May 15, 2018 at 2:34 AM Rui Li 
> >> wrote:
> >> > >
> >> > >> +1
> >> > >>
> >> > >> On Tue, May 15, 2018 at 2:24 PM, Prasanth Jayachandran <
> >> > >> pjayachand...@hortonworks.com> wrote:
> >> > >>
> >> > >>> +1
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>> Thanks
> >> > >>> Prasanth
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>> On Mon, May 14, 2018 at 10:44 PM -0700, "Jesus Camacho Rodriguez"
> <
> >> > >>> jcama...@apache.org> wrote:
> >> > >>>
> >> > >>>
> >> > >>> After work has been done to ignore most of the tests that were
> >> failing
> >> > >>> consistently/intermittently [1], I wanted to start this vote to
> >> gather
> >> > >>> support from the community to be stricter wrt committing patches
> to
> >> > Hive.
> >> > >>> The committers guide [2] already specifies that a +1 should be
> >> obtained
> >> > >>> before committing, but there is another clause that allows
> committing
> >> > >> under
> >> > >>> the presence of flaky tests (clause 4). Flaky tests are as good as
> >> > having
> >> > >>> no tests, hence I propose to remove clause 4 and enforce the +1
> from
> >> > >>> testing infra before committing.
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>> As I see it, by enforcing that we always get a +1 from the testing
> >> > infra
> >> > >>> before committing, 1) we will have a more stable project, and 2)
> we
> >> > will
> >> > >>> have another incentive as a community to create a more robust
> testing
> >> > >>> infra, e.g., replacing flaky tests for similar unit tests that are
> >> not
> >> > >>> flaky, trying to decrease running time for tests, etc.
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>> Please, share your thoughts about this.
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>> Here is my +1.
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>> Thanks,
> >> > >>>
> >> > >>> Jes?s
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>> [1] http://mail-archives.apache.org/mod_mbox/hive-dev/201805.
> >> > >>> mbox/%3C63023673-AEE5-41A9-BA52-5A5DFB2078B6%40apache.org%3E
> >> > >>>
> >> > >>> [2] https://cwiki.apache.org/confluence/display/Hive/
> >> > >>> HowToCommit#HowToCommit-PreCommitruns,andcommittingpatches
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>
> >> > >>
> >> > >> --
> >> > >> Best regards!
> >> > >> Rui Li
> >> > >>
> >> > >
> >> > >
> >> > >
> >> >
> >> >
> >>
> >
> >
> >
> >--
> >Sahil Takiar
> >Software Engineer
> >takiar.sa...@gmail.com | (510) 673-0309
>
>
>

Review Request 67121: HIVE-19527: Preparing for 2.4 development

[Sergio Pena via Review Board]

/pom.xml e2f00dd939bece1609ec6ce5169e1930dfa0978c 
  pom.xml 6fd2f60b84e704a50ded17339f8ddaf16d8751a0 
  ql/pom.xml 2d99c07268c560b2f5a1da1e27e8130fe236cb8a 
  serde/pom.xml d08c7ae7a354bd0881fad09c6b796e2e5f7bc632 
  service-rpc/pom.xml 598fccca66544ba3e738e9dd36d694b732d6ac71 
  service/pom.xml 0443dec3e692c209f5ab64c8887945297759c080 
  shims/0.23/pom.xml 5e7dc5bd1ad89f7182a9e41dde5b9f3f0d0abb4b 
  shims/aggregator/pom.xml d0733e1cf301fbe751cc454e08a7369649930463 
  shims/common/pom.xml a33f0eb5d2b3b58ed3b2fec97ada15ec666c1e6b 
  shims/pom.xml 82002ca36d3eb29f8bfb7876f7234f4c1c008c75 
  shims/scheduler/pom.xml 3ec6ae96f5fafa4e4eb127f46d3c51e6cedf3608 
  spark-client/pom.xml e3bdb47c9eb730233434b9b9ca8282888b09ef04 
  testutils/pom.xml 0ab295a59c04dfc6c384956f6d92a5096cc696cd 
  vector-code-gen/pom.xml 81fcdb89321669f922eb8d6cb3b04821671c80b1 


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


Testing
---


Thanks,

Sergio Pena

Re: Review Request 66979: HIVE-19374: Parse and process ALTER TABLE SET OWNER command syntax

[Sergio Pena via Review Board]

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

(Updated May 8, 2018, 11:57 p.m.)


Review request for hive and Vihang Karajgaonkar.


Changes
---

- Check if ownerType is null on JsonMetaDataFormatted and MetaDataFormatUtils
- Write a test case that verifies the owner and owner type are changed on HMS 
with the alter table set owner command


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


Repository: hive-git


Description
---

This patch implements the new ALTER SET ... SET OWNER command and calls the HMS 
api calls to change the owner of the table.

The command syntax is:
> ALTER TABLE  SET OWNER { USER |GROUP |ROLE 
>  }

Currently, Hive sets the owner of a table to the user who created that table. 
With this command, we will be able to change it to another user, group of role 
(as ALTER DATABASE does).

The changes are:
- HiveParser.g which adds the new syntax
- HiveOperation.java which adds the new ALTERTABLE_OWNER operation
- Table.java which gets/sets the owner type
- SemanticAnalyzer.java which returns the DDLSemanticAnalyzer if an 
ALTERTABLE_OWNER operation is detected
- DDLSemanticAnalyzer.java which analyzes the ALTERTABLE_OWNER Operation
- AlterTableDesc.java uses by the DDL semantic analyzer to change the new owner 
information
- MetaDataFormatUtils which displays the owner type when the DESCRIBE command 
is called
- JsonMetaDataFormatted which is another implementation to display the owner 
type in Json format


Diffs (updated)
-

  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/metadata/TestAlterTableMetadata.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
3141a7e981eb35a9fbc7f367f38f8ad420f1f928 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 
879b4224494c3a9adb0713f319e586db4865fb17 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 cd70eee26c06ee6476964508c54c2bb10b167530 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
 af283e693b5a0fc68e35221b2005fcf1910bdb8e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
defb8becdb5d767ae71d5c962afac43f0c068c3c 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
a837d67b9615ca1ee359c7aa26f79b6f2504dd99 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
820046388adbc65664ae36b08aaba72943ccb6af 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableDesc.java 
a767796a949da3c23ebe6d8c78b995c8638ebfef 
  ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java 
cd4c206a89f1bc1a6195b0f1f39d3c4b462dc027 


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

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


Testing
---

Waiting for HiveQA

- alter_table_set_owner.q which verifies that the new command works. Describe 
is not tested because the .q tests files mask the owner information.
- the describe command verified manually in my local hive environment


Thanks,

Sergio Pena

Review Request 66979: HIVE-19374: Parse and process ALTER TABLE SET OWNER command syntax

[Sergio Pena via Review Board]

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

Review request for hive and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

This patch implements the new ALTER SET ... SET OWNER command and calls the HMS 
api calls to change the owner of the table.

The command syntax is:
> ALTER TABLE  SET OWNER { USER |GROUP |ROLE 
>  }

Currently, Hive sets the owner of a table to the user who created that table. 
With this command, we will be able to change it to another user, group of role 
(as ALTER DATABASE does).

The changes are:
- HiveParser.g which adds the new syntax
- HiveOperation.java which adds the new ALTERTABLE_OWNER operation
- Table.java which gets/sets the owner type
- SemanticAnalyzer.java which returns the DDLSemanticAnalyzer if an 
ALTERTABLE_OWNER operation is detected
- DDLSemanticAnalyzer.java which analyzes the ALTERTABLE_OWNER Operation
- AlterTableDesc.java uses by the DDL semantic analyzer to change the new owner 
information
- MetaDataFormatUtils which displays the owner type when the DESCRIBE command 
is called
- JsonMetaDataFormatted which is another implementation to display the owner 
type in Json format


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
3141a7e981eb35a9fbc7f367f38f8ad420f1f928 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 
879b4224494c3a9adb0713f319e586db4865fb17 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 cd70eee26c06ee6476964508c54c2bb10b167530 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
 af283e693b5a0fc68e35221b2005fcf1910bdb8e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
defb8becdb5d767ae71d5c962afac43f0c068c3c 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
a837d67b9615ca1ee359c7aa26f79b6f2504dd99 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
820046388adbc65664ae36b08aaba72943ccb6af 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableDesc.java 
a767796a949da3c23ebe6d8c78b995c8638ebfef 
  ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java 
cd4c206a89f1bc1a6195b0f1f39d3c4b462dc027 
  ql/src/test/queries/clientpositive/alter_table_set_owner.q PRE-CREATION 
  ql/src/test/results/clientpositive/alter_table_set_owner.q.out PRE-CREATION 


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


Testing
---

Waiting for HiveQA

- alter_table_set_owner.q which verifies that the new command works. Describe 
is not tested because the .q tests files mask the owner information.
- the describe command verified manually in my local hive environment


Thanks,

Sergio Pena

Re: Review Request 66890: HIVE-19371: Add table ownerType to HMS thrift API

[Sergio Pena via Review Board]

> On May 2, 2018, 5:49 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb
> > Lines 1071 (patched)
> > <https://reviews.apache.org/r/66890/diff/1/?file=2015459#file2015459line1071>
> >
> > Is it possible to write a test to make sure this works as expected?

This is Ruby on Rails code. I don't know how to create a test to verify Ruby.
Should we verify it?

Btw, there are other tests in a follow-up patch which implements this Thrift 
API on the ObjectStore.

See https://reviews.apache.org/r/66909/ (needs review too)


- Sergio


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


On May 1, 2018, 8:30 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66890/
> ---
> 
> (Updated May 1, 2018, 8:30 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Bugs: HIVE-19371
> https://issues.apache.org/jira/browse/HIVE-19371
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This is a subtask part of 'HIVE-18762 Support ALTER TABLE SET OWNER command' 
> which adds a new ownerType field on the Table object of the Thrift API of HMS.
> 
> The only file updated (before generting the thrift code) is:
> - hive_metastore.thrift
> 
> The new field is added as 'optional' at the end of the 'struct Table' in 
> order to be backward compatible with older clients of HMS.
> The ownerType filed will be set to a USER as the default value as current 
> tables created are using the owner name as the user.
> 
> 
> Diffs
> -
> 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 
> 47877034746a1cf81cc6731bedb8a6da9cee5b6d 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 
> 629889389e29dcbf9802c348f3b73f6c65695f6f 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 
> c6fadf8a24a32bb208ae6819f159b9b4d5301a2b 
>   
> standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Table.java
>  8dfec980d9e14ad01fda27ad97e2e6a91846f782 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php 
> 9f6cc0e871754afc4fcdbdfb0907d78a0298634c 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 
> 69579e2f593d35c4d52ae7c81d5186d0ae3d379a 
>   standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 
> d7ebaaf914997371201a123f3fc28fac63464ce3 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 
> ccc3c93bcec88b96e524eeb6d47b7c77ccf49d34 
> 
> 
> Diff: https://reviews.apache.org/r/66890/diff/1/
> 
> 
> Testing
> ---
> 
> Waiting for Hive QA.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Review Request 66909: HIVE-19372: Add table ownerType to JDO/SQL and ObjectStore

[Sergio Pena via Review Board]

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

Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
Karajgaonkar.


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


Repository: hive-git


Description
---

This is a subtask part of 'HIVE-18762 Support ALTER TABLE SET OWNER command' 
which adds a new ownerType field to the JDO and SQL schemas, and does read and 
write the field on the ObjectStore.

The ObjectStore will set the ownerType to USER in case the value read is null 
or empty. This is needed for backward compatibility due to the new schema 
upgrades.

Modify a couple of tests to verify the ownerType is correctly altered to a ROLE 
as well.


Diffs
-

  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 6645e551c2299427c61bcf630caeaf8aae6faa82 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MTable.java
 a38a1250e0ede2b7e294aa1fc326a31fdd8e59f3 
  standalone-metastore/src/main/resources/package.jdo 
221192e37668b4af8dc94add71c148a101f8969e 
  standalone-metastore/src/main/sql/derby/hive-schema-3.0.0.derby.sql 
48d28cb7d8650e988e13950d693fa929849c3b40 
  standalone-metastore/src/main/sql/derby/upgrade-2.3.0-to-3.0.0.derby.sql 
b4c46cd237716b86c3f8dca945b81c6f112969fb 
  standalone-metastore/src/main/sql/mssql/hive-schema-3.0.0.mssql.sql 
4c5739f8b37b1b63b59ea3f7211ac53744654442 
  standalone-metastore/src/main/sql/mssql/upgrade-2.3.0-to-3.0.0.mssql.sql 
b338f0d061815719447f9bf9d641fffab96543c3 
  standalone-metastore/src/main/sql/mysql/hive-schema-3.0.0.mysql.sql 
fd71a1d9b02735770f3885c9cfb67114ce26b542 
  standalone-metastore/src/main/sql/mysql/upgrade-2.3.0-to-3.0.0.mysql.sql 
b0d19271e938752f13125c71689ad59aaa775830 
  standalone-metastore/src/main/sql/oracle/hive-schema-3.0.0.oracle.sql 
a45c7bbb0fa01969bffdf7bd70b8e8162e973d68 
  standalone-metastore/src/main/sql/oracle/upgrade-2.3.0-to-3.0.0.oracle.sql 
0b7c793f16463d35fab3f4f4968bb642421c12e8 
  standalone-metastore/src/main/sql/postgres/hive-schema-3.0.0.postgres.sql 
2484744adfa0b7ba5163d24c37c1d38f30b44054 
  
standalone-metastore/src/main/sql/postgres/upgrade-2.3.0-to-3.0.0.postgres.sql 
f06f0dd34e08bfc271e9acd3fa4dde96c695dd98 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java
 9490586aafac6452c8265a6c0b9410f5aeda2e7d 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
 bdc387e08135102db199776e0b90da2a5616aaf3 


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


Testing
---

Waiting for Hive QA


Thanks,

Sergio Pena

Review Request 66890: HIVE-19371: Add table ownerType to HMS thrift API

[Sergio Pena via Review Board]

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

Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
Karajgaonkar.


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


Repository: hive-git


Description
---

This is a subtask part of 'HIVE-18762 Support ALTER TABLE SET OWNER command' 
which adds a new ownerType field on the Table object of the Thrift API of HMS.

The only file updated (before generting the thrift code) is:
- hive_metastore.thrift

The new field is added as 'optional' at the end of the 'struct Table' in order 
to be backward compatible with older clients of HMS.
The ownerType filed will be set to a USER as the default value as current 
tables created are using the owner name as the user.


Diffs
-

  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 
47877034746a1cf81cc6731bedb8a6da9cee5b6d 
  standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 
629889389e29dcbf9802c348f3b73f6c65695f6f 
  standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 
c6fadf8a24a32bb208ae6819f159b9b4d5301a2b 
  
standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Table.java
 8dfec980d9e14ad01fda27ad97e2e6a91846f782 
  standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php 
9f6cc0e871754afc4fcdbdfb0907d78a0298634c 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 
69579e2f593d35c4d52ae7c81d5186d0ae3d379a 
  standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 
d7ebaaf914997371201a123f3fc28fac63464ce3 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 
ccc3c93bcec88b96e524eeb6d47b7c77ccf49d34 


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


Testing
---

Waiting for Hive QA.


Thanks,

Sergio Pena

Re: Review Request 65985: HIVE-18783: ALTER TABLE post-commit listener does not include the transactional listener responses

[Sergio Pena via Review Board]

> On April 4, 2018, 12:34 a.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
> > Lines 408 (patched)
> > <https://reviews.apache.org/r/65985/diff/2/?file=1976883#file1976883line409>
> >
> > why do we need !success check here?
> 
> Sergio Pena wrote:
> Ah? Thanks, I missed this. I removed the first condition and leave this 
> one instead.
> 
> Vihang Karajgaonkar wrote:
> I thought we need the first condition and not the second condition for 
> !success. Are we not generating the events only when the transaction is 
> successful? Am I missing something?

There is a comment I left to answer that question. I've seen in other 
operations that an event is generated even on failed events. I don't know why 
this is needed. I left the event generation just to be consistent with those 
operations, however, if the event failed, then at least just an ALTER_TABLE 
Event should be generated, and not the DROP/CREATE/ADD partitions events.


- Sergio


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


On April 4, 2018, 4:06 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65985/
> ---
> 
> (Updated April 4, 2018, 4:06 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Bugs: HIVE-18783
> https://issues.apache.org/jira/browse/HIVE-18783
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16164 introduced a mechanism to pass HMS notification events ID to the 
> post-commit listeners for all DDL operations, but it didn't add it to the 
> ALTER TABLE event. This patch in review adds the same behavior for ALTER 
> TABLE events.
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  5459554fec911b253583df1f528e5abf134f055b 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
>  ed1b8c5cc2a4cb974dd37419c6b5f0601a035232 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  8539fea42fa2743381833ab3137579caeac64672 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
>  f59f40bc33367cff7a8d0d24d0e200b16c2c30e5 
> 
> 
> Diff: https://reviews.apache.org/r/65985/diff/3/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 65985: HIVE-18783: ALTER TABLE post-commit listener does not include the transactional listener responses

[Sergio Pena via Review Board]

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

(Updated April 4, 2018, 4:06 p.m.)


Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
Karajgaonkar.


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


Repository: hive-git


Description
---

HIVE-16164 introduced a mechanism to pass HMS notification events ID to the 
post-commit listeners for all DDL operations, but it didn't add it to the ALTER 
TABLE event. This patch in review adds the same behavior for ALTER TABLE events.


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 5459554fec911b253583df1f528e5abf134f055b 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
 ed1b8c5cc2a4cb974dd37419c6b5f0601a035232 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 8539fea42fa2743381833ab3137579caeac64672 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
 f59f40bc33367cff7a8d0d24d0e200b16c2c30e5 


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

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


Testing
---

All tests passed.


Thanks,

Sergio Pena

Re: Review Request 65985: HIVE-18783: ALTER TABLE post-commit listener does not include the transactional listener responses

[Sergio Pena via Review Board]

> On April 4, 2018, 12:34 a.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
> > Lines 408 (patched)
> > <https://reviews.apache.org/r/65985/diff/2/?file=1976883#file1976883line409>
> >
> > why do we need !success check here?

Ah? Thanks, I missed this. I removed the first condition and leave this one 
instead.


- Sergio


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


On March 15, 2018, 5:45 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65985/
> ---
> 
> (Updated March 15, 2018, 5:45 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Bugs: HIVE-18783
> https://issues.apache.org/jira/browse/HIVE-18783
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16164 introduced a mechanism to pass HMS notification events ID to the 
> post-commit listeners for all DDL operations, but it didn't add it to the 
> ALTER TABLE event. This patch in review adds the same behavior for ALTER 
> TABLE events.
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  e0e29652da94bbdaca515a17955d1409824c1742 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
>  0dd3eb101709969a77998e1488e1c97214426cd3 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  66353e769b2d633ad9dfad2bcae25e8ad90f61d1 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
>  e6de00100003bc1be12b2772e2e97102ed476cf5 
> 
> 
> Diff: https://reviews.apache.org/r/65985/diff/2/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 65985: HIVE-18783: ALTER TABLE post-commit listener does not include the transactional listener responses

[Sergio Pena via Review Board]

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

(Updated March 15, 2018, 5:45 p.m.)


Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
Karajgaonkar.


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


Repository: hive-git


Description
---

HIVE-16164 introduced a mechanism to pass HMS notification events ID to the 
post-commit listeners for all DDL operations, but it didn't add it to the ALTER 
TABLE event. This patch in review adds the same behavior for ALTER TABLE events.


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 e0e29652da94bbdaca515a17955d1409824c1742 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
 0dd3eb101709969a77998e1488e1c97214426cd3 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 66353e769b2d633ad9dfad2bcae25e8ad90f61d1 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
 e6de0013bc1be12b2772e2e97102ed476cf5 


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

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


Testing
---

All tests passed.


Thanks,

Sergio Pena

Re: Review Request 65985: HIVE-18783: ALTER TABLE post-commit listener does not include the transactional listener responses

[Sergio Pena via Review Board]

> On March 12, 2018, 4:45 p.m., Na Li wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
> > Lines 372 (patched)
> > <https://reviews.apache.org/r/65985/diff/1/?file=1972648#file1972648line372>
> >
> > why when db name is changed at alter table,  the 
> > transactionalListenerResponses is not past into 
> > MetaStoreListenerNotifier.notifyEvent()? And the command won't be able to 
> > block until sentry gets the notification of alter table.

Good catch. I added other variables for each transaction response.


- Sergio


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


On March 8, 2018, 4:11 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65985/
> ---
> 
> (Updated March 8, 2018, 4:11 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Bugs: HIVE-18783
> https://issues.apache.org/jira/browse/HIVE-18783
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16164 introduced a mechanism to pass HMS notification events ID to the 
> post-commit listeners for all DDL operations, but it didn't add it to the 
> ALTER TABLE event. This patch in review adds the same behavior for ALTER 
> TABLE events.
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  e0e29652da94bbdaca515a17955d1409824c1742 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
>  89354a2d34249903a9ff13c4ed913a68de93057e 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  662de9a66767f27f31998f14c68f854e59993ab6 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
>  e6de0013bc1be12b2772e2e97102ed476cf5 
> 
> 
> Diff: https://reviews.apache.org/r/65985/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 65985: HIVE-18783: ALTER TABLE post-commit listener does not include the transactional listener responses

[Sergio Pena via Review Board]

> On March 13, 2018, 3:35 a.m., Alexander Kolbasov wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
> > Lines 362 (patched)
> > <https://reviews.apache.org/r/65985/diff/1/?file=1972648#file1972648line362>
> >
> > Would it make sense to move this out to its own try/catch block? This 
> > would avoid confusion with transaction try/catch block.

Yes, I moved it.


> On March 13, 2018, 3:35 a.m., Alexander Kolbasov wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
> > Lines 365 (patched)
> > <https://reviews.apache.org/r/65985/diff/1/?file=1972648#file1972648line365>
> >
> > This may cause weird situation where operation succeeds and is 
> > commitetd but throws an exception.

I moved it to its own try/catch to avoid such weird situation.


> On March 13, 2018, 3:35 a.m., Alexander Kolbasov wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
> > Lines 366 (patched)
> > <https://reviews.apache.org/r/65985/diff/1/?file=1972648#file1972648line366>
> >
> > It may be easier to read if you use 
> > 
> > if (listeners.isEmpty()) {
> >   return;
> > }

I'd like to keep the if (!listeners.isEmpty()) in case more code is added in 
the future that should be executed after the finally or this if statement and 
avoid the return if listeners is empty.


> On March 13, 2018, 3:35 a.m., Alexander Kolbasov wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
> > Lines 372 (patched)
> > <https://reviews.apache.org/r/65985/diff/1/?file=1972648#file1972648line372>
> >
> > Why are we creating dropTable/createTable notifications here? I 
> > understand that this mimics the code above, but it looks suspicious.

I left a comment there. The reason is if a rename happens between databases, 
then a drop/create table events between both databases should happen. Honestly, 
I don't think that is necessary and an ALTER event should be enough, but this 
code is already there and I don't know would affect other clients using 
notifications. Perhaps this is something to consider on the new HMS v2 design.


> On March 13, 2018, 3:35 a.m., Alexander Kolbasov wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
> > Lines 99 (patched)
> > <https://reviews.apache.org/r/65985/diff/1/?file=1972650#file1972650line99>
> >
> > There is only a single consumer of this interface - it seems odd to 
> > have an interface which is used in some special case. Should all access to 
> > listeners be converted to the interface?

I don't know. All listeners are used in the HiveMetaStore class which contains 
the HMSHandler subclass. Being listeners and transctionalListeners a variable 
of HMSHandler, then the HiveMetaStore can call them directly without need the 
interface. The only consumer is the HiveAlterHandler which should be re-design 
in my opinion as all DDL operations are inside the HiveMetaStore and only the 
alter is in its own class. Could this also be considered in the HMS v2 design?


- Sergio


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


On March 8, 2018, 4:11 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65985/
> ---
> 
> (Updated March 8, 2018, 4:11 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Bugs: HIVE-18783
> https://issues.apache.org/jira/browse/HIVE-18783
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16164 introduced a mechanism to pass HMS notification events ID to the 
> post-commit listeners for all DDL operations, but it didn't add it to the 
> ALTER TABLE event. This patch in review adds the same behavior for ALTER 
> TABLE events.
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  e0e29652da94bbdaca515a17955d1409824c1742 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
>  89354a2d34249903a9ff13c4ed913a68de93057e 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  662de9a66767f27f31998f14c68f854e59993ab6 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
>  e6de0013bc1be12b2772e2e97102ed476cf5 
> 
> 
> Diff: https://reviews.apache.org/r/65985/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Review Request 65985: HIVE-18783: ALTER TABLE post-commit listener does not include the transactional listener responses

[Sergio Pena via Review Board]

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

Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
Karajgaonkar.


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


Repository: hive-git


Description
---

HIVE-16164 introduced a mechanism to pass HMS notification events ID to the 
post-commit listeners for all DDL operations, but it didn't add it to the ALTER 
TABLE event. This patch in review adds the same behavior for ALTER TABLE events.


Diffs
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 e0e29652da94bbdaca515a17955d1409824c1742 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
 89354a2d34249903a9ff13c4ed913a68de93057e 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 662de9a66767f27f31998f14c68f854e59993ab6 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
 e6de0013bc1be12b2772e2e97102ed476cf5 


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


Testing
---

All tests passed.


Thanks,

Sergio Pena

Re: [VOTE] Apache Hive 2.3.2 Release Candidate 0

[Sergio Pena]

+1

I verified the release by doing the following:
* checked the gpg signature
* checked the md5 files
* installed hive 2.3.2 in my local machine with hadoop 2.7.2 and run a few
commands:
  > show databases
  > show tables
  > insert into table values()
  > select * from table
  > select count(*) from table
* checked the maven artifacts are correctly pulled by other components and
run unit tests
* checked that storage-api-2.4.0 is pulled
* checked the release tag
* checked the RELEASE_NOTES, NOTICE, LICENSE are correct

The release is working correctly.

Thanks Sahil for making this release.
- Sergio

On Thu, Nov 9, 2017 at 5:37 PM, Sahil Takiar  wrote:

> Apache Hive 2.3.2 Release Candidate 0 is available here:
> http://people.apache.org/~stakiar/hive-2.3.2/
>
> Maven artifacts are available here:
> https://repository.apache.org/content/repositories/orgapachehive-1082/
>
> Source tag for RCN is at:https://github.com/apache/hive/tree/release-2.3.2
>
> Voting will conclude in 72 hours.
>
> Hive PMC Members: Please test and vote.
>
> Thanks.
>

Re: [Announce] New committer: Tao Li

[Sergio Pena]

Congratulations Tao !!!

On Wed, Nov 1, 2017 at 2:13 PM, Peter Vary  wrote:

> Congratulations Tao! :)
>
> > On Nov 1, 2017, at 6:49 PM, Ashutosh Chauhan 
> wrote:
> >
> > The Project Management Committee (PMC) for Apache Hive has invited Tao
> Li to
> > become a committer and we are pleased to announce that he has accepted.
> >
> > Welcome, Tao!
> >
> > Thanks,
> > Ashutosh
>
>

Re: Review Request 62816: HIVE-17729 Add Database & Explain related blobstore tests

[Sergio Pena via Review Board]

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


Ship it!




Ship It!

- Sergio Pena


On Oct. 6, 2017, 7:13 p.m., Rentao Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62816/
> ---
> 
> (Updated Oct. 6, 2017, 7:13 p.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Bugs: HIVE-17729
> https://issues.apache.org/jira/browse/HIVE-17729
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-17729 Add Database & Explain related blobstore tests
> 
> 
> This patch introduces the following regression tests into the hive-blobstore 
> qtest module:
> create_database.q -> tests tables with location inherited from database
> multiple_db.q -> tests query spanning multiple databases
> explain.q -> tests EXPLAIN INSERT OVERWRITE command
> 
> 
> Diffs
> -
> 
>   data/files/single_int.txt PRE-CREATION 
>   itests/hive-blobstore/src/test/queries/clientpositive/create_database.q 
> PRE-CREATION 
>   itests/hive-blobstore/src/test/queries/clientpositive/explain.q 
> PRE-CREATION 
>   itests/hive-blobstore/src/test/queries/clientpositive/multiple_db.q 
> PRE-CREATION 
>   itests/hive-blobstore/src/test/results/clientpositive/create_database.q.out 
> PRE-CREATION 
>   itests/hive-blobstore/src/test/results/clientpositive/explain.q.out 
> PRE-CREATION 
>   itests/hive-blobstore/src/test/results/clientpositive/multiple_db.q.out 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62816/diff/1/
> 
> 
> Testing
> ---
> 
> under /itests/hive-blobstore/
> 
> $ mvn clean test -Dtest=TestBlobstoreCliDriver 
> -Dqfile=create_database.q,explain.q,multiple_db.q
> 
> ---
>  T E S T S
> ---
> Running org.apache.hadoop.hive.cli.TestBlobstoreCliDriver
> Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 48.4 sec - in 
> org.apache.hadoop.hive.cli.TestBlobstoreCliDriver
> 
> Results :
> 
> Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] 
> 
> [INFO] BUILD SUCCESS
> [INFO] 
> 
> [INFO] Total time: 01:12 min
> [INFO] Finished at: 2017-10-06T11:29:41-07:00
> [INFO] Final Memory: 88M/1003M
> [INFO] 
> 
> 
> 
> Thanks,
> 
> Rentao Wu
> 
>

Re: [VOTE] Apache Hive 2.3.1 Release Candidate 0

[Sergio Pena]

Great, thanks Jesus.

On Tue, Oct 24, 2017 at 5:00 PM, Jesus Camacho Rodriguez <
jcama...@apache.org> wrote:

> Sergio,
>
> branch-2.3 is open for commits again.
>
> Thanks,
> Jesús
>
>
>
> On 10/24/17, 10:56 AM, "Jesus Camacho Rodriguez"  hortonworks.com on behalf of jcama...@apache.org> wrote:
>
> >Thanks to everyone who has tested the release candidate and given
> >their comments and votes.
> >
> >The tally is as follows.
> >
> >
> >3 +1 votes:
> >* Alan (binding)
> >* Ashutosh (binding)
> >* Jesús (binding)
> >
> >No 0s or -1s.
> >
> >
> >Therefore I am delighted to announce that the proposal to release
> >Apache Hive 2.3.1 has passed.
> >
> >
> >@Sergio, bylaws establish a minimum of 72 hours for the vote but no
> >upper limit. I will now roll the release out to the mirrors and
> >ping you so you can push HIVE-17831.
> >
> >Thanks,
> >Jesús
> >
> >
> >
> >
> >On 10/24/17, 10:38 AM, "Alan Gates"  wrote:
> >
> >>+1.  Built from source with a clean repo, checked signatures, ran rat.
> >>
> >>Alan.
> >>
> >>On Tue, Oct 24, 2017 at 8:40 AM, Ashutosh Chauhan 
> >>wrote:
> >>
> >>> - Built from sources.
> >>> - Ran few unit tests.
> >>> - Checked md5sum.
> >>>
> >>> Everything looks good. +1
> >>>
> >>> Thanks Jesus for putting this release together.
> >>> Ashutosh
> >>>
> >>> On Thu, Oct 19, 2017 at 8:06 PM, Jesus Camacho Rodriguez <
> >>> jcama...@apache.org> wrote:
> >>>
> >>> > Apache Hive 2.3.1 Release Candidate 0 is available here:
> >>> > http://people.apache.org/~jcamacho/hive-2.3.1-rc0/
> >>> >
> >>> > Maven artifacts are available here:
> >>> > https://repository.apache.org/content/repositories/
> orgapachehive-1081/
> >>> >
> >>> > Source tag for RC0 is at:
> >>> > https://github.com/apache/hive/releases/tag/release-2.3.1-rc0/
> >>> >
> >>> > Voting will conclude in 72 hours.
> >>> >
> >>> > Hive PMC Members: Please test and vote.
> >>> >
> >>> > Thanks.
> >>> >
> >>> >
> >>> >
> >>> >
> >>>
> >
>
>

Re: [VOTE] Apache Hive 2.3.1 Release Candidate 0

[Sergio Pena]

Jesus, the vote expired. I wonder if we're allowed to commit HIVE-17831
into the branch 2.3 so it is available in this release?

On Tue, Oct 24, 2017 at 10:40 AM, Ashutosh Chauhan 
wrote:

> - Built from sources.
> - Ran few unit tests.
> - Checked md5sum.
>
> Everything looks good. +1
>
> Thanks Jesus for putting this release together.
> Ashutosh
>
> On Thu, Oct 19, 2017 at 8:06 PM, Jesus Camacho Rodriguez <
> jcama...@apache.org> wrote:
>
> > Apache Hive 2.3.1 Release Candidate 0 is available here:
> > http://people.apache.org/~jcamacho/hive-2.3.1-rc0/
> >
> > Maven artifacts are available here:
> > https://repository.apache.org/content/repositories/orgapachehive-1081/
> >
> > Source tag for RC0 is at:
> > https://github.com/apache/hive/releases/tag/release-2.3.1-rc0/
> >
> > Voting will conclude in 72 hours.
> >
> > Hive PMC Members: Please test and vote.
> >
> > Thanks.
> >
> >
> >
> >
>

Re: [VOTE] Apache Hive 2.3.1 Release Candidate 0

[Sergio Pena]

Great, thanks Jesus for preparing this. Just one request, can we include
HIVE-17831 in the 2.3.1 release? Sentry has been waiting to bump the Hive
version to 2.x for a while, and HIVE-17831 fixes an issue that is required
to make it work.

It's a small fix and is already part of 3.0 and 2.4.

- Sergio

On Thu, Oct 19, 2017 at 10:06 PM, Jesus Camacho Rodriguez <
jcama...@apache.org> wrote:

> Apache Hive 2.3.1 Release Candidate 0 is available here:
> http://people.apache.org/~jcamacho/hive-2.3.1-rc0/
>
> Maven artifacts are available here:
> https://repository.apache.org/content/repositories/orgapachehive-1081/
>
> Source tag for RC0 is at:
> https://github.com/apache/hive/releases/tag/release-2.3.1-rc0/
>
> Voting will conclude in 72 hours.
>
> Hive PMC Members: Please test and vote.
>
> Thanks.
>
>
>
>

Re: Review Request 62736: HIVE-17544: Provide classname info for function authorization

[Sergio Pena]

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


Ship it!




Ship It!

- Sergio Pena


On Oct. 2, 2017, 9:10 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62736/
> ---
> 
> (Updated Oct. 2, 2017, 9:10 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-17544: Provide classname info for function authorization
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> f01edf8156205ea452178f6539e5bbfe29742428 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 
> 820e4e2f67506e21efa0a1c7aaa37fb1c4b88f55 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java 
> 4707c4d1bbc71e256b16511f48f8650c027773fe 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/FunctionSemanticAnalyzer.java 
> c5380750f23cc731d33354ea09858882b2364ce7 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizerImpl.java
>  570571b274a090e2147d3ac485194082be12b813 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  fb4c3206be3a92b3fd52826082fe103d5f445d0e 
> 
> 
> Diff: https://reviews.apache.org/r/62736/diff/1/
> 
> 
> Testing
> ---
> 
> Manually tested.
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: New committer : Barna Zsombor Klara

[Sergio Pena]

Great, congratulations Zsombor !!!

On Mon, Sep 18, 2017 at 12:44 PM, Sahil Takiar 
wrote:

> Congrats Zsombor!
>
> On Mon, Sep 18, 2017 at 9:50 AM, Vihang Karajgaonkar 
> wrote:
>
> > Congratulations Zsombor!
> >
> > On Mon, Sep 18, 2017 at 8:15 AM, Zoltan Haindrich <
> > zhaindr...@hortonworks.com> wrote:
> >
> > > Congratulations!!
> > >
> > > On 16 Sep 2017 00:52, Ashutosh Chauhan  wrote:
> > > The Project Management Committee (PMC) for Apache Hive has invited
> Barna
> > > Zsombor Klara to become a committer and we are pleased to announce that
> > he
> > > has accepted.
> > >
> > > Barna, thank you for your contributions, and we look forward your
> > > further interactions
> > > with the community!
> > >
> > > Welcome, Barna!
> > >
> > > Thanks,
> > > Ashutosh (on behalf of Apache Hive PMC)
> > >
> > >
> >
>
>
>
> --
> Sahil Takiar
> Software Engineer at Cloudera
> takiar.sa...@gmail.com | (510) 673-0309
>

Re: Review Request 60085: HIVE-14747: Remove JAVA paths from profiles by sending them from ptest-client

[Sergio Pena]

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


Ship it!




Ship It!

- Sergio Pena


On Aug. 17, 2017, 7:56 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60085/
> ---
> 
> (Updated Aug. 17, 2017, 7:56 a.m.)
> 
> 
> Review request for hive, Marta Kuczora, Peter Vary, and Sergio Pena.
> 
> 
> Bugs: HIVE-14747
> https://issues.apache.org/jira/browse/HIVE-14747
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14747: Remove JAVA paths from profiles by sending them from ptest-client
> 
> 
> Diffs
> -
> 
>   testutils/ptest2/conf/server-env.properties.example PRE-CREATION 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/ExecutionController.java
>  2f96ad03023e9f51d44d203f34edd04183605a22 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/TestExecutor.java
>  b2c61f03c5bf5f170894141848c89fc26129115a 
>   testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java 
> 1cdfdb309acd8282e593abd7ed10c87721926c60 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/Context.java
>  14984bafdd18fb7636e729cc7fbbfa349b0f043e 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/ExecutionContextConfiguration.java
>  35ddd44accf34be1f5957c6df31802ee8c8022b5 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/TestConfiguration.java
>  e584f9c105fa134e3e267d6c6817d441b4c6b249 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/TestParser.java
>  a243774e52f3f5fda4a082bb99387cf5808c307b 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestJIRAService.java
>  b97b890dfe855539de2696788327ba9b4a841ff3 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestContext.java
>  PRE-CREATION 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestTestConfiguration.java
>  848faf27af1ed8945d7013b6562bab544605e4bc 
> 
> 
> Diff: https://reviews.apache.org/r/60085/diff/6/
> 
> 
> Testing
> ---
> 
> Added unit tests for the Context class.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 60085: HIVE-14747: Remove JAVA paths from profiles by sending them from ptest-client

[Sergio Pena]

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



Thanks Zsombor, just two more comments:
- There is an example for the cloudhost.properties file on 
hive/testutils/ptest2/conf/cloudhost.properties.example. Can we provide an 
example for the server properties as well? 
- How is the ptest server configured to pass the server properties? I'm trying 
to figure it out from the code (I see that using command line parameters and 
another place as an environment variable). Could you give an example?

- Sergio Pena


On Aug. 8, 2017, 12:50 p.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60085/
> ---
> 
> (Updated Aug. 8, 2017, 12:50 p.m.)
> 
> 
> Review request for hive, Marta Kuczora, Peter Vary, and Sergio Pena.
> 
> 
> Bugs: HIVE-14747
> https://issues.apache.org/jira/browse/HIVE-14747
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14747: Remove JAVA paths from profiles by sending them from ptest-client
> 
> 
> Diffs
> -
> 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/ExecutionController.java
>  2f96ad03023e9f51d44d203f34edd04183605a22 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/TestExecutor.java
>  b2c61f03c5bf5f170894141848c89fc26129115a 
>   testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java 
> 1cdfdb309acd8282e593abd7ed10c87721926c60 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/Context.java
>  14984bafdd18fb7636e729cc7fbbfa349b0f043e 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/ExecutionContextConfiguration.java
>  35ddd44accf34be1f5957c6df31802ee8c8022b5 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/TestConfiguration.java
>  e584f9c105fa134e3e267d6c6817d441b4c6b249 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/TestParser.java
>  a243774e52f3f5fda4a082bb99387cf5808c307b 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestJIRAService.java
>  b97b890dfe855539de2696788327ba9b4a841ff3 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestContext.java
>  PRE-CREATION 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestTestConfiguration.java
>  848faf27af1ed8945d7013b6562bab544605e4bc 
> 
> 
> Diff: https://reviews.apache.org/r/60085/diff/5/
> 
> 
> Testing
> ---
> 
> Added unit tests for the Context class.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 60085: HIVE-14747: Remove JAVA paths from profiles by sending them from ptest-client

[Sergio Pena]

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



How are we getting the Java variable information from the new code? I see we 
initialize a couple of classes with a specific context that may have a server 
environment file context, but how are we getting Java info from such context?

- Sergio Pena


On July 31, 2017, 8:05 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60085/
> ---
> 
> (Updated July 31, 2017, 8:05 a.m.)
> 
> 
> Review request for hive, Marta Kuczora, Peter Vary, and Sergio Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14747: Remove JAVA paths from profiles by sending them from ptest-client
> 
> 
> Diffs
> -
> 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/ExecutionController.java
>  2f96ad03023e9f51d44d203f34edd04183605a22 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/TestExecutor.java
>  b2c61f03c5bf5f170894141848c89fc26129115a 
>   testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java 
> 1cdfdb309acd8282e593abd7ed10c87721926c60 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/Context.java
>  14984bafdd18fb7636e729cc7fbbfa349b0f043e 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/ExecutionContextConfiguration.java
>  35ddd44accf34be1f5957c6df31802ee8c8022b5 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/TestConfiguration.java
>  e584f9c105fa134e3e267d6c6817d441b4c6b249 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/TestParser.java
>  a243774e52f3f5fda4a082bb99387cf5808c307b 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestJIRAService.java
>  b97b890dfe855539de2696788327ba9b4a841ff3 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestContext.java
>  PRE-CREATION 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestTestConfiguration.java
>  848faf27af1ed8945d7013b6562bab544605e4bc 
> 
> 
> Diff: https://reviews.apache.org/r/60085/diff/3/
> 
> 
> Testing
> ---
> 
> Added unit tests for the Context class.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 60085: HIVE-14747: Remove JAVA paths from profiles by sending them from ptest-client

[Sergio Pena]

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




testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/TestExecutor.java
Lines 120 (patched)
<https://reviews.apache.org/r/60085/#comment258171>

is there another wayt to get the file instead of the system environment 
variables?

How do we get the cloud execution properties file?



testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/Context.java
Lines 266 (patched)
<https://reviews.apache.org/r/60085/#comment258170>

Can you add javadoc to all the new public methods and ContextBuilder?


- Sergio Pena


On July 31, 2017, 8:05 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60085/
> ---
> 
> (Updated July 31, 2017, 8:05 a.m.)
> 
> 
> Review request for hive, Marta Kuczora, Peter Vary, and Sergio Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14747: Remove JAVA paths from profiles by sending them from ptest-client
> 
> 
> Diffs
> -
> 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/ExecutionController.java
>  2f96ad03023e9f51d44d203f34edd04183605a22 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/TestExecutor.java
>  b2c61f03c5bf5f170894141848c89fc26129115a 
>   testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java 
> 1cdfdb309acd8282e593abd7ed10c87721926c60 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/Context.java
>  14984bafdd18fb7636e729cc7fbbfa349b0f043e 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/ExecutionContextConfiguration.java
>  35ddd44accf34be1f5957c6df31802ee8c8022b5 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/TestConfiguration.java
>  e584f9c105fa134e3e267d6c6817d441b4c6b249 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/conf/TestParser.java
>  a243774e52f3f5fda4a082bb99387cf5808c307b 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/TestJIRAService.java
>  b97b890dfe855539de2696788327ba9b4a841ff3 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestContext.java
>  PRE-CREATION 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestTestConfiguration.java
>  848faf27af1ed8945d7013b6562bab544605e4bc 
> 
> 
> Diff: https://reviews.apache.org/r/60085/diff/3/
> 
> 
> Testing
> ---
> 
> Added unit tests for the Context class.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 60006: HIVE-14746: Remove branch and repositories from profiles by sending them from ptest-client

[Sergio Pena]

> On June 14, 2017, 4:03 p.m., Sergio Pena wrote:
> > testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/60006/diff/2/?file=1749730#file1749730line108>
> >
> > Why is a workingDirectoryWrapper needed?
> > 
> > Currently, Ptest works on a 'working' directory for any branch detected 
> > on the profile. Why do we want to change that for different branches?
> 
> Barna Zsombor Klara wrote:
> I did this based on Siddharth Seth's comments on the Jira, where he 
> mentioned that associating the working dir with the branch could prevent 
> frequent rebasing when the checkout occurs. We don't need it for the original 
> intent of the Jira. We can discuss if we want it or if the rebase would not 
> be that problematic.

It's a good time savings. However, it's done only once. I don't know how much 
time we do save. Anyway, can we have this as a follow up jira?


- Sergio


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


On June 13, 2017, 2:28 p.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60006/
> ---
> 
> (Updated June 13, 2017, 2:28 p.m.)
> 
> 
> Review request for hive, Sergio Pena and Siddharth Seth.
> 
> 
> Bugs: HIVE-14746
> https://issues.apache.org/jira/browse/HIVE-14746
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14746: Remove branch and repositories from profiles by sending them from 
> ptest-client
> 
> 
> Diffs
> -
> 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/PTestClient.java
>  8e2604d372ac29b94445b269f08423b058308efe 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/request/TestStartRequest.java
>  8deed52ae0307d4fc075654a4d75e6cb09a5d9db 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/TestExecutor.java
>  b2c61f03c5bf5f170894141848c89fc26129115a 
>   testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java 
> 1cdfdb309acd8282e593abd7ed10c87721926c60 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/CloudExecutionContextProvider.java
>  8b82497bdaf43694e0e1552e125b5ffdce40f56c 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/ExecutionContext.java
>  b09de1d4d930cf2d4d26b500f3457cea3fffa9ce 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/FixedExecutionContextProvider.java
>  f7b50d6a61962d2727b5181215be9de2e64b05b7 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/WorkingDirWrapper.java
>  PRE-CREATION 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/api/server/TestTestExecutor.java
>  a4a789b579305d9ed573d8c1fd0b6ce75787d50f 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestTestConfiguration.java
>  848faf27af1ed8945d7013b6562bab544605e4bc 
> 
> 
> Diff: https://reviews.apache.org/r/60006/diff/2/
> 
> 
> Testing
> ---
> 
> Manually tested the PTestClient with and without the branch argument.
> Updated and ran the unit tests.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 61041: HIVE-17150: CREATE INDEX execute HMS out-of-transaction listener calls inside a transaction

[Sergio Pena]

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

(Updated July 21, 2017, 5:51 p.m.)


Review request for hive, Alexander Kolbasov, Mohit Sabharwal, and Vihang 
Karajgaonkar.


Changes
---

Fixing HiveQA tests.


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


Repository: hive-git


Description
---

The patch adds a new parameter, HIVE_METASTORE_TRANSACTION_ACTIVE, to the 
parameters passed to the notification listeners. This parameter has a 
true/false value dependinf if the HMS is running in a transaction or not.


Diffs (updated)
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 6d7ee4cb824ba40537876bd0629831a19ac91d76 
  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
 a4f2d592ced6427a05177e67be27286c408744a6 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 b016920fa57501b9f07f1810b2f8010e40575efd 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 808c9c7c36fd0ba36adac7be942c4841ab0a08a8 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
58b9044930046758a83ee499692e5593cd82f9e0 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 20011ccec83e87a55de7668b86773bb817135cbd 
  metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 
8f6af9f346102359c3cd1a6c27000f46e1ddbae6 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
3ac4fe1604c7b0b455894b8e6293484e9226836e 
  metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
5a45051f4417352f20334319c796dc0ca2d8ad9e 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 bd33c7101f5bc01a482153f810b65b620a061636 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 94cbd5235dce0b49b92203ff92d0d2e29e8b2ca9 


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

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


Testing
---


Thanks,

Sergio Pena

Review Request 61041: HIVE-17150: CREATE INDEX execute HMS out-of-transaction listener calls inside a transaction

[Sergio Pena]

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

Review request for hive, Alexander Kolbasov, Mohit Sabharwal, and Vihang 
Karajgaonkar.


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


Repository: hive-git


Description
---

The patch adds a new parameter, HIVE_METASTORE_TRANSACTION_ACTIVE, to the 
parameters passed to the notification listeners. This parameter has a 
true/false value dependinf if the HMS is running in a transaction or not.


Diffs
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 6d7ee4cb824ba40537876bd0629831a19ac91d76 
  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
 a4f2d592ced6427a05177e67be27286c408744a6 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 b016920fa57501b9f07f1810b2f8010e40575efd 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
58b9044930046758a83ee499692e5593cd82f9e0 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 20011ccec83e87a55de7668b86773bb817135cbd 
  metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 
8f6af9f346102359c3cd1a6c27000f46e1ddbae6 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
3ac4fe1604c7b0b455894b8e6293484e9226836e 
  metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
5a45051f4417352f20334319c796dc0ca2d8ad9e 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 bd33c7101f5bc01a482153f810b65b620a061636 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 94cbd5235dce0b49b92203ff92d0d2e29e8b2ca9 


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


Testing
---


Thanks,

Sergio Pena

Re: [VOTE] Apache Hive 2.3.0 Release Candidate 1

[Sergio Pena]

What happened with the 2.2.0 release? is it confusing to jump from 2.1 ->
2.3 without having a 2.2 release previously?

On Fri, Jul 14, 2017 at 2:02 AM, Pengcheng Xiong  wrote:

> Apache Hive 2.3.0 Release Candidate 1 is available here:
>
> Artifacts:
> tag: *https://github.com/apache/hive/releases/tag/release-2.3.0-rc1
> *
> tar ball: http://home.apache.org/~pxiong/apache-hive-2.3.0
> 
>
> Voting will conclude in 72 hours.
>
> Hive PMC Members: Please test and vote.
>
> Here is my +1 after running rat check, md5 check and simple queries.
>
> Thanks.
>
> Best
> Pengcheng
>

Re: Review Request 59020: Support Parquet through HCatalog

[Sergio Pena]

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


Ship it!




Ship It!

- Sergio Pena


On July 12, 2017, 2:57 p.m., Adam Szita wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59020/
> ---
> 
> (Updated July 12, 2017, 2:57 p.m.)
> 
> 
> Review request for hive, Aihua Xu and Sergio Pena.
> 
> 
> Bugs: HIVE-8838
> https://issues.apache.org/jira/browse/HIVE-8838
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding support for HCatalog to write tables stored in Parquet format
> 
> 
> Diffs
> -
> 
>   
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java
>  b2abc5fbb3670893415354552239d67d072459ed 
>   
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java
>  60af5c0bf397273fb820f0ee31e578745dbc200f 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java
>  4c686fec596d39d41d458bc3ea2753877bd9df98 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java
>  ad11eab1b7e67541b56e90e4a85ba37b41a4db92 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java
>  918332ddfda58306707d326f8668b2c223110a29 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java
>  6cd382145b55d6b85fc3366faeaba2aaef65ab04 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java
>  6dfdc04954dd0b110b1a7194e69468b5dc2f842e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
>  379a9135d9c631b2f473976b00f3dc87f9fec0c4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java
>  c021dafa480e65d7c0c19a5a85988464112468cb 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java
>  ec85b5df0f95cbd45b87259346ae9c1e5aa604a4 
> 
> 
> Diff: https://reviews.apache.org/r/59020/diff/2/
> 
> 
> Testing
> ---
> 
> Tested on cluster, and re-enabled previously disabled tests in HCatalog (for 
> Parquet) that were failing (this adds ~40 tests to be run)
> 
> 
> Thanks,
> 
> Adam Szita
> 
>

Re: Review Request 59020: Support Parquet through HCatalog

[Sergio Pena]

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


Fix it, then Ship it!




The patch looks good Adam. Please just add the comments and remove the unused 
imports.


hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java
Lines 46-49 (original), 46-49 (patched)
<https://reviews.apache.org/r/59020/#comment255324>

There are a few unused imports. Remove them.



hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java
Lines 21-28 (original), 21-28 (patched)
<https://reviews.apache.org/r/59020/#comment255323>

There are a few unused imports. Remove them.



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
Line 76 (original), 76 (patched)
<https://reviews.apache.org/r/59020/#comment255325>

Could you add a javadoc comment about what parameters are expected on the 
JobConf object? 

I see that this constructor won't work unless the PARQUET_HIVE_SCHEMA is 
set by the caller (HCat is doing in on the SpecialCases), and we have to leave 
that comment about the assumptions.



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java
Lines 24 (patched)
<https://reviews.apache.org/r/59020/#comment255309>

I like to avoid short words on method names. What bout just using 
isParquetProperty() instead? The class name already implies this is meant for 
parquet tables.



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java
Lines 87 (patched)
<https://reviews.apache.org/r/59020/#comment255312>

Same thing with the short words on method names. 

However, makes more sense to use getParquetProperties(), what do you think? 
Table properties are set through the Table, but jobConf can bring other parquet 
properties that were set in hive-site.xml, right?


- Sergio Pena


On May 5, 2017, 8:11 a.m., Adam Szita wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59020/
> ---
> 
> (Updated May 5, 2017, 8:11 a.m.)
> 
> 
> Review request for hive, Aihua Xu and Sergio Pena.
> 
> 
> Bugs: HIVE-8838
> https://issues.apache.org/jira/browse/HIVE-8838
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding support for HCatalog to write tables stored in Parquet format
> 
> 
> Diffs
> -
> 
>   
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java
>  b2abc5fbb3670893415354552239d67d072459ed 
>   
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java
>  60af5c0bf397273fb820f0ee31e578745dbc200f 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java
>  4c686fec596d39d41d458bc3ea2753877bd9df98 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java
>  ad11eab1b7e67541b56e90e4a85ba37b41a4db92 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java
>  918332ddfda58306707d326f8668b2c223110a29 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java
>  6cd382145b55d6b85fc3366faeaba2aaef65ab04 
>   
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java
>  6dfdc04954dd0b110b1a7194e69468b5dc2f842e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
>  a7bb5eedbb99f3cea4601b9fce9a0ad3461567d0 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java 
> b339cc4347eea143dca2f6d98f9aaafdc427 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriteSupport.java
>  71a78cf040667bf14b6c720373e4acd102da19f4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java
>  c021dafa480e65d7c0c19a5a85988464112468cb 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java
>  ec85b5df0f95cbd45b87259346ae9c1e5aa604a4 
> 
> 
> Diff: https://reviews.apache.org/r/59020/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on cluster, and re-enabled previously disabled tests in HCatalog (for 
> Parquet) that were failing (this adds ~40 tests to be run)
> 
> 
> Thanks,
> 
> Adam Szita
> 
>

Re: [Announce] New committer: Deepesh Khandelwal

[Sergio Pena]

Congratulations Deepesh !!!

On Wed, Jul 5, 2017 at 7:48 PM, Ashutosh Chauhan 
wrote:

> The Project Management Committee (PMC) for Apache Hive has invited Deepesh
> Khandelwal to become a committer and we are pleased to announce that he has
> accepted.
>
> Welcome, Deepesh!
>
> Thanks,
> Ashutosh
>

Re: [Announce] New committer: Sahil Takiar

[Sergio Pena]

Congratulations Sahil !!!

On Wed, Jul 5, 2017 at 7:49 PM, Ashutosh Chauhan 
wrote:

> The Project Management Committee (PMC) for Apache Hive has invited Sahil
> Takiar to become a committer and we are pleased to announce that he has
> accepted.
>
> Welcome, Sahil!
>
> Thanks,
> Ashutosh
>

Re: [Announce] New committer: Vihang Karajgaonkar

[Sergio Pena]

Congratulations Vihang !!!.

- Sergio

On Fri, Jul 7, 2017 at 12:50 PM, Vaibhav Gumashta  wrote:

> Congratulations Vihang!
>
> On 7/7/17, 10:47 AM, "Vineet Garg"  wrote:
>
> >Congratulations Vihang!
> >
> >> On Jul 5, 2017, at 5:51 PM, Ashutosh Chauhan 
> >>wrote:
> >>
> >> The Project Management Committee (PMC) for Apache Hive has invited
> >>Vihang
> >> Karajgaonkar to become a committer and we are pleased to announce that
> >>he
> >> has accepted.
> >>
> >> Welcome, Vihang!
> >>
> >> Thanks,
> >> Ashutosh
> >
>
>

Re: [Announce] New committer: Peter Vary

[Sergio Pena]

Congrats Peter !!

On Fri, Jul 7, 2017 at 12:51 PM, Vaibhav Gumashta  wrote:

> Congratulations Peter!
>
> On 7/7/17, 10:46 AM, "Vineet Garg"  wrote:
>
> >Congrats Peter!
> >
> >> On Jul 7, 2017, at 9:27 AM, Xuefu Zhang  wrote:
> >>
> >> Congratulations!
> >>
> >> On Fri, Jul 7, 2017 at 4:17 AM, Adam Szita  wrote:
> >>
> >>> Congrats all!
> >>>
> >>> On 7 July 2017 at 10:03, Zoltan Haindrich 
> >>> wrote:
> >>>
>  Congratulations Peter, Teddy, Deepesh, Vihang and Sahil!
>  It's great to see that the Hive community is growing!
> 
>  On 6 Jul 2017 02:52, Ashutosh Chauhan  wrote:
>  The Project Management Committee (PMC) for Apache Hive has invited
> Peter
>  Vary to become a committer and we are pleased to announce that he has
>  accepted.
> 
>  Welcome, Peter!
> 
>  Thanks,
>  Ashutosh
> 
> 
> >>>
> >
>
>

Re: [ANNOUNCE] New PMC Member : Matt McCline

[Sergio Pena]

Congrats Matt !!

On Fri, Jul 7, 2017 at 12:51 PM, Vaibhav Gumashta  wrote:

> Congratulations Matt!
>
> On 7/7/17, 10:46 AM, "Vineet Garg"  wrote:
>
> >Congrats Matt!
> >> On Jul 7, 2017, at 9:57 AM, Vihang Karajgaonkar 
> >>wrote:
> >>
> >> Congratulations Matt!
> >>
> >> On Fri, Jul 7, 2017 at 9:27 AM, Xuefu Zhang  wrote:
> >>
> >>> Congratulations!
> >>>
> >>> On Fri, Jul 7, 2017 at 8:27 AM, Eugene Koifman
> >>>
> >>> wrote:
> >>>
>  Congratulations!
> 
>  On 7/7/17, 1:04 AM, "Zoltan Haindrich" 
> >>> wrote:
> 
> Congrats Matt!
> 
> On 7 Jul 2017 09:46, Peter Vary  wrote:
> Congratulations Matt! :)
> 
> 2017. júl. 7. 0:34 ezt írta ("Jesus Camacho Rodriguez" <
>  jcama...@apache.org
> > ):
> 
> > Congrats Matt!
> >
> > -Jesús
> >
> >
> >
> > On 7/6/17, 11:13 PM, "Lefty Leverenz" 
>  wrote:
> >
> >> Congratulations Matt!  Well deserved.
> >>
> >> -- Lefty
> >>
> >> On Thu, Jul 6, 2017 at 11:31 AM, Ashutosh Chauhan <
>  hashut...@apache.org>
> >> wrote:
> >>
> >>> On behalf of the Hive PMC I am delighted to announce Matt
> >>> McCline
>  is
> >>> joining Hive PMC.
> >>> Matt is a long time contributor in Hive and is focusing on
>  vectorization
> >>> these days.
> >>>
> >>> Welcome, Matt!
> >>>
> >>> Thanks,
> >>> Ashutosh
> >>>
> >
> >
> 
> 
> 
> 
> >>>
> >
>
>

Re: [Announce] New committer: Teddy Choi

[Sergio Pena]

Congratulations Teddy!

On Fri, Jul 7, 2017 at 12:51 PM, Vaibhav Gumashta  wrote:

> Congratulations Teddy!
>
> On 7/7/17, 10:46 AM, "Vineet Garg"  wrote:
>
> >Congratulations!
> >
> >> On Jul 7, 2017, at 10:25 AM, Gunther Hagleitner
> >> wrote:
> >>
> >> Congrats Teddy!
> >> 
> >> From: Xuefu Zhang 
> >> Sent: Friday, July 07, 2017 9:26 AM
> >> To: dev@hive.apache.org
> >> Subject: Re: [Announce] New committer: Teddy Choi
> >>
> >> Congratulations!
> >>
> >> On Fri, Jul 7, 2017 at 12:09 AM, Matthew McCline
> >>
> >> wrote:
> >>
> >>> Congratulations Teddy!
> >>>
> >>> Get Outlook for iOS
> >>>
> >>>
> >>>
> >>> On Wed, Jul 5, 2017 at 5:53 PM -0700, "Ashutosh Chauhan" <
> >>> hashut...@apache.org> wrote:
> >>>
> >>>
> >>> The Project Management Committee (PMC) for Apache Hive has invited
> >>>Teddy
> >>> Choi to become a committer and we are pleased to announce that he has
> >>> accepted.
> >>>
> >>> Welcome, Teddy!
> >>>
> >>> Thanks,
> >>> Ashutosh
> >>>
> >>>
> >>
> >>
> >
>
>

Re: Review Request 60445: HIVE-16935: Hive should strip comments from input before choosing which CommandProcessor to run.

[Sergio Pena]

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


Ship it!




Great, this looks very good.

- Sergio Pena


On July 5, 2017, 6:09 p.m., Andrew Sherman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60445/
> ---
> 
> (Updated July 5, 2017, 6:09 p.m.)
> 
> 
> Review request for hive and Sahil Takiar.
> 
> 
> Bugs: HIVE-16935
> https://issues.apache.org/jira/browse/HIVE-16935
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> We strip sql comments from a command string. The stripped command is use to 
> determine which
> CommandProcessor will execute the command. If the CommandProcessorFactory 
> does not select a special
> CommandProcessor then we execute the original unstripped command so that the 
> sql parser can remove comments.
> Move BeeLine's comment stripping code to HiveStringUtils and change BeeLine 
> to call it from there
> Add a better test with separate tokens for "set role" in 
> TestCommandProcessorFactory.
> Add a test case for comment removal in set_processor_namespaces.q using an 
> indented comment as
> unindented comments are removed by the test driver.
> 
> Change-Id: I166dc1e7588ec9802ba373d88e69e716aecd33c2
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/Commands.java 
> 3b2d72ed79771e6198e62c47060a7f80665dbcb2 
>   beeline/src/test/org/apache/hive/beeline/TestCommands.java 
> 04c939a04c7a56768286743c2bb9c9797507e3aa 
>   cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 
> 27fd66d35ea89b0de0d17763625fbf564584fcca 
>   common/src/java/org/apache/hive/common/util/HiveStringUtils.java 
> 4a6413a7c376ffb4de6d20d24707ac5bf89ebc0c 
>   common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java 
> 6bd7037152c6f809daec8af42708693c05fe00cf 
>   
> ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java
>  21bdcf44436a02b11f878fa439e916d4b55ac63d 
>   ql/src/test/queries/clientpositive/set_processor_namespaces.q 
> 612807f0c871b1881446d088e1c2c399d1afe970 
>   ql/src/test/results/clientpositive/set_processor_namespaces.q.out 
> c05ce4d61d00a9ee6671d97f2fd178f18d44cc8c 
>   
> service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java
>  2dd90b69b3bf789b1a3928129cf801b17884033f 
> 
> 
> Diff: https://reviews.apache.org/r/60445/diff/4/
> 
> 
> Testing
> ---
> 
> Added new test case.
> Hand tested with Hue and Jdbc.
> 
> 
> Thanks,
> 
> Andrew Sherman
> 
>

Re: [DISCUSS] Separating out the metastore as its own TLP

[Sergio Pena]

Great, thanks Alan for putting all this in the email.
+1

Allowing other components to continue to use the Metastore without the need
to use Hive dependencies is a big plus for them. I agree with everything
you mention on the email.

- Sergio

On Fri, Jun 30, 2017 at 1:49 PM, Julian Hyde <jh...@apache.org> wrote:

> +1
>
> As a Calcite PMC member, I am very pleased to see this change. Calcite
> reads metadata from a variety of sources (including JDBC databases, NoSQL
> databases such as Cassandra and Druid, and streaming systems), and if more
> of those sources choose to store their metadata in the metastore it will
> make our lives easier.
>
> Hive’s metastore has established a position as the place to go for
> metadata in the Hadoop ecosystem. Not all metadata is relational, or
> processed by Hive, so there are other parties using the metastore who
> justifiably would like to influence its direction. Opening up the metastore
> will help retain and extend this position.
>
> Julian
>
>
> On 2017-06-30 10:00 (-0700), "Dimitris ts...@apache.org> wrote:
> >
> >
> > On 2017-06-30 07:56 (-0700), Alan Gates <al...@gmail.com> wrote: >
> > > A few of us have been talking and come to the conclussion that it
> would be>
> > > a good thing to split out the Hive metastore into its own Apache
> project.>
> > > Below and in the linked wiki page we explain what we see as the
> advantages>
> > > to this and how we would go about it.>
> > > >
> > > Hive’s metastore has long been used by other projects in the Hadoop>
> > > ecosystem to store and access metadata.  Apache Impala, Apache Spark,>
> > > Apache Drill, Presto, and other systems all use Hive’s metastore.
> Some,>
> > > like Impala and Presto can use it as their own metadata system with
> the>
> > > rest of Hive not present.>
> > > >
> > > This sharing is excellent for the ecosystem.  Together with HDFS it
> allows>
> > > users to use the tool of their choice while still accessing the same
> shared>
> > > data.  But having this shared metadata inside the Hive project limits
> the>
> > > ability of other projects to contribute to the metastore.  It also
> makes it>
> > > harder for new systems that have similar but not identical metadata>
> > > requirements (for example, stream processing systems on top of Apache>
> > > Kafka) to use Hive’s metastore.  This difficulty for other systems
> comes>
> > > out in two ways.  One, it is hard for non-Hive community members to>
> > > participate in the project.  Second, it adds operational cost since
> users>
> > > are forced to deploy all of the Hive jars just to get the metastore to
> work.>
> > > >
> > > Therefore we propose to split Hive’s metastore out into a separate
> Apache>
> > > project.  This new project will continue to support the same Thrift
> API as>
> > > the current metastore.  It will continue to focus on being a high>
> > > performance, fault tolerant, large scale, operational metastore for
> SQL>
> > > engines and other systems that want to store schema information about
> their>
> > > data.>
> > > >
> > > By making it a separate project we will enable other projects to join
> us in>
> > > innovating on the metastore.  It will simplify operations for non-Hive>
> > > users that want to use the metastore as they will no longer need to
> install>
> > > Hive just to get the metastore.  And it will attract new projects that>
> > > might otherwise feel the need to solve their metadata problems on
> their own.>
> > > >
> > > Any Hive PMC member or committer will be welcome to join the new
> project at>
> > > the same level.  We propose this project go straight to a top level>
> > > project.  Given that the initial PMC will be formed from experienced
> Hive>
> > > PMC members we do not believe incubation will be necessary.  (Note
> that the>
> > > Apache board will need to approve this.)>
> > > >
> > > Obviously there a many details involved in a proposal like this.
> Rather>
> > > than make this a ten page email we have filled out many of the details
> in a>
> > > wiki page:>
> > > https://cwiki.apache.org/confluence/display/Hive/
> Metastore+TLP+Proposal>
> > > >
> > > Yongzhi Chen>
> > > Vihang Karajgaonkar>
> > > Sergio Pena>
> > > Sahil Takiar>
> > > Aihua Xu>
> > > Gunther Hagleitner>
> > > Thejas Nair>
> > > Alan Gates>
> > > >
> >
> > +1 (from Apache Impala's (incubating) perspective)>
> >
> > Dimitris>
> >
>

Re: Review Request 59446: HIVE-16559: Parquet schema evolution for partitioned tables may break if table and partition serdes differ

[Sergio Pena]

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


Ship it!




Ship It!

- Sergio Pena


On June 26, 2017, 11:48 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59446/
> ---
> 
> (Updated June 26, 2017, 11:48 a.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Bugs: HIVE-16559
> https://issues.apache.org/jira/browse/HIVE-16559
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16559: Parquet schema evolution for partitioned tables may break if 
> table and partition serdes differ
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 87928ee930b5ee974d5e4144a584773a243f8d6f 
>   ql/src/test/queries/clientnegative/parquet_alter_part_table_drop_columns.q 
> PRE-CREATION 
>   
> ql/src/test/results/clientnegative/parquet_alter_part_table_drop_columns.q.out
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59446/diff/3/
> 
> 
> Testing
> ---
> 
> Added a negative qtest. Manually tested that no regression is caused for avro 
> and textfile SerDes when columns are added or replaced in a partitioned table.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 59446: HIVE-16559: Parquet schema evolution for partitioned tables may break if table and partition serdes differ

[Sergio Pena]

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




ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
Lines 463 (patched)
<https://reviews.apache.org/r/59446/#comment252832>

What's the difference with REPLACE_CANNOT_DROP_COLUMNS ? Seems they are use 
for the same error message.



ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
Lines 464 (patched)
<https://reviews.apache.org/r/59446/#comment252831>

Should we use CASCADE in capital case for better info that it is a keyword?


- Sergio Pena


On June 19, 2017, 9:52 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59446/
> ---
> 
> (Updated June 19, 2017, 9:52 a.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Bugs: HIVE-16559
> https://issues.apache.org/jira/browse/HIVE-16559
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16559: Parquet schema evolution for partitioned tables may break if 
> table and partition serdes differ
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java 
> 6651900e79a5c3d4ad8329afbe3894544ce9f46e 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 87928ee930b5ee974d5e4144a584773a243f8d6f 
>   ql/src/test/queries/clientnegative/parquet_alter_part_table_drop_columns.q 
> PRE-CREATION 
>   
> ql/src/test/results/clientnegative/parquet_alter_part_table_drop_columns.q.out
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59446/diff/2/
> 
> 
> Testing
> ---
> 
> Added a negative qtest. Manually tested that no regression is caused for avro 
> and textfile SerDes when columns are added or replaced in a partitioned table.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Hadoop3 + hbase2 support in Hive

[Sergio Pena]

Aihua,

There are a few of JIRAs related to the Hadoop3 support:

https://issues.apache.org/jira/browse/HIVE-16531 (This is the Umbrella for
Hadoop 3.x support)
https://issues.apache.org/jira/browse/HIVE-15016
https://issues.apache.org/jira/browse/HIVE-15427

I started the work on HIVE-15016 some time ago to run tests against the
alpha versions of Hadoop. This work was going to be in-progress for a while
until the Hadoop3 was released (to avoid committing patches for alpha
versions due to big changes happening between alpha releases and
instability).

I don't know how the rest of the community feels about this approach,
though. We could do something similar for Hbase.



On Thu, Jun 15, 2017 at 9:10 AM, Aihua Xu  wrote:

> Hi hive community,
>
> Hadoop and Hbase released new alpha versions. I'm wondering what's our plan
> to support those versions in Hive?
>
> Are we planning to use Hive3 (master) branch to support them and how
> typically do we start the upstream work for it?
>
> Thanks
> Aihua
>

Re: Review Request 60006: HIVE-14746: Remove branch and repositories from profiles by sending them from ptest-client

[Sergio Pena]

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



Thank you so much Zsombor for working on this patch. This is going to help us 
alot on running patches on newer branches without manual interaction to create 
a new profile.
There's still some work to do, but I'm glad we're starting to have progress on 
this.

Btw, looking at the code, I think you need to edit jenkins-execute-build.sh to 
send the branch name?


testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/PTestClient.java
Line 322 (original), 324 (patched)
<https://reviews.apache.org/r/60006/#comment251613>

Should we check that BRANCH is not empty before starting the test? 

Is this a required argument or optional?

If it is optional, what branch will use as default? We're removing the 
branch option from the profiles, so this will not exist.



testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java
Lines 108 (patched)
<https://reviews.apache.org/r/60006/#comment251614>

Why is a workingDirectoryWrapper needed?

Currently, Ptest works on a 'working' directory for any branch detected on 
the profile. Why do we want to change that for different branches?


- Sergio Pena


On June 13, 2017, 2:28 p.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60006/
> ---
> 
> (Updated June 13, 2017, 2:28 p.m.)
> 
> 
> Review request for hive, Sergio Pena and Siddharth Seth.
> 
> 
> Bugs: HIVE-14746
> https://issues.apache.org/jira/browse/HIVE-14746
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14746: Remove branch and repositories from profiles by sending them from 
> ptest-client
> 
> 
> Diffs
> -
> 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/client/PTestClient.java
>  8e2604d372ac29b94445b269f08423b058308efe 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/request/TestStartRequest.java
>  8deed52ae0307d4fc075654a4d75e6cb09a5d9db 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/api/server/TestExecutor.java
>  b2c61f03c5bf5f170894141848c89fc26129115a 
>   testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/PTest.java 
> 1cdfdb309acd8282e593abd7ed10c87721926c60 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/CloudExecutionContextProvider.java
>  8b82497bdaf43694e0e1552e125b5ffdce40f56c 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/ExecutionContext.java
>  b09de1d4d930cf2d4d26b500f3457cea3fffa9ce 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/FixedExecutionContextProvider.java
>  f7b50d6a61962d2727b5181215be9de2e64b05b7 
>   
> testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/WorkingDirWrapper.java
>  PRE-CREATION 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/api/server/TestTestExecutor.java
>  a4a789b579305d9ed573d8c1fd0b6ce75787d50f 
>   
> testutils/ptest2/src/test/java/org/apache/hive/ptest/execution/conf/TestTestConfiguration.java
>  848faf27af1ed8945d7013b6562bab544605e4bc 
> 
> 
> Diff: https://reviews.apache.org/r/60006/diff/2/
> 
> 
> Testing
> ---
> 
> Manually tested the PTestClient with and without the branch argument.
> Updated and ran the unit tests.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 59654: HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database

[Sergio Pena]

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


Ship it!




The patch looks good. I agree this needs more refactoring to make it better, 
but it is more work to do.
Let's ship this for now.

- Sergio Pena


On May 30, 2017, 9:43 p.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59654/
> ---
> 
> (Updated May 30, 2017, 9:43 p.m.)
> 
> 
> Review request for hive, Naveen Gangam, Sergio Pena, and Sahil Takiar.
> 
> 
> Bugs: HIVE-16771
> https://issues.apache.org/jira/browse/HIVE-16771
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore 
> schema version from database
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 
> a4ecc089f8a19bd94af2eae17e534cc6e8d2a9ca 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java 
> f312e46d755fc843369167af6d4da3de7dbe4a61 
>   beeline/src/test/org/apache/hive/beeline/TestHiveSchemaTool.java 
> 716bce7289b6e7c4d4bcea38b06f591a9ad9c098 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
> d1658f7bb2e5306b6b5954165759a217d1ba235c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreSchemaInfo.java 
> d6627433cb0f3f1c8fe868b3748fd754a7bff821 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 6bddb8e864d26a6cb7e2fe9b8b8becf427779944 
> 
> 
> Diff: https://reviews.apache.org/r/59654/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vihang Karajgaonkar
> 
>

Re: CVE-2016-3083: Apache Hive SSL vulnerability bug disclosure

[Sergio Pena]

Hi Vaibhav,

Do you happen to know which JIRA or patches addressed this issue?

- Sergio

On Wed, May 24, 2017 at 5:56 PM, Vaibhav Gumashta  wrote:

> Severity: Important
>
> Vendor: The Apache Software Foundation
>
> Versions Affected:
> Apache Hive 0.13.x
> Apache Hive 0.14.x
> Apache Hive 1.0.0 - 1.0.1
> Apache Hive 1.1.0 - 1.1.1
> Apache Hive 1.2.0 - 1.2.1
> Apache Hive 2.0.0
>
> Description:
>
> Apache Hive (JDBC + HiveServer2) implements SSL for plain TCP and HTTP
> connections (it supports both transport modes). While validating the
> server's certificate during the connection setup, the client doesn't seem
> to be verifying the common name attribute of the certificate. In this way,
> if a JDBC client sends an SSL request to server abc.com, and the server
> responds with a valid certificate (certified by CA) but issued to xyz.com,
> the client will accept that as a valid certificate and the SSL handshake
> will go through.
>
> Mitigation:
>
> Upgrade to Apache Hive 1.2.2 for 1.x release line, or to Apache Hive 2.0.1
> or later for 2.0.x release line, or to Apache Hive 2.1.0 and later for
> 2.1.x release line.
>
> Credit: This issue was discovered by Branden Crawford from Inteco Systems
> Limited (inetco.com).
>

Re: [Announce] New PMC members

[Sergio Pena]

Thanks all for your comments, and congratulations to all new PMC members
too !!!

- Sergio

On Sun, May 28, 2017 at 8:43 PM, Rajesh Balamohan 
wrote:

> Congratulations to new PMC members!
>
> ~Rajesh.B
>
> On Sat, May 27, 2017 at 12:14 PM, Zoltan Haindrich <
> zhaindr...@hortonworks.com> wrote:
>
> > Congratulations!
> >
> > On 27 May 2017 06:26, Lefty Leverenz  wrote:
> > Congratulations to the new PMC members, and also to Hive for having such
> a
> > strong and talented community!
> >
> > -- Lefty
> >
> >
> > On Fri, May 26, 2017 at 1:12 PM, Eugene Koifman <
> ekoif...@hortonworks.com>
> > wrote:
> >
> > > Congratualtions!
> > >
> > > On 5/26/17, 10:10 AM, "Prasanth Jayachandran" <
> > > pjayachand...@hortonworks.com> wrote:
> > >
> > > Congratulations to all of you!
> > >
> > > Thanks
> > > Prasanth
> > > > On May 26, 2017, at 10:09 AM, Pengcheng Xiong  >
> > > wrote:
> > > >
> > > > Yongzhi, Daniel, Vaibhav, Sergio, Aihua, and Chaoyu!
> > > >
> > > > Congratulations to all of you!
> > > >
> > > > Best
> > > > Pengcheng
> > > >
> > > > On Thu, May 25, 2017 at 10:26 PM, Peter Vary  >
> > > wrote:
> > > >
> > > >> Wow!
> > > >> That's a spring shower of PMCs. :)
> > > >> Well deserved Yongzhi, Daniel, Vaibhav, Sergio, Aihua, Chaoyu!
> > > >>
> > > >> Congratulations to all of you!
> > > >>
> > > >> Peter
> > > >>
> > > >> 2017. máj. 26. 6:42 ezt írta ("Ashutosh Chauhan" <
> > > hashut...@apache.org>):
> > > >>
> > > >> The Project Management Committee (PMC) for Apache Hive has
> invited
> > > Yongzhi
> > > >> Chen to become a PMC member and we are pleased to announce that
> he
> > > has
> > > >> accepted.
> > > >>
> > > >> Please join me in congratulating Yongzhi!
> > > >>
> > > >> Thanks,
> > > >> Ashutosh on behalf of Hive PMC
> > > >>
> > >
> > >
> > >
> > >
> >
> >
>

Re: Jimmy Xiang now a Hive PMC member

[Sergio Pena]

Congratulations Jimmy !!!

On Thu, May 25, 2017 at 2:30 AM, Peter Vary  wrote:

> Congratulations Jimmy!
>
> > On May 25, 2017, at 6:16 AM, Xuefu Zhang  wrote:
> >
> > Hi all,
> >
> > It's an honer to announce that Apache Hive PMC has recently voted to
> invite Jimmy Xiang as a new Hive PMC member. Please join me in
> congratulating him and looking forward to a bigger role that he will play
> in Apache Hive project.
> >
> > Thanks,
> > Xuefu
>
>

Re: Welcome Rui Li to Hive PMC

[Sergio Pena]

Congratulations Rui !!!

On Thu, May 25, 2017 at 4:44 AM, Rui Li  wrote:

> Thank you guys :)
>
> On Thu, May 25, 2017 at 3:29 PM, Peter Vary  wrote:
>
> > Congratulations Rui!
> >
> > > On May 25, 2017, at 6:19 AM, Xuefu Zhang  wrote:
> > >
> > > Hi all,
> > >
> > > It's an honer to announce that Apache Hive PMC has recently voted to
> > invite
> > > Rui Li as a new Hive PMC member. Rui is a long time Hive contributor
> and
> > > committer, and has made significant contribution in Hive especially in
> > Hive
> > > on Spark. Please join me in congratulating him and looking forward to a
> > > bigger role that he will play in Apache Hive project.
> > >
> > > Thanks,
> > > Xuefu
> >
> >
>
>
> --
> Best regards!
> Rui Li
> Cell: (+86) 13564950210
>

Re: [Announce] New committer: Vineet Garg

[Sergio Pena]

Congratulations Vinnet !!

On Tue, May 9, 2017 at 3:42 PM, Wei Zheng  wrote:

> Congrats!
>
> Thanks,
> Wei
>
> On 5/9/17, 13:36, "Gunther Hagleitner" 
> wrote:
>
> Congrats Vineet!
> 
> From: Ashutosh Chauhan 
> Sent: Tuesday, May 09, 2017 1:24 PM
> To: dev@hive.apache.org
> Subject: [Announce] New committer: Vineet Garg
>
> The Project Management Committee (PMC) for Apache Hive has invited
> Vineet
> Garg to become a committer and we are pleased to announce that he has
> accepted.
>
> Welcome, Vineet!
>
> Thanks,
> Ashutosh
>
>
>
>

Re: Review Request 58501: HIVE-16469: Parquet timestamp table property is not always taken into account

[Sergio Pena]

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


Ship it!




Ship It!

- Sergio Pena


On May 4, 2017, 10:19 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58501/
> ---
> 
> (Updated May 4, 2017, 10:19 a.m.)
> 
> 
> Review request for hive, Sergio Pena and Zoltan Ivanfi.
> 
> 
> Bugs: HIVE-16469
> https://issues.apache.org/jira/browse/HIVE-16469
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16469: Parquet timestamp table property is not always taken into account
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 757b7fc0eaa39c956014aa446ab1b07fc4abf8d3 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java 
> 13750cdc34711d22f2adf2f483a6773ad05fb8d2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/StatsNoJobTask.java 
> 9c3a664b9aea2d6e050ffe2d7626127827dbc52a 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 
> 1bd4db7805689ae1f91921ffbb5ff7da59f4bf60 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java
>  f4fadbb61bf45f62945700284c0b050f0984b696 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java 
> 2954601ce5bb25905cdb29ca0ca4551c2ca12b95 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java 
> 6413c5add6db2e8c9298285b15dba33ee74379a8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java 
> b339cc4347eea143dca2f6d98f9aaafdc427 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java 
> dbd6fb3d0bc8c753abf86e99b52377617f248b5a 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/AbstractTestParquetDirect.java
>  c81499a91c84af3ba33f335506c1c44e7085f13d 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetRowGroupFilter.java
>  bf363f32a3ac0a4d790e2925d802c6e210adfb4b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java
>  f2d79cf9d215e9a6e2a5e88cfc78378be860fd1f 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/timestamp/TestNanoTimeUtils.java
>  1e10dbf18742524982606f1e6c6d447d683b2dc3 
>   ql/src/test/queries/clientnegative/parquet_int96_alter_invalid_timezone.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/parquet_int96_create_invalid_timezone.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/parquet_int96_timestamp.q 
> 6eadd1b0a3313cbba7a798890b802baae302749e 
>   
> ql/src/test/results/clientnegative/parquet_int96_alter_invalid_timezone.q.out 
> PRE-CREATION 
>   
> ql/src/test/results/clientnegative/parquet_int96_create_invalid_timezone.q.out
>  PRE-CREATION 
>   ql/src/test/results/clientpositive/parquet_int96_timestamp.q.out 
> b9a3664458a83f1856e4bc59eba5d56665df61cc 
>   ql/src/test/results/clientpositive/spark/parquet_int96_timestamp.q.out 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58501/diff/5/
> 
> 
> Testing
> ---
> 
> Added qtests for the following cases:
> - order by clause
> - selfjoin
> - calling UDFs with the timestamp values
> - where clause with a constant cast as timestamp
> - test for HoS
> - implicit and explicit timestamp conversions in insert clause
> 
> Tested manually but no qtests:
> - join between 3 tables all parquet but with different/no timezone property
> - subselect in from/where clauses
> - exists / union / no exists
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: pre-commit jenkins issues

[Sergio Pena]

I restarted hiveptest and seems is working now. There was a hiccup on the
server while using the libraries to create the slave nodes.

On Fri, May 5, 2017 at 12:05 AM, Sushanth Sowmyan 
wrote:

> Hi,
>
> It looks like the precommit queue is currently having issues :
> https://builds.apache.org/job/PreCommit-HIVE-Build/
>
> See builds# 5041,5042,5043 - It looks like it takes about 8 hours
> waiting for the tests to finish running and to report back, and kills
> it as it exceeds a 500minute time out, and returns without results. Is
> anyone able to look into this to see what is going on?
>
> Thanks!
> -Sush
>

Re: [VOTE] Apache Hive 2.3.0 Release Candidate 0

[Sergio Pena]

Thanks Rui.

Pengcheng, the patch is reverted, you may continue with the RC1.

On Tue, May 2, 2017 at 11:02 PM, Rui Li <lirui.fu...@gmail.com> wrote:

> The patch has been reverted in master and branch-2.3
>
> On Wed, May 3, 2017 at 3:01 AM, Sergio Pena <sergio.p...@cloudera.com>
> wrote:
>
> > Hi Pengcheng,
> >
> > There is a request from the HDFS team to revert the patch committed on
> > HIVE-16047 from
> > our code because it might cause problems when future Hadoop versions are
> > released due to being a
> > private API on Hadoop. This API method signature has been changed between
> > releases, and
> > we don't want to have additional shims to support future Hadoop versions
> > just for this method.
> >
> > I'd like to revert it from 2.3.0 release before doing the release. It is
> > marked as being fixed on 2.2 but it is not cherry-picked on branch-2.2
> but
> > branch-2.3.
> >
> > Do you agree?
> >
> > - Sergio
> >
> > On Fri, Apr 28, 2017 at 1:40 PM, Pengcheng Xiong <pxi...@apache.org>
> > wrote:
> >
> > > Withdraw the VOTE on candidate 0. Will propose candidate 1 soon.
> Thanks.
> > >
> > > On Thu, Apr 27, 2017 at 8:10 PM, Owen O'Malley <owen.omal...@gmail.com
> >
> > > wrote:
> > >
> > > > -1 you need a release of storage-API first.
> > > >
> > > > .. Owen
> > > >
> > > > > On Apr 27, 2017, at 17:43, Pengcheng Xiong <pxi...@apache.org>
> > wrote:
> > > > >
> > > > > Apache Hive 2.3.0 Release Candidate 0 is available here:
> > > > > http://home.apache.org/~pxiong/apache-hive-2.3.0-rc0/
> > > > >
> > > > >
> > > > > Maven artifacts are available here:
> > > > > https://repository.apache.org/content/repositories/
> > orgapachehive-1073/
> > > > >
> > > > >
> > > > > Source tag for RC0 is at:
> > > > >
> > > > > https://github.com/apache/hive/releases/tag/release-2.3.0-rc0
> > > > >
> > > > > Voting will conclude in 72 hours.
> > > > >
> > > > > Hive PMC Members: Please test and vote.
> > > > >
> > > > > Thanks.
> > > >
> > >
> >
>
>
>
> --
> Best regards!
> Rui Li
> Cell: (+86) 13564950210
>

Re: [VOTE] Apache Hive 2.3.0 Release Candidate 0

[Sergio Pena]

Hi Pengcheng,

There is a request from the HDFS team to revert the patch committed on
HIVE-16047 from
our code because it might cause problems when future Hadoop versions are
released due to being a
private API on Hadoop. This API method signature has been changed between
releases, and
we don't want to have additional shims to support future Hadoop versions
just for this method.

I'd like to revert it from 2.3.0 release before doing the release. It is
marked as being fixed on 2.2 but it is not cherry-picked on branch-2.2 but
branch-2.3.

Do you agree?

- Sergio

On Fri, Apr 28, 2017 at 1:40 PM, Pengcheng Xiong  wrote:

> Withdraw the VOTE on candidate 0. Will propose candidate 1 soon. Thanks.
>
> On Thu, Apr 27, 2017 at 8:10 PM, Owen O'Malley 
> wrote:
>
> > -1 you need a release of storage-API first.
> >
> > .. Owen
> >
> > > On Apr 27, 2017, at 17:43, Pengcheng Xiong  wrote:
> > >
> > > Apache Hive 2.3.0 Release Candidate 0 is available here:
> > > http://home.apache.org/~pxiong/apache-hive-2.3.0-rc0/
> > >
> > >
> > > Maven artifacts are available here:
> > > https://repository.apache.org/content/repositories/orgapachehive-1073/
> > >
> > >
> > > Source tag for RC0 is at:
> > >
> > > https://github.com/apache/hive/releases/tag/release-2.3.0-rc0
> > >
> > > Voting will conclude in 72 hours.
> > >
> > > Hive PMC Members: Please test and vote.
> > >
> > > Thanks.
> >
>

Re: Review Request 58501: HIVE-16469: Parquet timestamp table property is not always taken into account

[Sergio Pena]

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




ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
Lines 333 (patched)
<https://reviews.apache.org/r/58501/#comment246597>

If getNextSplits() already sets the table property if it is not set, why 
are we doing it again here?



ql/src/java/org/apache/hadoop/hive/ql/exec/StatsNoJobTask.java
Lines 262 (patched)
<https://reviews.apache.org/r/58501/#comment246598>

I've seen this check a few times on the code. Shouldn't be good to create a 
static method that wraps this check? like 
ParquetHiveSerDe.isParquetTable(table)?



ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java
Line 72 (original), 72 (patched)
<https://reviews.apache.org/r/58501/#comment246603>

How does this work? I don't understand this change.



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java
Lines 72 (patched)
<https://reviews.apache.org/r/58501/#comment246601>

Can this new code be wrapped in another method?



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java
Line 181 (original), 181 (patched)
<https://reviews.apache.org/r/58501/#comment246600>

Is this compatible with old parquet tables? if the property is not set, 
then the validateTimeZonemight fail, right? If so, do we want to fail 
reading tables that do not have a property set?

Something else to consider, if a user sets a timezone improperly in a 
different tool or something  happened that we got an invalid timezone, then 
do we want to fail when reading those files? Just  wondering this scenario, 
no need to fix it right away.



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java
Lines 32 (patched)
<https://reviews.apache.org/r/58501/#comment246595>

Could you write the information about parameters?



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java
Lines 35 (patched)
<https://reviews.apache.org/r/58501/#comment246596>

Why is Map used instead of Map<String, String>? Aren't all table 
properties key, value string pairs?

Also, the ensureTablePropertySet() name seems not related to what we want 
to do. I thought it was going to throw an exception if the property was not 
set, but it is setting the value on the JobConf. Should we use a different 
name, such as setParquetTimeZoneIfNotSet(),  setParquetTimeZoneIfAbsent() 
or something like that helps us understand quickly without looking at the 
javadoc.



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java
Lines 168 (patched)
<https://reviews.apache.org/r/58501/#comment246594>

Shouldn't we throw an IllegalArgumentException here? The same for line 174?

I think it makes more sense to use the above exception when arguments are 
not valid.



ql/src/test/org/apache/hadoop/hive/ql/io/parquet/AbstractTestParquetDirect.java
Line 24 (original), 24 (patched)
<https://reviews.apache.org/r/58501/#comment246599>

Keep the standard here. Let's import each class instead of all.


- Sergio Pena


On April 20, 2017, 2:11 p.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58501/
> ---
> 
> (Updated April 20, 2017, 2:11 p.m.)
> 
> 
> Review request for hive, Sergio Pena and Zoltan Ivanfi.
> 
> 
> Bugs: HIVE-16469
> https://issues.apache.org/jira/browse/HIVE-16469
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16469: Parquet timestamp table property is not always taken into account
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 917e565f28b2c9aaea18033ea3b6b20fa41fcd0a 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java 
> 004bb2f60299a0635b8f9ca7649ead00b8e16d08 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/StatsNoJobTask.java 
> 9c3a664b9aea2d6e050ffe2d7626127827dbc52a 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 
> 1bd4db7805689ae1f91921ffbb5ff7da59f4bf60 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java
>  f4fadbb61bf45f62945700284c0b050f0984b696 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java 
> 2954601ce5bb25905cdb29ca0ca4551c2ca12b95 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java 
> b339cc4347eea143dca2f6d98f9aaafdc427 
>   
> ql/src/j

Re: ptest not running?

[Sergio Pena]

This seems a problem with the Jenkins slave resources. We'll need to wait
until Jenkins reserves a executor for Hive.

On Wed, Apr 26, 2017 at 3:18 PM, Vihang Karajgaonkar 
wrote:

> Does anyone have access to https://builds.apache.org/job/PreCommit-Admin/
> job? It is supposed to run every 10 min. It shows at the last build ran 10
> hours ago.
>
> On Wed, Apr 26, 2017 at 9:28 AM, Chaoyu Tang  wrote:
>
>> It looks the ptest is still having the problem. Either submitting patch in
>> JIRA or manual build could not trigger the tests.
>>
>> On Tue, Apr 25, 2017 at 11:31 PM, Eugene Koifman <
>> ekoif...@hortonworks.com>
>> wrote:
>>
>> > I see HIVE-16445 in it – it was picked up automatically
>> >
>> > On 4/25/17, 5:44 PM, "Vihang Karajgaonkar"  wrote:
>> >
>> > I see some builds queued up on the pre-commit job. Were these
>> submitted
>> > manually?
>> >
>> > On Tue, Apr 25, 2017 at 2:00 PM, Thejas Nair > >
>> > wrote:
>> >
>> > > Thanks for the update Vihang!
>> > >
>> > >
>> > > On Tue, Apr 25, 2017 at 12:54 PM, Vihang Karajgaonkar <
>> > vih...@cloudera.com
>> > > >
>> > > wrote:
>> > >
>> > > > This seems to be related to a larger infrastructure issue. Other
>> > projects
>> > > > (hadoop, sentry, hbase etc) are facing similar issues as well.
>> > > >
>> > > > https://issues.apache.org/jira/browse/INFRA-13985
>> > > >
>> > > > On Tue, Apr 25, 2017 at 11:54 AM, Thejas Nair <
>> > thejas.n...@gmail.com>
>> > > > wrote:
>> > > >
>> > > > > Looks like nothing is getting scheduled since yesterday
>> evening,
>> > even
>> > > > with
>> > > > > the manual launch
>> > > > >
>> > > > > https://builds.apache.org/job/PreCommit-HIVE-Build/
>> > > > >
>> > > > > Can someone with access please take a look ?
>> > > > >
>> > > > >
>> > > > > On Mon, Apr 24, 2017 at 5:26 PM, Eugene Koifman <
>> > > > ekoif...@hortonworks.com>
>> > > > > wrote:
>> > > > >
>> > > > > > Something is not working with the part of the flow that
>> > triggers the
>> > > > > build
>> > > > > > automatically.
>> > > > > > In the meantime, if you log in with apache credentials, and
>> go
>> > to
>> > > > “Build
>> > > > > > with Parameters”
>> > > > > > you can manually launch the test by entering the bug number:
>> > just the
>> > > > > > digits, skip the “HIVE-“ part
>> > > > > >
>> > > > > >
>> > > > > > On 4/24/17, 5:01 PM, "Vihang Karajgaonkar" <
>> > vih...@cloudera.com>
>> > > > wrote:
>> > > > > >
>> > > > > > I see that builds are getting picked up at
>> > > > > https://builds.apache.org/
>> > > > > > job/PreCommit-HIVE-Build/ The PTest server was
>> restarted a
>> > couple
>> > > > of
>> > > > > > days
>> > > > > > back. If you had a patch in the queue around that time,
>> > may be it
>> > > > was
>> > > > > > lost
>> > > > > > due to restart. You may have to re-attach the patch on
>> the
>> > > upstream
>> > > > > > JIRA if
>> > > > > > that is the case.
>> > > > > >
>> > > > > >
>> > > > > > On Mon, Apr 24, 2017 at 2:44 PM, Eugene Koifman <
>> > > > > > ekoif...@hortonworks.com>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Hi,
>> > > > > > > It is not picking up new patches and there are none
>> > queued up.
>> > > > > Could
>> > > > > > > someone check please?
>> > > > > > >
>> > > > > > > Thanks,
>> > > > > > > Eugene
>> > > > > > >
>> > > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>> >
>> >
>>
>
>

Re: Review Request 57353: Intern Properties objects referenced from PartitionDesc to reduce memory pressure.

[Sergio Pena]

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


Ship it!




Ship It!

- Sergio Pena


On April 21, 2017, 8:26 p.m., Misha Dmitriev wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57353/
> ---
> 
> (Updated April 21, 2017, 8:26 p.m.)
> 
> 
> Review request for hive, Chaozhong Yang, Alan Gates, Rui Li, Prasanth_J, 
> Sergio Pena, Sahil Takiar, Vihang Karajgaonkar, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-16079
> https://issues.apache.org/jira/browse/HIVE-16079
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> When multiple concurrent Hive queries run, a separate copy of
> org.apache.hadoop.hive.ql.metadata.Partition and
> ql.plan.PartitionDesc is created for each table partition
> per each query instance. So when in my benchmark explained in
> HIVE-16079 we have 2000 partitions and 50 concurrent queries running
> over them, we end up, in the worst case, with 2000*50=100,000 instances
> of Partition and PartitionDesc in memory. These objects themselves
> collectively take just ~2% of memory. However, other data structures
> that each of them reference, take a lot more. In particular, Properties
> objects take more than 20% of memory. When we have 50 concurrent
> read-only queries, there are 50 identical copies of Properties per
> each partition. That's a huge waste of memory.
> 
> This change introduces a new class that extends Properties, called
> CopyOnFirstWriteProperties. It utilizes a unique interned copy of
> Properties whenever possible. However, when one of the methods that
> modify properties is called, a copy is created. When this class is
> used, memory consumption by Properties falls from 20% to 5..6%.
> 
> 
> Diffs
> -
> 
>   
> common/src/java/org/apache/hadoop/hive/common/CopyOnFirstWriteProperties.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/SerializationUtilities.java 
> 247d5890ea8131404b9543d22876ca4c052578e0 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionDesc.java 
> d05c1c68fdb7296c0346d73967071da1ebe7bb72 
> 
> 
> Diff: https://reviews.apache.org/r/57353/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Misha Dmitriev
> 
>

Re: Review Request 57353: Intern Properties objects referenced from PartitionDesc to reduce memory pressure.

[Sergio Pena]

> On April 21, 2017, 4:35 p.m., Sergio Pena wrote:
> > Misha, whers is CopyOnFirstWriteProperties used? The patch looks pretty 
> > good, but I don't see where CopyOnFirstWriteProperties is instatiated.
> 
> Misha Dmitriev wrote:
> It's not instantiated directly. Rather, see the 
> serialization/deserialization code in SerializationUtilities.java, where this 
> class is indirectly instantiated. My understanding is that this is how 
> Partitions and their child data structures are created, by transferring data 
> from HMS.

I still not found how this happens. Could you describe how you understand this 
happens? Maybe I can follow you better than the code.


> On April 21, 2017, 4:35 p.m., Sergio Pena wrote:
> > common/src/java/org/apache/hadoop/hive/common/CopyOnFirstWriteProperties.java
> > Lines 314 (patched)
> > <https://reviews.apache.org/r/57353/diff/1/?file=1656856#file1656856line314>
> >
> > If Interners.newWeakInterner() returns a thread-safe interner, why do 
> > we have to lock the INTERNER only when updating it?
> 
> Misha Dmitriev wrote:
> I don't think newWeakInterner() returns a thread-safe interner, at least 
> from inspecting its code?

I read the javadoc that states that:

newWeakInterner()
Returns a new thread-safe interner which retains a weak reference to each 
instance it has interned, and so does not prevent these instances from being 
garbage-collected.


- Sergio


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


On March 7, 2017, 1:22 a.m., Misha Dmitriev wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57353/
> ---
> 
> (Updated March 7, 2017, 1:22 a.m.)
> 
> 
> Review request for hive, Chaozhong Yang, Alan Gates, Rui Li, Prasanth_J, 
> Sergio Pena, Sahil Takiar, Vihang Karajgaonkar, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-16079
> https://issues.apache.org/jira/browse/HIVE-16079
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> When multiple concurrent Hive queries run, a separate copy of
> org.apache.hadoop.hive.ql.metadata.Partition and
> ql.plan.PartitionDesc is created for each table partition
> per each query instance. So when in my benchmark explained in
> HIVE-16079 we have 2000 partitions and 50 concurrent queries running
> over them, we end up, in the worst case, with 2000*50=100,000 instances
> of Partition and PartitionDesc in memory. These objects themselves
> collectively take just ~2% of memory. However, other data structures
> that each of them reference, take a lot more. In particular, Properties
> objects take more than 20% of memory. When we have 50 concurrent
> read-only queries, there are 50 identical copies of Properties per
> each partition. That's a huge waste of memory.
> 
> This change introduces a new class that extends Properties, called
> CopyOnFirstWriteProperties. It utilizes a unique interned copy of
> Properties whenever possible. However, when one of the methods that
> modify properties is called, a copy is created. When this class is
> used, memory consumption by Properties falls from 20% to 5..6%.
> 
> 
> Diffs
> -
> 
>   
> common/src/java/org/apache/hadoop/hive/common/CopyOnFirstWriteProperties.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/SerializationUtilities.java 
> 247d5890ea8131404b9543d22876ca4c052578e0 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionDesc.java 
> d05c1c68fdb7296c0346d73967071da1ebe7bb72 
> 
> 
> Diff: https://reviews.apache.org/r/57353/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Misha Dmitriev
> 
>

Re: ptest is taking 8h

[Sergio Pena]

Done. There was an issue with some permissions I changed yesterday on the
hiveptest ssh key. The server is up and running now.

On Fri, Apr 21, 2017 at 1:34 PM, Sergio Pena <sergio.p...@cloudera.com>
wrote:

> I'm taking a look at this now. There is something wrong on the server.
>
> On Fri, Apr 21, 2017 at 10:27 AM, Vihang Karajgaonkar <vih...@cloudera.com
> > wrote:
>
>> Looks like PtestServer is not responding.
>>
>> org.apache.hive.ptest.api.client.PTestClient --command testStart
>> --outputDir
>> /home/jenkins/jenkins-slave/workspace/PreCommit-HIVE-Build/
>> hive/build/hive/testutils/ptest2/target
>> --password '[***]' --testHandle PreCommit-HIVE-Build-4795 --endpoint
>> http://104.198.109.242:8080/hive-ptest-1.0 --logsEndpoint
>> http://104.198.109.242/logs/ --profile master-mr2 --patch
>> https://issues.apache.org/jira/secure/attachment/12864318/
>> HIVE-16483.1.patch
>> --jira
>> <https://issues.apache.org/jira/secure/attachment/12864318/HIVE-16483.1.patch--jira>
>> HIVE-16483
>>
>> Build timed out (after 500 minutes). Marking the build as aborted.
>> Build was aborted
>> + ret=143
>> + unpack_test_results
>> + '[' -z /home/jenkins/jenkins-slave/workspace/PreCommit-HIVE-Build/
>> hive/build
>> ']'
>> + cd /home/jenkins/jenkins-slave/workspace/PreCommit-HIVE-Build/
>> hive/build/hive/testutils/ptest2/target
>> Recording test results
>> + [[ -f test-results.tar.gz ]]
>>
>> + exit 143
>>
>> On Fri, Apr 21, 2017 at 8:20 AM, Eugene Koifman <ekoif...@hortonworks.com
>> >
>> wrote:
>>
>> > Hi,
>> > The last few runs are very slow.  Does anyone know what can be done?
>> >
>> > https://builds.apache.org/job/PreCommit-HIVE-Build/
>> >
>> > Thanks,
>> > Eugene
>> >
>>
>
>

Re: ptest is taking 8h

[Sergio Pena]

I'm taking a look at this now. There is something wrong on the server.

On Fri, Apr 21, 2017 at 10:27 AM, Vihang Karajgaonkar 
wrote:

> Looks like PtestServer is not responding.
>
> org.apache.hive.ptest.api.client.PTestClient --command testStart
> --outputDir
> /home/jenkins/jenkins-slave/workspace/PreCommit-HIVE-
> Build/hive/build/hive/testutils/ptest2/target
> --password '[***]' --testHandle PreCommit-HIVE-Build-4795 --endpoint
> http://104.198.109.242:8080/hive-ptest-1.0 --logsEndpoint
> http://104.198.109.242/logs/ --profile master-mr2 --patch
> https://issues.apache.org/jira/secure/attachment/
> 12864318/HIVE-16483.1.patch
> --jira HIVE-16483
>
> Build timed out (after 500 minutes). Marking the build as aborted.
> Build was aborted
> + ret=143
> + unpack_test_results
> + '[' -z /home/jenkins/jenkins-slave/workspace/PreCommit-HIVE-
> Build/hive/build
> ']'
> + cd /home/jenkins/jenkins-slave/workspace/PreCommit-HIVE-
> Build/hive/build/hive/testutils/ptest2/target
> Recording test results
> + [[ -f test-results.tar.gz ]]
>
> + exit 143
>
> On Fri, Apr 21, 2017 at 8:20 AM, Eugene Koifman 
> wrote:
>
> > Hi,
> > The last few runs are very slow.  Does anyone know what can be done?
> >
> > https://builds.apache.org/job/PreCommit-HIVE-Build/
> >
> > Thanks,
> > Eugene
> >
>

Re: Review Request 57353: Intern Properties objects referenced from PartitionDesc to reduce memory pressure.

[Sergio Pena]

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



Misha, whers is CopyOnFirstWriteProperties used? The patch looks pretty good, 
but I don't see where CopyOnFirstWriteProperties is instatiated.


common/src/java/org/apache/hadoop/hive/common/CopyOnFirstWriteProperties.java
Lines 314 (patched)
<https://reviews.apache.org/r/57353/#comment245727>

If Interners.newWeakInterner() returns a thread-safe interner, why do we 
have to lock the INTERNER only when updating it?


- Sergio Pena


On March 7, 2017, 1:22 a.m., Misha Dmitriev wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57353/
> ---
> 
> (Updated March 7, 2017, 1:22 a.m.)
> 
> 
> Review request for hive, Chaozhong Yang, Alan Gates, Rui Li, Prasanth_J, 
> Sergio Pena, Sahil Takiar, Vihang Karajgaonkar, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-16079
> https://issues.apache.org/jira/browse/HIVE-16079
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> When multiple concurrent Hive queries run, a separate copy of
> org.apache.hadoop.hive.ql.metadata.Partition and
> ql.plan.PartitionDesc is created for each table partition
> per each query instance. So when in my benchmark explained in
> HIVE-16079 we have 2000 partitions and 50 concurrent queries running
> over them, we end up, in the worst case, with 2000*50=100,000 instances
> of Partition and PartitionDesc in memory. These objects themselves
> collectively take just ~2% of memory. However, other data structures
> that each of them reference, take a lot more. In particular, Properties
> objects take more than 20% of memory. When we have 50 concurrent
> read-only queries, there are 50 identical copies of Properties per
> each partition. That's a huge waste of memory.
> 
> This change introduces a new class that extends Properties, called
> CopyOnFirstWriteProperties. It utilizes a unique interned copy of
> Properties whenever possible. However, when one of the methods that
> modify properties is called, a copy is created. When this class is
> used, memory consumption by Properties falls from 20% to 5..6%.
> 
> 
> Diffs
> -
> 
>   
> common/src/java/org/apache/hadoop/hive/common/CopyOnFirstWriteProperties.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/SerializationUtilities.java 
> 247d5890ea8131404b9543d22876ca4c052578e0 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionDesc.java 
> d05c1c68fdb7296c0346d73967071da1ebe7bb72 
> 
> 
> Diff: https://reviews.apache.org/r/57353/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Misha Dmitriev
> 
>

Re: Review Request 58516: HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception

[Sergio Pena]

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


Ship it!




It looks good.

- Sergio Pena


On April 18, 2017, 7:15 p.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58516/
> ---
> 
> (Updated April 18, 2017, 7:15 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
> 
> 
> Bugs: HIVE-16213
> https://issues.apache.org/jira/browse/HIVE-16213
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an 
> exception
> 
> 
> Diffs
> -
> 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 6b217516bc74c612348b8edeca077dfbdbdb1a40 
> 
> 
> Diff: https://reviews.apache.org/r/58516/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vihang Karajgaonkar
> 
>

Re: [VOTE] Apache Hive 1.2.2 Release Candidate 0

[Sergio Pena]

+1 (no-binding)

I unpacked the bin and src packages.
Verified gpg and md5 signatures.
Check license and release notes files.
Run a few queries from hive-cli.

- Sergio

On Tue, Apr 4, 2017 at 11:12 AM, Ashutosh Chauhan 
wrote:

> Verified md5 of src and binary tar balls.
> Built from src.
> Ran some simple queries like join, group by.
> All looks good.
>
> +1
>
> Thanks,
> Ashutosh
>
> On Mon, Apr 3, 2017 at 4:47 PM, Vaibhav Gumashta <
> vgumas...@hortonworks.com>
> wrote:
>
> > Thanks for pointing out Ashutosh. Link to my PGP key:
> > http://pgp.mit.edu/pks/lookup?search=gumashta=index.
> >
> > I think it will take a day or so for the KEYS file to be updated (it is
> > auto generated), but if you want to test the release in the meantime,
> > please use the above link to access the signing key.
> >
> > Thanks,
> > ‹Vaibhav
> >
> > On 4/3/17, 2:53 PM, "Ashutosh Chauhan"  wrote:
> >
> > >Hi Vaibhav,
> > >
> > >Can't locate your key at any of standard location. Can you point out
> which
> > >key you used to sign the release?
> > >
> > >Thanks,
> > >Ashutosh
> > >
> > >On Mon, Apr 3, 2017 at 12:51 AM, Vaibhav Gumashta
> > > > >> wrote:
> > >> Hi everyone,
> > >>
> > >> Apache Hive 1.2.2 Release Candidate 0 is available here:
> > >>
> > >> https://dist.apache.org/repos/dist/dev/hive/apache-hive-1.2.2-rc0/
> > >>
> > >> Maven artifacts are available here:
> > >>
> > >> https://repository.apache.org/content/repositories/
> orgapachehive-1072/
> > >>
> > >> Source tag for RC0 is at:
> > >> https://github.com/apache/hive/releases/tag/release-1.2.2-rc0
> > >>
> > >> Voting will conclude in 72 hours.
> > >>
> > >> Hive PMC Members: Please test and vote.
> > >>
> > >> Thanks,
> > >> -Vaibhav
> > >>
> > >>
> >
> >
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

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

(Updated April 3, 2017, 10:34 p.m.)


Review request for hive.


Changes
---

Address Mohit changes.


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


Repository: hive-git


Description
---

This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
property from withing the DbNotificationListener class. It then passes the 
EnvironmentContext from transactional listeners to non-transactional listeners 
so that the eventId is shared between them.

The patch provides the following changes:
- DbNotificationListener   Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- HiveAlterHandler Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- MetaStoreListenerNotifierNew helper class that wraps the notification 
call to the listeners.
- TestObjectStore  Verifies that the addNotificationEvent() method 
saves the eventId on the NotificationEvent object.
- TestDbNotificationListener   Verifies that any HMS call is passing the 
DB_NOTIFICATION_EVENT_ID to non-transactional listeners.


Diffs (updated)
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 ea6cb790bc03fc38be07a3be7be34aa9c5d293ba 
  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
 PRE-CREATION 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
4ce6a659c94265d83abbaedb9c52d7e15bf8dde6 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
80b1e98b9efbf068f0cd32303499a3a9b5bbd67b 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
1f87eeb18f6edf7351b3c8da6a6826c08656e48c 


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

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


Testing
---

HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
complete 100% tests.


Thanks,

Sergio Pena

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

> On March 30, 2017, 1:54 p.m., Mohit Sabharwal wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java
> > Lines 51-52 (patched)
> > <https://reviews.apache.org/r/57626/diff/6/?file=1677663#file1677663line51>
> >
> > What's the point of have duplicate parameter map ? What is the use case 
> > ?
> > 
> > If you need unmodifiable paramters, why can't you add a new method 
> > called getunmodifiableParameters() which simply return 
> > Collections.unmodifiableMap(parameters) ?
> > 
> > Do you really need to call updateUnmodifiableParameters() everytime ? 
> > Looks like you're only updating 'parameters' variable, but then creating a 
> > new updateUnmodifiableParameters every time a new items is added. 
> > 
> > As I said, you can simply add a new method called 
> > getunmodifiableParameters() which returns 
> > Collections.unmodifiableMap(parameters) without the need for this copy.
> 
> Sergio Pena wrote:
> We want to avoid that an external listener gets a reference to the 
> mutable map object and override any key/value already set by other listeners. 
> Part of the contract on the ListenerEvent.putParameter() is that you cannot 
> override an existing key/value. Because we don't control the external 
> listeners, we prefered to return an inmutable map whenever getParameters() 
> was called.
> 
> Does it make sense?
> 
> Mohit Sabharwal wrote:
> That makes complete sense. But why do you need two explicit references to 
> the same map stored as class members ? You could just have one reference 
> called 'parameters' and add a getter (called, say, getunmodifiableParameters) 
> that returns Collections.unmodifiableMap(parameters).
> 
> Sergio Pena wrote:
> I did that to avoid having a "live" unmodifiable map in case the 
> ListenerEvent was called by multiple threads. But, I don't think that would 
> happen. I even documented the class to state that this was not supposed to be 
> called by multiple threads. So, I think it make sense just using 
> Collections.unmodifiableMap(parameters) directly  to avoid making a copy of 
> the mutable reference for every new key/value pair.

Mohit, forget what I said on the above response. I had in my mind I was doing a 
copy of mutable map values to inmutable maps, but I see I am only wrapping the 
mutable map into the inmutable map.

Anyways, this is what Sasha commented last time when I had the following code:

public final Map<String, String> getParameters() {
   return Collections.unmodifiableMap(parameters);
}

Sasha comment:
This is fine conceptually, but I'm a bit concerned about performance - 
there are many gets (for each call to notify()) and each one has to create a 
new copy of the map.
If you want to preserve the code structure (many getters), I would suggest 
caching an unmodifiable map when it is updated since updates are rare.

So far we only have 1 put on the DbNotificationListener. After that, many 
events can call the getParameters(), and we won't know how they handle that, if 
storing a reference of the inmutable parameters once, or if they like to do 
'event.getParameters().get(key)' many times. I thing that's what Sasha tried to 
avoid.


- Sergio


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


On March 28, 2017, 3:17 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57626/
> ---
> 
> (Updated March 28, 2017, 3:17 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-16164
> https://issues.apache.org/jira/browse/HIVE-16164
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
> property from withing the DbNotificationListener class. It then passes the 
> EnvironmentContext from transactional listeners to non-transactional 
> listeners so that the eventId is shared between them.
> 
> The patch provides the following changes:
> - DbNotificationListener   Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - HiveAlterHandler Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - MetaStoreListenerNotifierNew helper class that wraps the notification 
> call to the listeners.
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

> On March 30, 2017, 1:54 p.m., Mohit Sabharwal wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java
> > Lines 51-52 (patched)
> > <https://reviews.apache.org/r/57626/diff/6/?file=1677663#file1677663line51>
> >
> > What's the point of have duplicate parameter map ? What is the use case 
> > ?
> > 
> > If you need unmodifiable paramters, why can't you add a new method 
> > called getunmodifiableParameters() which simply return 
> > Collections.unmodifiableMap(parameters) ?
> > 
> > Do you really need to call updateUnmodifiableParameters() everytime ? 
> > Looks like you're only updating 'parameters' variable, but then creating a 
> > new updateUnmodifiableParameters every time a new items is added. 
> > 
> > As I said, you can simply add a new method called 
> > getunmodifiableParameters() which returns 
> > Collections.unmodifiableMap(parameters) without the need for this copy.
> 
> Sergio Pena wrote:
> We want to avoid that an external listener gets a reference to the 
> mutable map object and override any key/value already set by other listeners. 
> Part of the contract on the ListenerEvent.putParameter() is that you cannot 
> override an existing key/value. Because we don't control the external 
> listeners, we prefered to return an inmutable map whenever getParameters() 
> was called.
> 
> Does it make sense?
> 
> Mohit Sabharwal wrote:
> That makes complete sense. But why do you need two explicit references to 
> the same map stored as class members ? You could just have one reference 
> called 'parameters' and add a getter (called, say, getunmodifiableParameters) 
> that returns Collections.unmodifiableMap(parameters).

I did that to avoid having a "live" unmodifiable map in case the ListenerEvent 
was called by multiple threads. But, I don't think that would happen. I even 
documented the class to state that this was not supposed to be called by 
multiple threads. So, I think it make sense just using 
Collections.unmodifiableMap(parameters) directly  to avoid making a copy of the 
mutable reference for every new key/value pair.


- Sergio


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


On March 28, 2017, 3:17 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57626/
> ---
> 
> (Updated March 28, 2017, 3:17 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-16164
> https://issues.apache.org/jira/browse/HIVE-16164
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
> property from withing the DbNotificationListener class. It then passes the 
> EnvironmentContext from transactional listeners to non-transactional 
> listeners so that the eventId is shared between them.
> 
> The patch provides the following changes:
> - DbNotificationListener   Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - HiveAlterHandler Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - MetaStoreListenerNotifierNew helper class that wraps the notification 
> call to the listeners.
> - TestObjectStore  Verifies that the addNotificationEvent() 
> method saves the eventId on the NotificationEvent object.
> - TestDbNotificationListener   Verifies that any HMS call is passing the 
> DB_NOTIFICATION_EVENT_ID to non-transactional listeners.
> 
> 
> Diffs
> -
> 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  ea6cb790bc03fc38be07a3be7be34aa9c5d293ba 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
>  PRE-CREATION 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
> 4ce6a659c94265d83abbaedb9c52d7e15bf8dde6 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 80b1e98b9efbf068f0cd32303499a3a9b5bbd67b 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifi

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

> On March 30, 2017, 1:54 p.m., Mohit Sabharwal wrote:
> > hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
> > Lines 487 (patched)
> > <https://reviews.apache.org/r/57626/diff/6/?file=1677657#file1677657line487>
> >
> > I didn't mean write a new method called isSetEventId. I meant that 
> > NotificationEvent already has a public method called isSetEventId. You can 
> > use that.
> > 
> > My basic objection is that you cannot use eventId > 0 as a proxy for 
> > whether eventid is set or not, because 0 can be a valid event id if the 
> > code ObjectStore#addNotificationEvent is changed. (The eventid > 0 
> > assumption depends on code in ObjectStore#addNotificationEvent remaining 
> > same, which is not a safe assumption).

Thanks. I didn't know the NotificationEvent had the isSetEventId(). That makes 
more sense as it checks if it was already set with any value.


> On March 30, 2017, 1:54 p.m., Mohit Sabharwal wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java
> > Lines 51-52 (patched)
> > <https://reviews.apache.org/r/57626/diff/6/?file=1677663#file1677663line51>
> >
> > What's the point of have duplicate parameter map ? What is the use case 
> > ?
> > 
> > If you need unmodifiable paramters, why can't you add a new method 
> > called getunmodifiableParameters() which simply return 
> > Collections.unmodifiableMap(parameters) ?
> > 
> > Do you really need to call updateUnmodifiableParameters() everytime ? 
> > Looks like you're only updating 'parameters' variable, but then creating a 
> > new updateUnmodifiableParameters every time a new items is added. 
> > 
> > As I said, you can simply add a new method called 
> > getunmodifiableParameters() which returns 
> > Collections.unmodifiableMap(parameters) without the need for this copy.

We want to avoid that an external listener gets a reference to the mutable map 
object and override any key/value already set by other listeners. Part of the 
contract on the ListenerEvent.putParameter() is that you cannot override an 
existing key/value. Because we don't control the external listeners, we 
prefered to return an inmutable map whenever getParameters() was called.

Does it make sense?


- Sergio


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


On March 28, 2017, 3:17 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57626/
> ---
> 
> (Updated March 28, 2017, 3:17 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-16164
> https://issues.apache.org/jira/browse/HIVE-16164
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
> property from withing the DbNotificationListener class. It then passes the 
> EnvironmentContext from transactional listeners to non-transactional 
> listeners so that the eventId is shared between them.
> 
> The patch provides the following changes:
> - DbNotificationListener   Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - HiveAlterHandler Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - MetaStoreListenerNotifierNew helper class that wraps the notification 
> call to the listeners.
> - TestObjectStore  Verifies that the addNotificationEvent() 
> method saves the eventId on the NotificationEvent object.
> - TestDbNotificationListener   Verifies that any HMS call is passing the 
> DB_NOTIFICATION_EVENT_ID to non-transactional listeners.
> 
> 
> Diffs
> -
> 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  ea6cb790bc03fc38be07a3be7be34aa9c5d293ba 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
>  PRE-CREATION 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
> 4ce6a659c94265d83abbaedb9c52d7e15bf8dde6 
>   metastore/src/java/org/apache/had

Re: Review Request 57805: HIVE-16049: upgrade to jetty 9

[Sergio Pena]

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


Ship it!




Ship It!

- Sergio Pena


On March 29, 2017, 2:15 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57805/
> ---
> 
> (Updated March 29, 2017, 2:15 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16049: upgrade to jetty 9
> 
> 
> Diffs
> -
> 
>   common/pom.xml 8474a87 
>   common/src/java/org/apache/hive/http/HttpServer.java db5650d 
>   hcatalog/pom.xml 34de177 
>   hcatalog/webhcat/svr/pom.xml c5ad387 
>   
> hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/Main.java
>  5208bf4 
>   llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapServiceDriver.java 
> 22e5ee8 
>   pom.xml 4dd5262 
>   service/pom.xml 9306739 
>   
> service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 
> ebec165 
>   service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java 
> 4d50dd9 
> 
> 
> Diff: https://reviews.apache.org/r/57805/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

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

(Updated March 28, 2017, 3:17 p.m.)


Review request for hive.


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


Repository: hive-git


Description
---

This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
property from withing the DbNotificationListener class. It then passes the 
EnvironmentContext from transactional listeners to non-transactional listeners 
so that the eventId is shared between them.

The patch provides the following changes:
- DbNotificationListener   Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- HiveAlterHandler Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- MetaStoreListenerNotifierNew helper class that wraps the notification 
call to the listeners.
- TestObjectStore  Verifies that the addNotificationEvent() method 
saves the eventId on the NotificationEvent object.
- TestDbNotificationListener   Verifies that any HMS call is passing the 
DB_NOTIFICATION_EVENT_ID to non-transactional listeners.


Diffs (updated)
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 ea6cb790bc03fc38be07a3be7be34aa9c5d293ba 
  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
 PRE-CREATION 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
4ce6a659c94265d83abbaedb9c52d7e15bf8dde6 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
80b1e98b9efbf068f0cd32303499a3a9b5bbd67b 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
1f87eeb18f6edf7351b3c8da6a6826c08656e48c 


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

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


Testing
---

HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
complete 100% tests.


Thanks,

Sergio Pena

Re: Review Request 57728: HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767

[Sergio Pena]

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


Ship it!




Ship It!

- Sergio Pena


On March 27, 2017, 8 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57728/
> ---
> 
> (Updated March 27, 2017, 8 a.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
>  26f1e75c7d659a634cd4eef3a0cb8e886b22722f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java 
> 8e33b7d437894b33b35f32913a3bc02f2a849ce3 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java 
> 5dc808800290f3274afbdff12134ac34387a746b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/timestamp/TestNanoTimeUtils.java
>  37cf0e2d74589cfa97fa24c9d2d8d00ea62390ee 
>   ql/src/test/queries/clientpositive/parquet_int96_timestamp.q 
> 5de2c3f1244b8340b97eb0547fe66e52d80fb065 
> 
> 
> Diff: https://reviews.apache.org/r/57728/diff/4/
> 
> 
> Testing
> ---
> 
> Tested loading timestamps from a parquet file written by spark.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 57805: HIVE-16049: upgrade to jetty 9

[Sergio Pena]

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




common/src/java/org/apache/hive/http/HttpServer.java
Lines 101-108 (patched)
<https://reviews.apache.org/r/57805/#comment242983>

Wouldn't be good to wrap this code into a private method that creates the 
server? It will make the constructor cleaner if we see this:

webServer = createServer(b);
appDir = getWebAppsPath(b.name);
webAppContext = createWebAppContext(b);



common/src/java/org/apache/hive/http/HttpServer.java
Line 393 (original), 402 (patched)
<https://reviews.apache.org/r/57805/#comment242986>

Should we declare a final static variable for the 200 number? probably add 
a comment why 200 is a good value? was it unlimited before?


- Sergio Pena


On March 27, 2017, 4:31 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57805/
> ---
> 
> (Updated March 27, 2017, 4:31 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16049: upgrade to jetty 9
> 
> 
> Diffs
> -
> 
>   common/pom.xml 8474a87 
>   common/src/java/org/apache/hive/http/HttpServer.java db5650d 
>   hcatalog/pom.xml 34de177 
>   hcatalog/webhcat/svr/pom.xml c5ad387 
>   
> hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/Main.java
>  5208bf4 
>   llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapServiceDriver.java 
> 22e5ee8 
>   pom.xml 4c73e27 
>   service/pom.xml 9306739 
>   
> service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 
> ebec165 
>   service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java 
> 4d50dd9 
> 
> 
> Diff: https://reviews.apache.org/r/57805/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

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

(Updated March 24, 2017, 10:23 p.m.)


Review request for hive.


Changes
---

Addressed all the comments from Sasha.

Also, we use a static method to notify all listeners instead of creating a new 
MetaStoreListenerNotifier object.


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


Repository: hive-git


Description
---

This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
property from withing the DbNotificationListener class. It then passes the 
EnvironmentContext from transactional listeners to non-transactional listeners 
so that the eventId is shared between them.

The patch provides the following changes:
- DbNotificationListener   Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- HiveAlterHandler Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- MetaStoreListenerNotifierNew helper class that wraps the notification 
call to the listeners.
- TestObjectStore  Verifies that the addNotificationEvent() method 
saves the eventId on the NotificationEvent object.
- TestDbNotificationListener   Verifies that any HMS call is passing the 
DB_NOTIFICATION_EVENT_ID to non-transactional listeners.


Diffs (updated)
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 ea6cb790bc03fc38be07a3be7be34aa9c5d293ba 
  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
 PRE-CREATION 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
4ce6a659c94265d83abbaedb9c52d7e15bf8dde6 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
80b1e98b9efbf068f0cd32303499a3a9b5bbd67b 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
1f87eeb18f6edf7351b3c8da6a6826c08656e48c 


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

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


Testing
---

HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
complete 100% tests.


Thanks,

Sergio Pena

Re: Review Request 57728: HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767

[Sergio Pena]

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


Ship it!




Just remove the empty line from DateUtils. 
+1

- Sergio Pena


On March 24, 2017, 9:56 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57728/
> ---
> 
> (Updated March 24, 2017, 9:56 a.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> a1068ecce94e9ff1ae78008a0d8c6d67ca4f2690 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
>  26f1e75c7d659a634cd4eef3a0cb8e886b22722f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java 
> 8e33b7d437894b33b35f32913a3bc02f2a849ce3 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java 
> 5dc808800290f3274afbdff12134ac34387a746b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/timestamp/TestNanoTimeUtils.java
>  37cf0e2d74589cfa97fa24c9d2d8d00ea62390ee 
>   ql/src/test/queries/clientpositive/parquet_int96_timestamp.q 
> 5de2c3f1244b8340b97eb0547fe66e52d80fb065 
> 
> 
> Diff: https://reviews.apache.org/r/57728/diff/3/
> 
> 
> Testing
> ---
> 
> Tested loading timestamps from a parquet file written by spark.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 57728: HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767

[Sergio Pena]

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




common/src/java/org/apache/hive/common/util/DateUtils.java
Lines 77 (patched)
<https://reviews.apache.org/r/57728/#comment242748>

Empty line.


- Sergio Pena


On March 24, 2017, 9:56 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57728/
> ---
> 
> (Updated March 24, 2017, 9:56 a.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> a1068ecce94e9ff1ae78008a0d8c6d67ca4f2690 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
>  26f1e75c7d659a634cd4eef3a0cb8e886b22722f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java 
> 8e33b7d437894b33b35f32913a3bc02f2a849ce3 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java 
> 5dc808800290f3274afbdff12134ac34387a746b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/timestamp/TestNanoTimeUtils.java
>  37cf0e2d74589cfa97fa24c9d2d8d00ea62390ee 
>   ql/src/test/queries/clientpositive/parquet_int96_timestamp.q 
> 5de2c3f1244b8340b97eb0547fe66e52d80fb065 
> 
> 
> Diff: https://reviews.apache.org/r/57728/diff/3/
> 
> 
> Testing
> ---
> 
> Tested loading timestamps from a parquet file written by spark.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

 >   .notify(listeners);
> > }
> > 
> > This is essentially equivalent to constructing a new event with the 
> > given set of parameters.

They're always used only on the non-transactional listeners but 
transactionalListeners. I will think about another static method to pass all 
parameters instead.


> On March 23, 2017, 12:27 a.m., Alexander Kolbasov wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java
> > Lines 121 (patched)
> > <https://reviews.apache.org/r/57626/diff/4/?file=1670535#file1670535line121>
> >
> > If you decide to keep this interface,it would be cleaner to require 
> > that parameters isn't null and verify with Preconditions.checkNotNull

This is a public method that may be called by external listeners. I'd like to 
avoid throwing an exception and message if the parameters is null. I like that 
idea of just ignoring the null and not adding anything to the parameters. 
That's why I used the 'addParameters' name instead of 'setParameter' to giving 
a meaning.


- Sergio


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


On March 20, 2017, 10:33 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57626/
> ---
> 
> (Updated March 20, 2017, 10:33 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-16164
> https://issues.apache.org/jira/browse/HIVE-16164
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
> property from withing the DbNotificationListener class. It then passes the 
> EnvironmentContext from transactional listeners to non-transactional 
> listeners so that the eventId is shared between them.
> 
> The patch provides the following changes:
> - DbNotificationListener   Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - HiveAlterHandler Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - MetaStoreListenerNotifierNew helper class that wraps the notification 
> call to the listeners.
> - TestObjectStore  Verifies that the addNotificationEvent() 
> method saves the eventId on the NotificationEvent object.
> - TestDbNotificationListener   Verifies that any HMS call is passing the 
> DB_NOTIFICATION_EVENT_ID to non-transactional listeners.
> 
> 
> Diffs
> -
> 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  f7e3e3a0a71094992fdf4bd3ceea2da0bf7d1ff0 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
>  PRE-CREATION 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
> 4ce6a659c94265d83abbaedb9c52d7e15bf8dde6 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 07eca38190c1b05bb4a3977e9154423449828957 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
> 62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
> 1f87eeb18f6edf7351b3c8da6a6826c08656e48c 
> 
> 
> Diff: https://reviews.apache.org/r/57626/diff/4/
> 
> 
> Testing
> ---
> 
> HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
> complete 100% tests.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 57728: HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767

[Sergio Pena]

> On March 22, 2017, 6:27 p.m., Sergio Pena wrote:
> > common/src/java/org/apache/hive/common/util/DateUtils.java
> > Lines 84 (patched)
> > <https://reviews.apache.org/r/57728/diff/2/?file=1670971#file1670971line84>
> >
> > Is there another class where to put this method? I don't think 
> > DateUtils is the place where we should keep this.
> 
> Barna Zsombor Klara wrote:
> I couldn't find a much better fit. I looked at HiveUtils and 
> ParquetTableUtils but DateUtils seemed better. I can create a TimeZoneUtils 
> class, but I don't know if we will ever have a second function in it. Do you 
> have a utility class in mind that would be better?

Well, NanoTimeUtils is not a good name either, but it has the getCalendar() 
public method that returns a calendar based on UTC or local timezone. 
Eventually, we should rename this method to TimeUtils.java or something, but I 
don't know if external apps are using it.


- Sergio


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


On March 21, 2017, 5:28 p.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57728/
> ---
> 
> (Updated March 21, 2017, 5:28 p.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> a1068ecce94e9ff1ae78008a0d8c6d67ca4f2690 
>   common/src/test/org/apache/hive/common/util/TestDateUtils.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
>  26f1e75c7d659a634cd4eef3a0cb8e886b22722f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java 
> 8e33b7d437894b33b35f32913a3bc02f2a849ce3 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java 
> 5dc808800290f3274afbdff12134ac34387a746b 
>   ql/src/test/queries/clientpositive/parquet_int96_timestamp.q 
> 5de2c3f1244b8340b97eb0547fe66e52d80fb065 
> 
> 
> Diff: https://reviews.apache.org/r/57728/diff/2/
> 
> 
> Testing
> ---
> 
> Tested loading timestamps from a parquet file written by spark.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Alter table partition rename with location - moves partition back to hive warehouse

[Sergio Pena]

I don't think that behavior is correct. The rename should create the new
partition name in the same location of the table. Could you create a JIRA
for that?

On Wed, Mar 22, 2017 at 10:24 AM, Ying Chen  wrote:

> cross posting from u...@hive.apache.org.  (sorry for additional posting)
> I would like some opinions from the dev community relating to - if change
> of behavior is desired, how should it be changed ?
> Also, would it be better that I create a JIRA and have comments there?
>
> Thanks much..
>
> 
> Hello all -
>
> I was renaming my partition in a table that I've created using the location
> clause, and noticed that when after rename is completed, my partition is
> moved to the hive warehouse (hive.metastore.warehouse.dir). I was
> wondering, what should be the correct behavior?  Should the partition be
> renamed and maintain on the same file system, or no name change and not
> moved (so treating it like if someone declared external table) ?  I don't
> think it should be moved to hive.metastore.warehouse.dir
>
> A similar JIRA was open for renaming table:  https://issues.apache.org/
> jira/browse/HIVE-14909
> In which, if the table is determined not belonging to /apps/hive/warehouse
> (ie created by location clause), then table is not moved.
>
> Thanks much!
> Ying
>
> ==
> This is a problem for Hive 2.1 ...
>
> create table test_local_part (col1 int) partitioned by (col2 int) location
> '/tmp/testtable/test_local_part';
> insert into test_local_part  partition (col2=1) values (1),(3);
> insert into test_local_part  partition (col2=2) values (3);
> alter table test_local_part partition (col2='1') rename to partition
> (col2='4');
>
> Running:
>   describe formatted test_local_part partition (col2='2')
>
> # Detailed Partition Information
> Partition Value: [2]
> Database:   default
> Table:   test_local_part
> CreateTime: Mon Mar 20 13:25:28 PDT 2017
> LastAccessTime: UNKNOWN
> Protect Mode:   None
> Location:
> *hdfs://my.server.com:8020/tmp/testtable/test_local_part/col2=2
> *
>
> Running:
>describe formatted test_local_part partition (col2='4')
>
> # Detailed Partition Information
> Partition Value: [4]
> Database:   default
> Table:   test_local_part
> CreateTime: Mon Mar 20 13:24:53 PDT 2017
> LastAccessTime: UNKNOWN
> Protect Mode:   None
> Location:
> *hdfs://my.server.com:8020/apps/hive/warehouse/test_local_part/col2=4
>   *
> Partition Parameters:
>

Re: Review Request 57728: HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767

[Sergio Pena]

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




common/src/java/org/apache/hive/common/util/DateUtils.java
Lines 84 (patched)
<https://reviews.apache.org/r/57728/#comment242327>

Is there another class where to put this method? I don't think DateUtils is 
the place where we should keep this.


- Sergio Pena


On March 21, 2017, 5:28 p.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57728/
> ---
> 
> (Updated March 21, 2017, 5:28 p.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16231: Parquet timestamp may be stored differently since HIVE-12767
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hive/common/util/DateUtils.java 
> a1068ecce94e9ff1ae78008a0d8c6d67ca4f2690 
>   common/src/test/org/apache/hive/common/util/TestDateUtils.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
>  26f1e75c7d659a634cd4eef3a0cb8e886b22722f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java 
> 8e33b7d437894b33b35f32913a3bc02f2a849ce3 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java 
> 5dc808800290f3274afbdff12134ac34387a746b 
>   ql/src/test/queries/clientpositive/parquet_int96_timestamp.q 
> 5de2c3f1244b8340b97eb0547fe66e52d80fb065 
> 
> 
> Diff: https://reviews.apache.org/r/57728/diff/2/
> 
> 
> Testing
> ---
> 
> Tested loading timestamps from a parquet file written by spark.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 57805: HIVE-16049: upgrade to jetty 9

[Sergio Pena]

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



I see there is code that is used to support only JDK8 compilation on Hive. 
Should we create another JIRA for this? I think this is an important change so 
that the community is aware of the JIRA instead of keeping it hide on the Jetty 
upgrade.

- Sergio Pena


On March 21, 2017, 1:15 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57805/
> ---
> 
> (Updated March 21, 2017, 1:15 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16049: upgrade to jetty 9
> 
> 
> Diffs
> -
> 
>   .travis.yml d0e156849cde0b3f713888c4d8033e6603df302c 
>   common/pom.xml 8474a8700fca568e19ba723a076212470e7c9d3b 
>   common/src/java/org/apache/hive/http/HttpServer.java 
> db5650d0d2695011cfb07b23950bd611186a25f6 
>   hcatalog/build.properties dea1a44debdd2832220aaee02a94ce72cd228285 
>   hcatalog/pom.xml 34de177a85538baa14b4ec9f142ea7b668ecf790 
>   hcatalog/webhcat/svr/pom.xml c5ad38707b4473dfbee816be82875713fc9676e7 
>   
> hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/Main.java
>  5208bf4a4ca9e2b76e58c01f89f7f3e3f31a8d90 
>   llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapServiceDriver.java 
> 22e5ee87985b9b1ebbd023c6055d6afd0753c610 
>   pom.xml 3ea3c77e238524e979355d52bfc8dfdea9a66b7a 
>   service/pom.xml 9306739859a0c8b2dd59f252de741a31795421a0 
>   
> service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 
> ebec165fedf9a1bc7bb339781d9a2ce13a5593b5 
>   service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java 
> 4d50dd9082fc2d17a97831872bced75d4b229a5e 
>   spark-client/pom.xml effc13b95ebee705736450849210b6fd6d3a4f41 
> 
> 
> Diff: https://reviews.apache.org/r/57805/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

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

(Updated March 20, 2017, 10:33 p.m.)


Review request for hive.


Changes
---

Addressed Mothi's comments.

I still need to figure out a way to avoid listeners checks twice by using a 
static method (see feedback comments).


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


Repository: hive-git


Description
---

This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
property from withing the DbNotificationListener class. It then passes the 
EnvironmentContext from transactional listeners to non-transactional listeners 
so that the eventId is shared between them.

The patch provides the following changes:
- DbNotificationListener   Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- HiveAlterHandler Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- MetaStoreListenerNotifierNew helper class that wraps the notification 
call to the listeners.
- TestObjectStore  Verifies that the addNotificationEvent() method 
saves the eventId on the NotificationEvent object.
- TestDbNotificationListener   Verifies that any HMS call is passing the 
DB_NOTIFICATION_EVENT_ID to non-transactional listeners.


Diffs (updated)
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 f7e3e3a0a71094992fdf4bd3ceea2da0bf7d1ff0 
  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
 PRE-CREATION 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
4ce6a659c94265d83abbaedb9c52d7e15bf8dde6 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
07eca38190c1b05bb4a3977e9154423449828957 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
1f87eeb18f6edf7351b3c8da6a6826c08656e48c 


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

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


Testing
---

HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
complete 100% tests.


Thanks,

Sergio Pena

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

> On March 17, 2017, 10:34 p.m., Mohit Sabharwal wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Line 2476 (original), 2484 (patched)
> > <https://reviews.apache.org/r/57626/diff/2/?file=1666419#file1666419line2519>
> >
> > I like the idea of keeping fireMetaStoreAddPartitionEventTransactional 
> > method - because it makes it more readable that way. Otherwise, very 
> > similar looking code is duplicated 7 times. 
> > 
> > You can have fireMetaStoreAddPartitionEventTransactional return 
> > transactionalListenerResponses for later re-use.  But I wouldn't inline 
> > this code directly in add_partitions_core for readability reasons.

Agree. If we agree on a good readable way to notify all listeners, then this 
fire..() method won't be even need it. I'll let you know if that's the case.


> On March 17, 2017, 10:34 p.m., Mohit Sabharwal wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 3020 (patched)
> > <https://reviews.apache.org/r/57626/diff/2/?file=1666419#file1666419line3115>
> >
> > use i for consistency :)

The i already existed on the method, and my first intuition was to create a new 
one. But, the i is not used anymore, so I can reuse it.
Thanks.


> On March 17, 2017, 10:34 p.m., Mohit Sabharwal wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
> > Lines 138 (patched)
> > <https://reviews.apache.org/r/57626/diff/2/?file=1666420#file1666420line138>
> >
> > I agree with Alexander. We can avoid code duplication by doing most 
> > work inside notify.
> > 
> > I also agree that a simple static method is going to be much cleaner 
> > than MetaStoreListenerNotifier in this use case. 
> > 
> > The point of builder-like pattern (like MetaStoreListenerNotifier) to 
> > make it cleaner to construct an object. But in this case, we are 
> > introducing a new object when none is really needed. A simple static method 
> > is going to way more readable.
> > 
> > Even cost-wise the branch (listeners != null) is expensive, so let's 
> > not do it twice.

I can avoid the creationg of the new object, and just have a static 
notifyEvent() method that accepts some parameters, perhaps something like this:

if (!listeners.isEmpty()) {
  CreateTableEvent createTableEvent = new CreateTableEvent(tbl, success, this);
  createTableEvent.addParameters(transactionalListenerResponses);
  createTableEvent.setEnvironmentContext(envContext);

  MetaStoreListenerNotifier
.notifyEvent(EventType.CREATE_TABLE, createTableEvent, listeners);
}
  
But, I think I still need to check that listeners is not empty before calling 
the above code to avoid creating the CreateTableEvent object if listeners is 
empty.

I can avoid too much code, and make it cleaner by passing the 2 extra 
parameters to the notifier, but I still need to create the event.

if (!listeners.isEmpty()) {
  CreateTableEvent createTableEvent = new CreateTableEvent(tbl, success, this);

  MetaStoreListenerNotifier
.notifyEvent(EventType.CREATE_TABLE, createTableEvent, 
transactionalListenerResponses, envContext, listeners);
}

I was also thinking on create one method per event, and pass the same 
parameters, and let the method to create the event only if listeners is not 
empty. Something like this?

MetaStoreListenerNotifier.
   .onCreateTable(tbl, success, this, transactionalListenerResponses, 
envContext, listeners);
   
The above code would look cleaner on the HiveMetaStore, but the 
MetaStoreListenerNotifier must support every event method ,and accept at least 
6 parameters like the above code.

Sasha, Mohit, which option would you think is ideal? I can also go back to the 
original code, and keep the for() loops to walk through each listener.


- Sergio


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


On March 17, 2017, 8:14 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57626/
> ---
> 
> (Updated March 17, 2017, 8:14 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-16164
> https://issues.apache.org/jira/browse/HIVE-16164
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
> property from withing the DbNotificationListener class

Re: Review Request 57503: HIVE-16024: MSCK Repair Requires nonstrict hive.mapred.mode

[Sergio Pena]

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




ql/src/test/queries/clientpositive/msck_repair_0.q
Lines 19-21 (patched)
<https://reviews.apache.org/r/57503/#comment241669>

Is this actually adding new partitios to the metastore? Those are already 
discovered above. I think this part is not testing the changes correctly.


- Sergio Pena


On March 16, 2017, 3:55 p.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57503/
> ---
> 
> (Updated March 16, 2017, 3:55 p.m.)
> 
> 
> Review request for hive, Peter Vary, Sergio Pena, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16024: MSCK Repair Requires nonstrict hive.mapred.mode
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreChecker.java 
> 6805c17a116f5ef0febd36c59d454fa631ae0024 
>   ql/src/test/queries/clientpositive/msck_repair_0.q 
> ce8ef426a2a58845afc8333259d66725db416584 
>   ql/src/test/results/clientpositive/msck_repair_0.q.out 
> 3f2fe75b194f1248bd5c073dd7db6b71b2ffc2ba 
> 
> 
> Diff: https://reviews.apache.org/r/57503/diff/2/
> 
> 
> Testing
> ---
> 
> Tested locally and added qtests/unit tests.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

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

(Updated March 17, 2017, 8:14 p.m.)


Review request for hive.


Changes
---

Addressed feedback comments.


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


Repository: hive-git


Description
---

This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
property from withing the DbNotificationListener class. It then passes the 
EnvironmentContext from transactional listeners to non-transactional listeners 
so that the eventId is shared between them.

The patch provides the following changes:
- DbNotificationListener   Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- HiveAlterHandler Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- MetaStoreListenerNotifierNew helper class that wraps the notification 
call to the listeners.
- TestObjectStore  Verifies that the addNotificationEvent() method 
saves the eventId on the NotificationEvent object.
- TestDbNotificationListener   Verifies that any HMS call is passing the 
DB_NOTIFICATION_EVENT_ID to non-transactional listeners.


Diffs (updated)
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 f7e3e3a0a71094992fdf4bd3ceea2da0bf7d1ff0 
  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java
 PRE-CREATION 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
bae39acafeb86d04ac8ec66098be125cd3cef3e0 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
07eca38190c1b05bb4a3977e9154423449828957 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
1f87eeb18f6edf7351b3c8da6a6826c08656e48c 


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

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


Testing
---

HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
complete 100% tests.


Thanks,

Sergio Pena

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

> On March 17, 2017, 1:28 a.m., Alexander Kolbasov wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java
> > Line 33 (original), 37 (patched)
> > <https://reviews.apache.org/r/57626/diff/2/?file=1666421#file1666421line37>
> >
> > Status isn't realy a boolean - it is, essentially an enum, which can 
> > only change from Ok to Failure but never another way around.

I ended up removing the setStatus(), Because I am passing the transactional 
parameters to non-transactional listeners, then there is not need to re-use the 
object. I also notice that Optional.or() was creating a new instance of the 
event even though was not used at all.


> On March 17, 2017, 1:28 a.m., Alexander Kolbasov wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java
> > Lines 60 (patched)
> > <https://reviews.apache.org/r/57626/diff/2/?file=1666421#file1666421line60>
> >
> > I think, this should be setFailed()

I removed this setSTatus()


- Sergio


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


On March 16, 2017, 10:36 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57626/
> ---
> 
> (Updated March 16, 2017, 10:36 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-16164
> https://issues.apache.org/jira/browse/HIVE-16164
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
> property from withing the DbNotificationListener class. It then passes the 
> EnvironmentContext from transactional listeners to non-transactional 
> listeners so that the eventId is shared between them.
> 
> The patch provides the following changes:
> - DbNotificationListener   Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - HiveAlterHandler Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - MetaStoreListenerNotifierNew helper class that wraps the notification 
> call to the listeners.
> - TestObjectStore  Verifies that the addNotificationEvent() 
> method saves the eventId on the NotificationEvent object.
> - TestDbNotificationListener   Verifies that any HMS call is passing the 
> DB_NOTIFICATION_EVENT_ID to non-transactional listeners.
> 
> 
> Diffs
> -
> 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  f7e3e3a0a71094992fdf4bd3ceea2da0bf7d1ff0 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
> bae39acafeb86d04ac8ec66098be125cd3cef3e0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 07eca38190c1b05bb4a3977e9154423449828957 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
> 62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
> 1f87eeb18f6edf7351b3c8da6a6826c08656e48c 
> 
> 
> Diff: https://reviews.apache.org/r/57626/diff/2/
> 
> 
> Testing
> ---
> 
> HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
> complete 100% tests.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

steners because I couldn't reuse the ListenerEvent in 
all the HiveMetaStore cases.


> On March 17, 2017, 1:28 a.m., Alexander Kolbasov wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java
> > Lines 90 (patched)
> > <https://reviews.apache.org/r/57626/diff/2/?file=1666421#file1666421line90>
> >
> > Naming - may be addParameter() ?
> >     Also - is it allowed to modify existing parameter or not?
> > addParameter() may throw RuntimeException if an attempt is made to 
> > modify existing parameter.

Yeah, we should do that.


- Sergio


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


On March 16, 2017, 10:36 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57626/
> ---
> 
> (Updated March 16, 2017, 10:36 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-16164
> https://issues.apache.org/jira/browse/HIVE-16164
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
> property from withing the DbNotificationListener class. It then passes the 
> EnvironmentContext from transactional listeners to non-transactional 
> listeners so that the eventId is shared between them.
> 
> The patch provides the following changes:
> - DbNotificationListener   Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - HiveAlterHandler Changes to pass the EnvironmentContext from 
> transactional to non-transactional listeners.
> - MetaStoreListenerNotifierNew helper class that wraps the notification 
> call to the listeners.
> - TestObjectStore  Verifies that the addNotificationEvent() 
> method saves the eventId on the NotificationEvent object.
> - TestDbNotificationListener   Verifies that any HMS call is passing the 
> DB_NOTIFICATION_EVENT_ID to non-transactional listeners.
> 
> 
> Diffs
> -
> 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  f7e3e3a0a71094992fdf4bd3ceea2da0bf7d1ff0 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
> bae39acafeb86d04ac8ec66098be125cd3cef3e0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 07eca38190c1b05bb4a3977e9154423449828957 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
> 62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
> 1f87eeb18f6edf7351b3c8da6a6826c08656e48c 
> 
> 
> Diff: https://reviews.apache.org/r/57626/diff/2/
> 
> 
> Testing
> ---
> 
> HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
> complete 100% tests.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: HiveQA is unable to add lira comments

[Sergio Pena]

The bug was fixed some time ago on the server. The following steps to check
if the bug is still there are passing:
https://confluence.atlassian.com/kb/unable-to-connect-to-ssl-services-due-to-pkix-path-building-failed-779355358.html

Anyway, I see there are a couple of package upgrades available on the
server, such as jdk8 and ca-certificates-java. I could upgrade
ca-certificates-java but jd8 due to breaking issues between those and jdk7.
I will need to stop the ptest server to do the maintenance and see if that
helps.

I'll try to do it by the eod.

On Thu, Mar 16, 2017 at 9:11 AM, Zoltan Haindrich <
zhaindr...@hortonworks.com> wrote:

> Hello,
>
>
> it seems like HiveQA is having some difficulty posting the test results to
> jira…
>
>
> https://builds.apache.org/job/PreCommit-HIVE-Build/4171/console
>
> https://builds.apache.org/job/PreCommit-HIVE-Build/4176/console
>
>
> from the exception .. it seems like some kind of ca-cert problem -
> however, I’m not sure..
>
>
> cheers,
>
> Zoltan
>
>
>
> ATTACHMENT ID: 12859012 - PreCommit-HIVE-Build
>
> 2017-03-16 13:44:52,061 ERROR [TestExecutor] JIRAService.publishComments:194
> Encountered error attempting to post comment to HIVE-16219 
> javax.net.ssl.SSLHandshakeException:
> sun.security.validator.ValidatorException: PKIX path building failed:
> sun.security.provider.certpath.SunCertPathBuilderException: unable to
> find valid certification path to requested target
> at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
> at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1914)
> at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:279)
>
> […]
>
> Caused by: sun.security.provider.certpath.SunCertPathBuilderException:
> unable to find valid certification path to requested target
> at sun.security.provider.certpath.SunCertPathBuilder.engineBuild(
> SunCertPathBuilder.java:196)
> at java.security.cert.CertPathBuilder.build(
> CertPathBuilder.java:268)
> at sun.security.validator.PKIXValidator.doBuild(
> PKIXValidator.java:380)
> ... 26 more
>
>

Re: Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

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

(Updated March 16, 2017, 10:36 p.m.)


Review request for hive.


Changes
---

This patch removes the use of EnvironmentContext to pass parameters between 
listeners. Instead, a new parameters field is added to the ListenerEvent, and 
used by listeners to update their results instead.


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


Repository: hive-git


Description
---

This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
property from withing the DbNotificationListener class. It then passes the 
EnvironmentContext from transactional listeners to non-transactional listeners 
so that the eventId is shared between them.

The patch provides the following changes:
- DbNotificationListener   Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- HiveAlterHandler Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- MetaStoreListenerNotifierNew helper class that wraps the notification 
call to the listeners.
- TestObjectStore  Verifies that the addNotificationEvent() method 
saves the eventId on the NotificationEvent object.
- TestDbNotificationListener   Verifies that any HMS call is passing the 
DB_NOTIFICATION_EVENT_ID to non-transactional listeners.


Diffs (updated)
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 f7e3e3a0a71094992fdf4bd3ceea2da0bf7d1ff0 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
bae39acafeb86d04ac8ec66098be125cd3cef3e0 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
07eca38190c1b05bb4a3977e9154423449828957 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/events/ListenerEvent.java 
62aeb8cc343fc4aab72e76da890e36ac3a16b7bf 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
1f87eeb18f6edf7351b3c8da6a6826c08656e48c 


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

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


Testing
---

HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
complete 100% tests.


Thanks,

Sergio Pena

Re: [ANNOUNCE] New PMC Member : Eugene Koifman

[Sergio Pena]

Congratulations Eugene !!

On Wed, Mar 15, 2017 at 5:41 AM, Prasanth Jayachandran <
pjayachand...@hortonworks.com> wrote:

> Congratulations Eugene!
>
> Thanks
> Prasanth
>
>
>
>
> On Tue, Mar 14, 2017 at 10:02 PM -1000, "Zoltan Haindrich" <
> zhaindr...@hortonworks.com> wrote:
>
>
> Congrats Eugene!!
>
> On 15 Mar 2017 07:50, Peter Vary  wrote:
> Congratulations! :)
>
> 2017. márc. 15. 7:05 ezt írta ("Vaibhav Gumashta" ):
>
> > Congrats Eugene!
> >
> >
> > On 3/14/17, 11:03 PM, "Rajesh Balamohan"  wrote:
> >
> > >Congrats Eugene!! :)
> > >
> > >~Rajesh.B
> > >
> > >On Wed, Mar 15, 2017 at 11:21 AM, Pengcheng Xiong
> > >wrote:
> > >
> > >> Congrats! Well deserved!
> > >>
> > >> Thanks.
> > >> Pengcheng
> > >>
> > >> On Tue, Mar 14, 2017 at 10:39 PM, Ashutosh Chauhan
> > >>
> > >> wrote:
> > >>
> > >> > On behalf of the Hive PMC I am delighted to announce Eugene Koifman
> is
> > >> > joining Hive PMC.
> > >> > Eugene is a long time contributor in Hive and is focusing on ACID
> > >>support
> > >> > areas these days.
> > >> >
> > >> > Welcome, Eugene!
> > >> >
> > >> > Thanks,
> > >> > Ashutosh
> > >> >
> > >>
> >
> >
>
>
>

Review Request 57626: HIVE-16164: Provide mechanism for passing HMS notification ID between transactional and non-transactional listeners.

[Sergio Pena]

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

Review request for hive.


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


Repository: hive-git


Description
---

This fix updates the EnvironmentContext with a DB_NOTIFICATION_EVENT_ID 
property from withing the DbNotificationListener class. It then passes the 
EnvironmentContext from transactional listeners to non-transactional listeners 
so that the eventId is shared between them.

The patch provides the following changes:
- DbNotificationListener   Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- HiveAlterHandler Changes to pass the EnvironmentContext from 
transactional to non-transactional listeners.
- MetaStoreListenerNotifierNew helper class that wraps the notification 
call to the listeners.
- TestObjectStore  Verifies that the addNotificationEvent() method 
saves the eventId on the NotificationEvent object.
- TestDbNotificationListener   Verifies that any HMS call is passing the 
DB_NOTIFICATION_EVENT_ID to non-transactional listeners.


Diffs
-

  
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
 f7e3e3a0a71094992fdf4bd3ceea2da0bf7d1ff0 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 1cf47c36cb490ce0b17ffe312cd2e9fc4bb7cd9a 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 
bae39acafeb86d04ac8ec66098be125cd3cef3e0 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
07eca38190c1b05bb4a3977e9154423449828957 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java
 PRE-CREATION 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
1f87eeb18f6edf7351b3c8da6a6826c08656e48c 


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


Testing
---

HiveQA showed only one test failure. it is fixed, and waiting for HiveQA to 
complete 100% tests.


Thanks,

Sergio Pena

Re: Review Request 57503: HIVE-16024: MSCK Repair Requires nonstrict hive.mapred.mode

[Sergio Pena]

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




common/src/test/org/apache/hadoop/hive/common/TestFixedSizeCollection.java
Lines 32 (patched)
<https://reviews.apache.org/r/57503/#comment241253>

Should we use the java naming convention for method names?



ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreChecker.java
Line 219 (original), 223-228 (patched)
<https://reviews.apache.org/r/57503/#comment241252>

Does this mean that if a user attempts to repair a table with more than 
500_000 partitions, the MSCK will fail?

If so, I think we're having the same problem as before. Users won't be able 
to discover all partitions from a table with more than 500_000.

Did we have problems before the patch which added this regression issue? If 
not, should we use PartitionIterable with unlimited number of partitions 
instead? The number of HMS transactions due to PartitionIterable shouldn't be a 
problem if the user increases the batch size. Also, as Vihang mentioned, we're 
just storing two values (partition name + table name), so that consumes less 
memory than using the hive.getPartition() method call.


- Sergio Pena


On March 10, 2017, 10:36 a.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57503/
> ---
> 
> (Updated March 10, 2017, 10:36 a.m.)
> 
> 
> Review request for hive, Peter Vary, Sergio Pena, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16024: MSCK Repair Requires nonstrict hive.mapred.mode
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/common/FixedSizeCollection.java 
> PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> a479deb7c0c6b779277f1029009b7dfab6dcb9e3 
>   common/src/test/org/apache/hadoop/hive/common/TestFixedSizeCollection.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreChecker.java 
> 6805c17a116f5ef0febd36c59d454fa631ae0024 
>   ql/src/test/queries/clientnegative/msck_repair_4.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/msck_repair_0.q 
> ce8ef426a2a58845afc8333259d66725db416584 
>   ql/src/test/results/clientnegative/msck_repair_4.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/msck_repair_0.q.out 
> 3f2fe75b194f1248bd5c073dd7db6b71b2ffc2ba 
> 
> 
> Diff: https://reviews.apache.org/r/57503/diff/1/
> 
> 
> Testing
> ---
> 
> Tested locally and added qtests/unit tests.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>

Re: Backward incompatible changes

[Sergio Pena]

Hey Ashutosh, thanks for soliciting feedback on this.

I like the idea you're proposing; maintaining compatibility and at the
same time adding newer features to
Hive consumes a lot of development time and effort.

However, I think some users and companies have just started to use Hive 2.x
branch as their main major upgrade on Hive
(possible due to waiting for stabilization and testing upgrades), but
cutting this major branch that just has 1 year of life
might make us look like we will forget about the quality of Hive 2.x as we
did with branch-1.

Hive 1.x latest version was 1.2, and its development stopped because new
features on Hive 2.x
Hive 2.x latest version is 2.1, and we want to create Hive 3.x because of
newer features and incompatibilities.
Will Hive 3.x have the same future after 3.1 is released?

What I'm also concerned is about these three things:

   - *Branch-2 quality commitment*.
   How will we keep the community motivated on fixing both master and
   branch-2?
   - *Harder cherry-picks between master and branch-2*.
   Because master will be incompatible by nature, then cherry-picks to
   branch-2 will be harder.
   - *Removal of MR2 on the master branch*.
   This was marked as deprecated just last year, but MR2 is still an engine
   that is used by several users.

I accept that the end of life of major versions will come at some point,
and these concerns will expire,
but Hive 2.x is kind of young, isn't it?

Should we try to stabilize the Hive 2.x line first, and have a few more
releases before starting to work on Hive 3.0?
Should we add more test coverage to Hive jenkins jobs to validate Hive 2.x
quality?
Should we agree on a date about when we should drop community support on
Hive versions to let users know about this?

Again, I like your proposal, but I'm afraid that users who just upgraded to
2.x won't have any more features and improvements
because they will be developed on 3.0.

- Sergio



On Mon, Mar 6, 2017 at 1:24 PM, Ashutosh Chauhan  wrote:

> The way it helps shedding debt  is because dev can now do refactoring
> without fear of breaking some rarely used features. The way that helps for
> adding feature faster is since codebase is lean and easier to reason about
> its much easier to add new features.
>
> More importantly though, it also helps users because we are setting the
> expectation from dev community. They can expect that future releases of 2.x
> to be backward compatible. At the same time whenever they decide to upgrade
> they only need to test their application once against 3.x as oppose to
> continuous breakage of one form or another if we continue to make
> incompatible changes in master without branching for 2.x
>
> Thanks,
> Ashutosh
>
> On Sat, Mar 4, 2017 at 10:19 AM, Edward Capriolo 
> wrote:
>
> > Also i dont follow how we remove
> >
> > On Saturday, March 4, 2017, Edward Capriolo 
> wrote:
> >
> > >
> > >
> > > On Fri, Mar 3, 2017 at 8:46 PM, Thejas Nair  > > > wrote:
> > >
> > >> +1
> > >> There are some features that are incomplete and what I would not
> > recommend
> > >> for any real production use.The 'legacy authorization mode' is a great
> > >> example of that -
> > >> https://cwiki.apache.org/confluence/display/Hive/Hive+Defaul
> > >> t+Authorization+-+Legacy+Mode
> > >> . It is inherently insecure mode that nobody should be using.
> > >>
> > >> There is also potential to cleanup of the thrift api. However, there
> are
> > >> many users of this api, we would need to go the deprecation then
> remove
> > >> after couple of releases route or so for that.
> > >>
> > >> I am sure there are many other candidates. We will have to evaluate
> each
> > >> of
> > >> those features on the risk/benefit of keeping them and arriving at a
> > >> decision.
> > >>
> > >> Also, +1 on getting a 2.2 release out before we branch.
> > >>
> > >>
> > >>
> > >> On Fri, Mar 3, 2017 at 1:50 PM, Ashutosh Chauhan <
> hashut...@apache.org
> > >> >
> > >> wrote:
> > >>
> > >> > Hi all,
> > >> >
> > >> > Hive project has come a long way. With wide-spread adoption also
> comes
> > >> > expectations. Expectation of being backward compatible and not
> > breaking
> > >> > things. However that doesn't come free of cost and results in lot of
> > >> legacy
> > >> > code which can't be refactored without fear of breaking things. As a
> > >> result
> > >> > project has accumulated lot of debt over time. At the same time
> there
> > >> are
> > >> > also lot of features which have seen little uptake. We may want to
> > drop
> > >> > some of those.
> > >> >
> > >> > In order to move forward and shed that debt we may need a major
> > version
> > >> > release which allows us to make backward incompatible changes and
> drop
> > >> > rarely used features. At the same time there are lots of users 

Re: Green unit test results

[Sergio Pena]

- Probably avoiding committing a patch if a flaky test is shown on the test
results?
- Should we add a jenkins job that checks for flaky tests like the hbase
project did?
https://builds.apache.org/view/H-L/view/HBase/job/HBase-Find-Flaky-Tests/

On Thu, Mar 9, 2017 at 10:21 AM, Ashutosh Chauhan <hashut...@apache.org>
wrote:

> Great news! Thanks to everyone who contributed in getting our tests and
> test infra sorted out.
> We would definitely want to keep the status either green or blue definitely
> not red :) All our previous efforts in keeping builds green didn't bear
> fruit.
> So, I think we need to make some changes here.
>
> Any ideas what we can do to ensure green builds going forward?
>
> Thanks,
> Ashutosh
>
> On Thu, Mar 9, 2017 at 8:07 AM, Sergio Pena <sergio.p...@cloudera.com>
> wrote:
>
> > It's actually blue Peter :).
> >
> > But good job, I see that the console output is:
> >
> > {color:red}ERROR:{color} -1 due to no test(s) being added or modified.
> >
> > {color:green}SUCCESS:{color} +1 due to 10336 tests passed
> >
> >
> > On Thu, Mar 9, 2017 at 8:12 AM, Peter Vary <pv...@cloudera.com> wrote:
> >
> > > Hi,
> > >
> > > Congratulations for everyone who have helped taking care of the unit
> test
> > > failures!
> > > I have got my first green run! :)
> > >
> > > If any of you interested in:
> > > https://builds.apache.org/job/PreCommit-HIVE-Build/4049/testReport/ <
> > > https://builds.apache.org/job/PreCommit-HIVE-Build/4049/testReport/>
> :)
> > >
> > > Great day, and again thanks everyone!
> > >
> > > Peter
> >
>

Re: Green unit test results

[Sergio Pena]

It's actually blue Peter :).

But good job, I see that the console output is:

{color:red}ERROR:{color} -1 due to no test(s) being added or modified.

{color:green}SUCCESS:{color} +1 due to 10336 tests passed


On Thu, Mar 9, 2017 at 8:12 AM, Peter Vary  wrote:

> Hi,
>
> Congratulations for everyone who have helped taking care of the unit test
> failures!
> I have got my first green run! :)
>
> If any of you interested in:
> https://builds.apache.org/job/PreCommit-HIVE-Build/4049/testReport/ <
> https://builds.apache.org/job/PreCommit-HIVE-Build/4049/testReport/> :)
>
> Great day, and again thanks everyone!
>
> Peter

Re: Pre-commit job failing - No disk space

[Sergio Pena]

Hey Vihang,

Seems there is a UT test causing all this noise. See this:

30G
./PreCommit-HIVE-Build-3902/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3900/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3899/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3898/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3897/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3896/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3893/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3892/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3891/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3890/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3888/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3887/failed/272_UTBatch_ql_10_tests/logs/hive.log
30G
./PreCommit-HIVE-Build-3877/failed/272_UTBatch_ql_10_tests/logs/hive.log
29G
./PreCommit-HIVE-Build-3903/failed/272_UTBatch_ql_10_tests/logs/hive.log
29G
./PreCommit-HIVE-Build-3894/failed/272_UTBatch_ql_10_tests/logs/hive.log
26G
./PreCommit-HIVE-Build-3904/failed/272_UTBatch_ql_10_tests/logs/hive.log
26G
./PreCommit-HIVE-Build-3876/failed/272_UTBatch_ql_10_tests/logs/hive.log
24G
./PreCommit-HIVE-Build-3895/failed/272_UTBatch_ql_10_tests/logs/hive.log
19G
./PreCommit-HIVE-Build-3918/failed/272_UTBatch_ql_10_tests/logs/.hive.log.9uk9Av
5.1G
 
./PreCommit-HIVE-Build-3913/failed/14-TestCliDriver-authorization_create_temp_table.q-skewjoinopt16.q-drop_partitions_filter3.q-and-27-more/logs/hive.log
4.8G
 
./PreCommit-HIVE-Build-3913/failed/20-TestCliDriver-rename_column.q-union5.q-insert_into1.q-and-27-more/logs/hive.log
4.7G
 
./PreCommit-HIVE-Build-3913/failed/19-TestCliDriver-cp_mj_rc.q-order.q-udf_bitwise_shiftleft.q-and-27-more/logs/hive.log
4.0G
 ./PreCommit-HIVE-Build-3918/succeeded/185_UTBatch_service_8_tests/logs/hive.log
4.0G
 ./PreCommit-HIVE-Build-3917/succeeded/185_UTBatch_service_8_tests/logs/hive.log

You can see all the logs from this link:
http://104.198.109.242/logs/



On Fri, Mar 3, 2017 at 2:21 PM, Sergio Pena <sergio.p...@cloudera.com>
wrote:

> Hey Vihang,
>
> The ptest master shows how the jobs has increased the log size, could you
> investigate what causes this increase?
>
> 44G PreCommit-HIVE-Build-3902
> 44G PreCommit-HIVE-Build-3900
> 44G PreCommit-HIVE-Build-3899
> 44G PreCommit-HIVE-Build-3898
> 44G PreCommit-HIVE-Build-3897
> 44G PreCommit-HIVE-Build-3896
> 44G PreCommit-HIVE-Build-3893
> 44G PreCommit-HIVE-Build-3892
> 44G PreCommit-HIVE-Build-3891
> 44G PreCommit-HIVE-Build-3890
> 44G PreCommit-HIVE-Build-3888
> 44G PreCommit-HIVE-Build-3877
> 43G PreCommit-HIVE-Build-3903
> 43G PreCommit-HIVE-Build-3894
> 41G PreCommit-HIVE-Build-3887
> 40G PreCommit-HIVE-Build-3904
> 40G PreCommit-HIVE-Build-3876
> 38G PreCommit-HIVE-Build-3895
> 16G PreCommit-HIVE-Build-3918
> 15G PreCommit-HIVE-Build-3917
> 15G PreCommit-HIVE-Build-3916
> 15G PreCommit-HIVE-Build-3915
> 15G PreCommit-HIVE-Build-3913
> 15G PreCommit-HIVE-Build-3912
> 15G PreCommit-HIVE-Build-3911
> 15G PreCommit-HIVE-Build-3910
> 15G PreCommit-HIVE-Build-3909
> 15G PreCommit-HIVE-Build-3905
> 14G PreCommit-HIVE-Build-3914
> 14G PreCommit-HIVE-Build-3878
> 212KPreCommit-HIVE-Build-3889
> 212KPreCommit-HIVE-Build-3880
> 188KPreCommit-HIVE-Build-3901
> 156KPreCommit-HIVE-Build-3885
> 148KPreCommit-HIVE-Build-3884
> 96K PreCommit-HIVE-Build-3882
> 88K PreCommit-HIVE-Build-3886
> 84K PreCommit-HIVE-Build-3881
> 80K PreCommit-HIVE-Build-3883
> 60K PreCommit-HIVE-Build-3879
> 56K PreCommit-HIVE-Build-3908
>
> - Sergio
>
> On Fri, Mar 3, 2017 at 12:03 PM, Vihang Karajgaonkar <vih...@cloudera.com>
> wrote:
>
>> Hi Sergio,
>>
>> Looks like the pre-commit jobs are failing due to disk space issues. Can
>> you please help or let me know how can I fix it?
>>
>> Thanks,
>> Vihang
>>
>
>

Re: Pre-commit job failing - No disk space

[Sergio Pena]

Hey Vihang,

The ptest master shows how the jobs has increased the log size, could you
investigate what causes this increase?

44G PreCommit-HIVE-Build-3902
44G PreCommit-HIVE-Build-3900
44G PreCommit-HIVE-Build-3899
44G PreCommit-HIVE-Build-3898
44G PreCommit-HIVE-Build-3897
44G PreCommit-HIVE-Build-3896
44G PreCommit-HIVE-Build-3893
44G PreCommit-HIVE-Build-3892
44G PreCommit-HIVE-Build-3891
44G PreCommit-HIVE-Build-3890
44G PreCommit-HIVE-Build-3888
44G PreCommit-HIVE-Build-3877
43G PreCommit-HIVE-Build-3903
43G PreCommit-HIVE-Build-3894
41G PreCommit-HIVE-Build-3887
40G PreCommit-HIVE-Build-3904
40G PreCommit-HIVE-Build-3876
38G PreCommit-HIVE-Build-3895
16G PreCommit-HIVE-Build-3918
15G PreCommit-HIVE-Build-3917
15G PreCommit-HIVE-Build-3916
15G PreCommit-HIVE-Build-3915
15G PreCommit-HIVE-Build-3913
15G PreCommit-HIVE-Build-3912
15G PreCommit-HIVE-Build-3911
15G PreCommit-HIVE-Build-3910
15G PreCommit-HIVE-Build-3909
15G PreCommit-HIVE-Build-3905
14G PreCommit-HIVE-Build-3914
14G PreCommit-HIVE-Build-3878
212KPreCommit-HIVE-Build-3889
212KPreCommit-HIVE-Build-3880
188KPreCommit-HIVE-Build-3901
156KPreCommit-HIVE-Build-3885
148KPreCommit-HIVE-Build-3884
96K PreCommit-HIVE-Build-3882
88K PreCommit-HIVE-Build-3886
84K PreCommit-HIVE-Build-3881
80K PreCommit-HIVE-Build-3883
60K PreCommit-HIVE-Build-3879
56K PreCommit-HIVE-Build-3908

- Sergio

On Fri, Mar 3, 2017 at 12:03 PM, Vihang Karajgaonkar 
wrote:

> Hi Sergio,
>
> Looks like the pre-commit jobs are failing due to disk space issues. Can
> you please help or let me know how can I fix it?
>
> Thanks,
> Vihang
>

Re: [VOTE] Drop support for Java7 in master branch

[Sergio Pena]

Thanks Alan for the proposal. JDK8 seems to have very useful API. It should
be good to start using it.

+1.

On Tue, Feb 28, 2017 at 12:55 AM, Rajat Khandelwal <
rajat.khandel...@inmobi.com> wrote:

> +1
>
> On Tue, Feb 28, 2017 at 10:37 AM, Prasanth Jayachandran <
> pjayachand...@hortonworks.com> wrote:
>
> > +1
> >
> > Thanks
> > Prasanth
> >
> >
> >
> >
> > On Mon, Feb 27, 2017 at 8:55 PM -0800, "Thejas Nair" <
> > thejas.n...@gmail.com> wrote:
> >
> >
> > There was a [DISCUSS] thread on the topic of moving to jdk8 for unit
> tests
> > [1], and many people also expressed the opinion that we should drop JDK 7
> > support in Hive. Public updates by Oracle was stopped on Apr 2015 [2].
> >
> > This vote thread proposes to dropping JDK 7 support in the next Apache
> Hive
> > 2.x release (ie master branch), so that we can start leveraging new
> > features in Java 8 and also libraries that require java8.
> >
> > [1] https://s.apache.org/hive-jdk8-test
> > [2] http://www.oracle.com/technetwork/java/eol-135779.html
> >
> > Here is my +1.
> >
> > Vote ends in 72 hours.
> > Thanks,
> > Thejas
> >
> > PS: I think this would fall under "Code change" under Hive-bylaws, so it
> > doesn't seem to really require a formal vote thread. But I think this
> does
> > merit wider attention than a jira ticket.
> >
> >
>
>
> --
> Rajat Khandelwal
> Tech Lead
>
> --
> _
> The information contained in this communication is intended solely for the
> use of the individual or entity to whom it is addressed and others
> authorized to receive it. It may contain confidential or legally privileged
> information. If you are not the intended recipient you are hereby notified
> that any disclosure, copying, distribution or taking any action in reliance
> on the contents of this information is strictly prohibited and may be
> unlawful. If you have received this communication in error, please notify
> us immediately by responding to this email and then delete it from your
> system. The firm is neither liable for the proper and complete transmission
> of the information contained in this communication nor for any delay in its
> receipt.
>

Re: Review Request 56334: HIVE-12767: Implement table property to address Parquet int96 timestamp bug

[Sergio Pena]

> On Feb. 27, 2017, 11:20 p.m., Sergio Pena wrote:
> > Ship It!

Thanks Zsombor. This is good work.


- Sergio


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


On Feb. 24, 2017, 2:56 p.m., Barna Zsombor Klara wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56334/
> ---
> 
> (Updated Feb. 24, 2017, 2:56 p.m.)
> 
> 
> Review request for hive, Ryan Blue and Sergio Pena.
> 
> 
> Bugs: HIVE-12767
> https://issues.apache.org/jira/browse/HIVE-12767
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> This is a followup on this review request: https://reviews.apache.org/r/41821
> The following exit criteria is addressed in this patch:
> 
> - Hive will read Parquet MR int96 timestamp data and adjust values using a 
> time zone from a table property, if set, or using the local time zone if it 
> is absent. No adjustment will be applied to data written by Impala.
> - Hive will write Parquet int96 timestamps using a time zone adjustment from 
> the same table property, if set, or using the local time zone if it is 
> absent. This keeps the data in the table consistent.
> - New tables created by Hive will set the table property to UTC if the global 
> option to set the property for new tables is enabled.
> - Tables created using CREATE TABLE and CREATE TABLE LIKE FILE will not set 
> the property unless the global setting to do so is enabled.
> - Tables created using CREATE TABLE LIKE  will copy the property 
> of the table that is copied.
> 
> To set the timezone table property, use this:
>   create table tbl1 (ts timestamp) stored as parquet tblproperties 
> ('parquet.mr.int96.write.zone'='PST');
> 
> To set UTC as default timezone table property on new tables created, use 
> this: 
>   set parquet.mr.int96.enable.utc.write.zone=true;
>   create table tbl2 (ts timestamp) stored as parquet;
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> f0c129bcbe25f07f30eba14234e30a6442649437 
>   data/files/impala_int96_timestamp.parq PRE-CREATION 
>   
> itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java
>  a14b7900afb00a7d304b0dc4f6482a2b87716919 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> adabe70fa8f0fe1b990c6ac578a14ff5af06fc93 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
>  379a9135d9c631b2f473976b00f3dc87f9fec0c4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java 
> 167f9b6516ac093fa30091daf6965de25e3eccb3 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java 
> 76d93b8e02a98c95da8a534f2820cd3e77b4bb43 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java
>  604cbbcc2a9daa8594397e315cc4fd8064cc5005 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java
>  ac430a67682d3dcbddee89ce132fc0c1b421e368 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java 
> 3fd75d24f3fda36967e4957e650aec19050b22f8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java
>  b6a1a7a64db6db0bf06d2eea70a308b88f06156e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java
>  3d5c6e6a092dd6a0303fadc6a244dad2e31cd853 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriteSupport.java
>  f4621e5dbb81e8d58c4572c901ec9d1a7ca8c012 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java
>  6b7b50a25e553629f0f492e964cc4913417cb500 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestDataWritableWriter.java 
> 934ae9f255d0c4ccaa422054fcc9e725873810d4 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java
>  670bfa609704d3001dd171b703b657f57fbd4c74 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java
>  f537ceee505c5f41d513df3c89b63453012c9979 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/convert/TestETypeConverter.java
>  PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetTimestampUtils.java