> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote: > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > > Lines 2458 (patched) > > <https://reviews.apache.org/r/69585/diff/2/?file=2114850#file2114850line2458> > > > > This is the same method as the one on the HiveMetaStore. Would it be > > possible to move it to a common method under > > hive-standalone-metastore-common?
some variables used are different. such as isClientFilterEnabled vs isServerFilterEnabled and filterHook. If we make is generate function, those need to be input. No sure how valuable to make that change. > On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote: > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > > Lines 662 (patched) > > <https://reviews.apache.org/r/69585/diff/2/?file=2114851#file2114851line662> > > > > The second parameter in ConfVars corresponds to the Hive property name. > > > > E.g. metastore.client.filter.result -> > > hive.metastore.client.filter.result > > > > So this should be: > > METASTORE_CLIENT_FILTER_RESULT("metastore.client.filter.result", > > "hive.metastore.client.filter.result" ...) this is new field, and does not exist in hive configuration. Since we are splitting HMS from Hive, does it make sense to add this new one in hive configuration? > On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote: > > standalone-metastore/metastore-server/pom.xml > > Lines 242 (patched) > > <https://reviews.apache.org/r/69585/diff/2/?file=2114852#file2114852line242> > > > > Nit: Won't this add a Hive dependency to the standalone metastore > > server? > > > > May need to modify this comment here: > > https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/pom.xml#L158 good catch. I have removed this dependency. It is not needed. > On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > > Lines 78 (patched) > > <https://reviews.apache.org/r/69585/diff/2/?file=2114853#file2114853line78> > > > > I don't think this is being used anywhere, could probably remove Yes. I removed it. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69585/#review211446 ----------------------------------------------------------- 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/ > > > Testing > ------- > > Existing unit tests passed. > add new unit tests for filtering at HMS server and HMS client > add code to enabled/disable filtering at HMS client based on configuration > > > Thanks, > > Na Li > >
