> On Jan. 3, 2019, 1:42 a.m., Karthik Manamcheri wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java
> > Lines 374 (patched)
> > <https://reviews.apache.org/r/69642/diff/4/?file=2117205#file2117205line374>
> >
> >     The behavior of the getPartitions\* changed to be more inline with how 
> > the other getTable/getDatabase calls work.
> >     
> >     Before this change, if you issue a getPartitionsByNames with an empty 
> > database, we threw an exception. After this change, we will return an empty 
> > list of partitions instead. This behavior is similar to what happens if you 
> > issue a getTablesByNames call (an empty list of tables are returned)
> 
> Peter Vary wrote:
>     This change is also worrying. This is also an API change which might 
> cause backward incompatibility problems with customers expecting empty list.
>     We proposed this kind of changes a way back, and the community consensus 
> was that we should not change even the Exception types that have been thrown.

Actually I am fixing a regression bug. In the master branch (without my 
change), if you remove the TransactionalValidationListener from the list of 
pre-listeners, the GetPartitions API tests fail! The getPartitions API test 
depends on the fact there is a a pre-event listener. Customers already expect 
the API to throw an exception (or return an empty list) depending on if there 
is a listener attached or not. So the API contract itself is that it can throw 
an exception (OR return an empty list). This is how earlier versions of Hive 
are. HIVE-12064 introduced a bug which changed the behavior of clients. My 
change makes it so that it behaves similar to how earlier versions of Hive 
behaved (before HIVE-12064).

We should also fix the tests so that they don't depend of the existence (or 
non-existence) of listeners or plugins.

Basically what I am saying is that this API change is not new and is how Hive 
1.x/2.x behaved (before HIVE-12064). We wrote the unit tests around the bug!


- Karthik


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


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
> 
>

Reply via email to