> On jan. 3, 2019, 1:42 de, 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. > > Karthik Manamcheri wrote: > 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!
Bugs which are exist for too long prone to become features :D I personally agree with you. but I would be more confortible if we could find 1 more committer to validate our view - maybe Alan Gates, Thejas Nair who were the ones who actively participated in our "IMetaStoreClient and HMS Thrift API exception handling" discussion. You might want to ping them. Thanks, Peter - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69642/#review211625 ----------------------------------------------------------- On jan. 3, 2019, 1:40 de, 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 de) > > > 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 > >