----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69585/#review211446 -----------------------------------------------------------
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java Lines 816 (patched) <https://reviews.apache.org/r/69585/#comment296572> Nit: This might benefit from some extra spacing or spreading them out into if statements. I had a difficult time reading the nested ternary operators (or maybe that's just me?) standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java Lines 2458 (patched) <https://reviews.apache.org/r/69585/#comment296580> 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? standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 662 (patched) <https://reviews.apache.org/r/69585/#comment296573> 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" ...) standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 663 (patched) <https://reviews.apache.org/r/69585/#comment296574> I think it'd be useful to add what the default value will be to the description. So add "Defaults to true". standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 664 (patched) <https://reviews.apache.org/r/69585/#comment296575> Same here - second value should be the Hive property name. standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 665 (patched) <https://reviews.apache.org/r/69585/#comment296576> Same here - add default value to the description standalone-metastore/metastore-server/pom.xml Lines 242 (patched) <https://reviews.apache.org/r/69585/#comment296577> 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 standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java Lines 78 (patched) <https://reviews.apache.org/r/69585/#comment296578> I don't think this is being used anywhere, could probably remove standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java Lines 4621 (patched) <https://reviews.apache.org/r/69585/#comment296579> This is the same method as the one on the HiveMetaStoreClient. Would it be possible to move it to a common method under hive-standalone-metastore-common? - Morio Ramdenbourg 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 > >
