You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/11/09 05:02:03 UTC
[GitHub] [hive] shameersss1 opened a new pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
shameersss1 opened a new pull request #2770:
URL: https://github.com/apache/hive/pull/2770
### What changes were proposed in this pull request?
This change will gate #get_table_meta HMS API to abide by any of the authotization model with which the HiveMetastore comes up with
### Why are the changes needed?
When Apache Hue or any other application which uses #get_table_meta API is not gated to use any of the authorization model which HiveMetastore provides when all the tables in the database is shown even if the user doesn't have grant access on the database
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
The changes were tested in a customer Hadoop + Hive + Hue cluster with the changes and verified that the tables in the database is not shown if the user doesn't have grant access on the database
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-968519984
@kgyrtkirk Are we good to take this forward?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on a change in pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on a change in pull request #2770:
URL: https://github.com/apache/hive/pull/2770#discussion_r753058152
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -3633,21 +3633,46 @@ private Table getTableInternal(GetTableRequest getTableRequest) throws MetaExcep
@Override
public List<TableMeta> get_table_meta(String dbnames, String tblNames, List<String> tblTypes)
throws MetaException, NoSuchObjectException {
+
List<TableMeta> t = null;
+ List<TableMeta> finalT = new ArrayList<>();
String[] parsedDbName = parseDbName(dbnames, conf);
startTableFunction("get_table_metas", parsedDbName[CAT_NAME], parsedDbName[DB_NAME], tblNames);
Exception ex = null;
try {
t = getMS().getTableMeta(parsedDbName[CAT_NAME], parsedDbName[DB_NAME], tblNames, tblTypes);
t = FilterUtils.filterTableMetasIfEnabled(isServerFilterEnabled, filterHook,
parsedDbName[CAT_NAME], parsedDbName[DB_NAME], t);
+
+ // metastore authorization : If any listeners are attached then for each tablemeta check if
+ // the database is readable, if so keep the tablemeta else remove it from the final list.
+ if(preListeners.size() > 0) {
+ Map<String, Boolean> databaseNames = new HashMap();
+ for (TableMeta tableMeta : t) {
+ String fullDbName = prependCatalogToDbName(parsedDbName[CAT_NAME], tableMeta.getDbName(), conf);
+ if (databaseNames.get(fullDbName) == null) {
+ boolean isExecptionThrown = false;
+ try {
+ fireReadDatabasePreEvent(fullDbName);
+ } catch (MetaException e) {
+ isExecptionThrown = true;
+ }
+ databaseNames.put(fullDbName, isExecptionThrown);
+ }
+ if (!databaseNames.get(fullDbName)) {
+ finalT.add(tableMeta);
+ }
+ }
+ } else {
+ finalT.addAll(t);
+ }
Review comment:
ack
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -5484,6 +5509,29 @@ private void fireReadTablePreEvent(String catName, String dbName, String tblName
}
}
+ /**
+ * Fire a pre-event for read database operation, if there are any
+ * pre-event listeners registered
+ */
+ private void fireReadDatabasePreEvent(final String name)
+ throws MetaException, NoSuchObjectException {
+ if(preListeners.size() > 0) {
+ // do this only if there is a pre event listener registered (avoid unnecessary
+ // metastore api call)
Review comment:
ack
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 removed a comment on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 removed a comment on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-971208942
@kgyrtkirk - Could you please re-review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kgyrtkirk merged pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
kgyrtkirk merged pull request #2770:
URL: https://github.com/apache/hive/pull/2770
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on a change in pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on a change in pull request #2770:
URL: https://github.com/apache/hive/pull/2770#discussion_r753053040
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -3633,21 +3633,46 @@ private Table getTableInternal(GetTableRequest getTableRequest) throws MetaExcep
@Override
public List<TableMeta> get_table_meta(String dbnames, String tblNames, List<String> tblTypes)
throws MetaException, NoSuchObjectException {
+
List<TableMeta> t = null;
+ List<TableMeta> finalT = new ArrayList<>();
String[] parsedDbName = parseDbName(dbnames, conf);
startTableFunction("get_table_metas", parsedDbName[CAT_NAME], parsedDbName[DB_NAME], tblNames);
Exception ex = null;
try {
t = getMS().getTableMeta(parsedDbName[CAT_NAME], parsedDbName[DB_NAME], tblNames, tblTypes);
t = FilterUtils.filterTableMetasIfEnabled(isServerFilterEnabled, filterHook,
parsedDbName[CAT_NAME], parsedDbName[DB_NAME], t);
+
+ // metastore authorization : If any listeners are attached then for each tablemeta check if
+ // the database is readable, if so keep the tablemeta else remove it from the final list.
+ if(preListeners.size() > 0) {
+ Map<String, Boolean> databaseNames = new HashMap();
+ for (TableMeta tableMeta : t) {
+ String fullDbName = prependCatalogToDbName(parsedDbName[CAT_NAME], tableMeta.getDbName(), conf);
+ if (databaseNames.get(fullDbName) == null) {
+ boolean isExecptionThrown = false;
+ try {
+ fireReadDatabasePreEvent(fullDbName);
+ } catch (MetaException e) {
+ isExecptionThrown = true;
+ }
+ databaseNames.put(fullDbName, isExecptionThrown);
+ }
+ if (!databaseNames.get(fullDbName)) {
+ finalT.add(tableMeta);
+ }
+ }
+ } else {
+ finalT.addAll(t);
+ }
Review comment:
ack
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -5484,6 +5509,29 @@ private void fireReadTablePreEvent(String catName, String dbName, String tblName
}
}
+ /**
+ * Fire a pre-event for read database operation, if there are any
+ * pre-event listeners registered
+ */
+ private void fireReadDatabasePreEvent(final String name)
+ throws MetaException, NoSuchObjectException {
+ if(preListeners.size() > 0) {
+ // do this only if there is a pre event listener registered (avoid unnecessary
+ // metastore api call)
Review comment:
ack
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-974652694
@kgyrtkirk - I have added tests for the same. Could you please re-review it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-964783671
@kgyrtkirk All the tests have passed now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on a change in pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on a change in pull request #2770:
URL: https://github.com/apache/hive/pull/2770#discussion_r753058526
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -5484,6 +5509,29 @@ private void fireReadTablePreEvent(String catName, String dbName, String tblName
}
}
+ /**
+ * Fire a pre-event for read database operation, if there are any
+ * pre-event listeners registered
+ */
+ private void fireReadDatabasePreEvent(final String name)
+ throws MetaException, NoSuchObjectException {
+ if(preListeners.size() > 0) {
+ // do this only if there is a pre event listener registered (avoid unnecessary
+ // metastore api call)
+ String[] parsedDbName = parseDbName(name, conf);
+ Database db = null;
+ try {
+ db = get_database_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME]);
+ if (db == null) {
+ throw new NoSuchObjectException("Database: " + name + " not found");
+ }
+ } catch(MetaException | NoSuchObjectException e) {
+ throw new RuntimeException(e);
Review comment:
firePreEvent throws metaexception when the we don't have permission on the database hence i have catched metaexception and checked for that,
yes it works. I have verified this on the cluster. I am not sure how to add tests for the same. Let me explore that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kgyrtkirk commented on a change in pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #2770:
URL: https://github.com/apache/hive/pull/2770#discussion_r752457342
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -5484,6 +5509,29 @@ private void fireReadTablePreEvent(String catName, String dbName, String tblName
}
}
+ /**
+ * Fire a pre-event for read database operation, if there are any
+ * pre-event listeners registered
+ */
+ private void fireReadDatabasePreEvent(final String name)
+ throws MetaException, NoSuchObjectException {
+ if(preListeners.size() > 0) {
+ // do this only if there is a pre event listener registered (avoid unnecessary
+ // metastore api call)
Review comment:
I think this comment is kinda redundant after `if(preListeners.size() > 0)`
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -3633,21 +3633,46 @@ private Table getTableInternal(GetTableRequest getTableRequest) throws MetaExcep
@Override
public List<TableMeta> get_table_meta(String dbnames, String tblNames, List<String> tblTypes)
throws MetaException, NoSuchObjectException {
+
List<TableMeta> t = null;
+ List<TableMeta> finalT = new ArrayList<>();
String[] parsedDbName = parseDbName(dbnames, conf);
startTableFunction("get_table_metas", parsedDbName[CAT_NAME], parsedDbName[DB_NAME], tblNames);
Exception ex = null;
try {
t = getMS().getTableMeta(parsedDbName[CAT_NAME], parsedDbName[DB_NAME], tblNames, tblTypes);
t = FilterUtils.filterTableMetasIfEnabled(isServerFilterEnabled, filterHook,
parsedDbName[CAT_NAME], parsedDbName[DB_NAME], t);
+
+ // metastore authorization : If any listeners are attached then for each tablemeta check if
+ // the database is readable, if so keep the tablemeta else remove it from the final list.
+ if(preListeners.size() > 0) {
+ Map<String, Boolean> databaseNames = new HashMap();
+ for (TableMeta tableMeta : t) {
+ String fullDbName = prependCatalogToDbName(parsedDbName[CAT_NAME], tableMeta.getDbName(), conf);
+ if (databaseNames.get(fullDbName) == null) {
+ boolean isExecptionThrown = false;
+ try {
+ fireReadDatabasePreEvent(fullDbName);
+ } catch (MetaException e) {
+ isExecptionThrown = true;
+ }
+ databaseNames.put(fullDbName, isExecptionThrown);
+ }
+ if (!databaseNames.get(fullDbName)) {
+ finalT.add(tableMeta);
+ }
+ }
+ } else {
+ finalT.addAll(t);
+ }
Review comment:
I think instead of adding this block here ; you could move this content into a method; and follow the existing
```
t= someFilteringMethod(t, ...)
```
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -5484,6 +5509,29 @@ private void fireReadTablePreEvent(String catName, String dbName, String tblName
}
}
+ /**
+ * Fire a pre-event for read database operation, if there are any
+ * pre-event listeners registered
+ */
+ private void fireReadDatabasePreEvent(final String name)
+ throws MetaException, NoSuchObjectException {
+ if(preListeners.size() > 0) {
+ // do this only if there is a pre event listener registered (avoid unnecessary
+ // metastore api call)
+ String[] parsedDbName = parseDbName(name, conf);
+ Database db = null;
+ try {
+ db = get_database_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME]);
+ if (db == null) {
+ throw new NoSuchObjectException("Database: " + name + " not found");
+ }
+ } catch(MetaException | NoSuchObjectException e) {
+ throw new RuntimeException(e);
Review comment:
you are packaging all exceptions (including `MetaException`) into a `RuntimeException`
a) is this really neccessary?
b) at the call site the catch is only about `MetaException` which in its current form will never be thrown
does this work correctly?
could you add a test?
note: I think it might worth to consider to flatten this code into the new method I suggested to create in the other comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on a change in pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on a change in pull request #2770:
URL: https://github.com/apache/hive/pull/2770#discussion_r753676946
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -5484,6 +5509,29 @@ private void fireReadTablePreEvent(String catName, String dbName, String tblName
}
}
+ /**
+ * Fire a pre-event for read database operation, if there are any
+ * pre-event listeners registered
+ */
+ private void fireReadDatabasePreEvent(final String name)
+ throws MetaException, NoSuchObjectException {
+ if(preListeners.size() > 0) {
+ // do this only if there is a pre event listener registered (avoid unnecessary
+ // metastore api call)
+ String[] parsedDbName = parseDbName(name, conf);
+ Database db = null;
+ try {
+ db = get_database_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME]);
+ if (db == null) {
+ throw new NoSuchObjectException("Database: " + name + " not found");
+ }
+ } catch(MetaException | NoSuchObjectException e) {
+ throw new RuntimeException(e);
Review comment:
Added tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 removed a comment on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 removed a comment on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-968519984
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on a change in pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on a change in pull request #2770:
URL: https://github.com/apache/hive/pull/2770#discussion_r753054946
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -5484,6 +5509,29 @@ private void fireReadTablePreEvent(String catName, String dbName, String tblName
}
}
+ /**
+ * Fire a pre-event for read database operation, if there are any
+ * pre-event listeners registered
+ */
+ private void fireReadDatabasePreEvent(final String name)
+ throws MetaException, NoSuchObjectException {
+ if(preListeners.size() > 0) {
+ // do this only if there is a pre event listener registered (avoid unnecessary
+ // metastore api call)
+ String[] parsedDbName = parseDbName(name, conf);
+ Database db = null;
+ try {
+ db = get_database_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME]);
+ if (db == null) {
+ throw new NoSuchObjectException("Database: " + name + " not found");
+ }
+ } catch(MetaException | NoSuchObjectException e) {
+ throw new RuntimeException(e);
Review comment:
firePreEvent throws metaexception when the we don't have permission on the database hence i have catched metaexception and checked for that,
yes it works. I have verified this on the cluster. I am not sure how to add tests for the same. Let me explore that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-971208942
@kgyrtkirk - Could you please re-review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-974652694
@kgyrtkirk - I have added tests for the same. Could you please re-review it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-964157916
> @kgyrtkirk - Thanks for the review. It looks like there are some tests failures. I am debugging them
I have found the issue and fixed the same. @kgyrtkirk Please do a re-review.
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on a change in pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on a change in pull request #2770:
URL: https://github.com/apache/hive/pull/2770#discussion_r753676946
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -5484,6 +5509,29 @@ private void fireReadTablePreEvent(String catName, String dbName, String tblName
}
}
+ /**
+ * Fire a pre-event for read database operation, if there are any
+ * pre-event listeners registered
+ */
+ private void fireReadDatabasePreEvent(final String name)
+ throws MetaException, NoSuchObjectException {
+ if(preListeners.size() > 0) {
+ // do this only if there is a pre event listener registered (avoid unnecessary
+ // metastore api call)
+ String[] parsedDbName = parseDbName(name, conf);
+ Database db = null;
+ try {
+ db = get_database_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME]);
+ if (db == null) {
+ throw new NoSuchObjectException("Database: " + name + " not found");
+ }
+ } catch(MetaException | NoSuchObjectException e) {
+ throw new RuntimeException(e);
Review comment:
Added tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-976183831
@kgyrtkirk - Thanks for the review. Are we good to merge this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-963820131
@kgyrtkirk Could you please review the changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shameersss1 commented on pull request #2770: HIVE-25680 : Authorize #get_table_meta HiveMetastore Server API to use any of the HiveMetastore Authorization model
Posted by GitBox <gi...@apache.org>.
shameersss1 commented on pull request #2770:
URL: https://github.com/apache/hive/pull/2770#issuecomment-964078299
@kgyrtkirk - Thanks for the review. It looks like there are some tests failures. I am debugging them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org