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 2022/04/17 16:47:12 UTC
[GitHub] [hive] wecharyu opened a new pull request, #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
wecharyu opened a new pull request, #3218:
URL: https://github.com/apache/hive/pull/3218
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Add some interfaces of `MetaStoreFilterHook` removed while introducing catalogs to Hive 3.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
To keep interfaces compacibility.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
No additional 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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r861733097
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -152,24 +151,31 @@ public enum HivePrivObjectActionType {
private String rowFilterExpression;
public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
Review Comment:
What does dbName mean for a `DFS_URL` or a `SERVICE_NAME`?
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r2426068604
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/AuthorizationMetaStoreFilterHook.java:
##########
@@ -62,19 +65,24 @@ public List<Table> filterTables(List<Table> tableList) throws MetaException {
private List<Table> getFilteredTableList(List<HivePrivilegeObject> hivePrivilegeObjects, List<Table> tableList) {
List<Table> ret = new ArrayList<>();
for(HivePrivilegeObject hivePrivilegeObject:hivePrivilegeObjects) {
+ String catName = hivePrivilegeObject.getCatName();
String dbName = hivePrivilegeObject.getDbname();
String tblName = hivePrivilegeObject.getObjectName();
- Table table = getFilteredTable(dbName,tblName,tableList);
+ Table table = getFilteredTable(catName, dbName, tblName, tableList);
if (table != null) {
ret.add(table);
}
}
return ret;
}
- private Table getFilteredTable(String dbName, String tblName, List<Table> tableList) {
+ private Table getFilteredTable(String catName, String dbName, String tblName, List<Table> tableList) {
Table ret = null;
for (Table table: tableList) {
+ // do not check catalog name if catName is null
+ if (catName != null && table.getCatName() != null && !catName.equals(table.getCatName())) {
Review Comment:
If the input `tableList` belong to different catalogs, this check would take effect. It's similar to the `dbName` check.
--
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] wecharyu commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
wecharyu commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r861774552
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -152,24 +151,31 @@ public enum HivePrivObjectActionType {
private String rowFilterExpression;
public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
Review Comment:
My mistake, these constructors can be optimized.
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r862658017
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchronizer.java:
##########
@@ -199,57 +206,58 @@ public void run() {
LOG.info("Not selected as leader, skip");
continue;
}
- int numDc = 0, numDb = 0, numTbl = 0;
+ int numDc = 0, numCat = 0, numDb = 0, numTbl = 0;
for (String dcName : hiveClient.getAllDataConnectorNames()) {
numDc++;
- HiveObjectRef dcToRefresh = getObjToRefresh(HiveObjectType.DATACONNECTOR, null, dcName);
+ HiveObjectRef dcToRefresh = getObjToRefresh(HiveObjectType.DATACONNECTOR, null, null, dcName);
PrivilegeBag grantDataConnectorBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantDataConnectorBag, HiveObjectType.DATACONNECTOR,
- null, dcName, null, authorizer);
+ addGrantPrivilegesToBag(policyProvider, grantDataConnectorBag, HiveObjectType.DATACONNECTOR, null, null,
+ dcName, null, authorizer);
hiveClient.refresh_privileges(dcToRefresh, authorizer, grantDataConnectorBag);
LOG.debug("processing data connector: " + dcName);
}
LOG.info("Success synchronize privilege " + policyProvider.getClass().getName() + ":" + numDc + " dataconnectors");
- for (String dbName : hiveClient.getAllDatabases()) {
- numDb++;
- HiveObjectRef dbToRefresh = getObjToRefresh(HiveObjectType.DATABASE, dbName, null);
- PrivilegeBag grantDatabaseBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantDatabaseBag, HiveObjectType.DATABASE,
- dbName, null, null, authorizer);
- hiveClient.refresh_privileges(dbToRefresh, authorizer, grantDatabaseBag);
- LOG.debug("processing " + dbName);
+ for (String catName: hiveClient.getCatalogs()) {
+ numCat++;
+ for (String dbName : hiveClient.getAllDatabases(catName)) {
+ numDb++;
+ HiveObjectRef dbToRefresh = getObjToRefresh(HiveObjectType.DATABASE, catName, dbName, null);
+ PrivilegeBag grantDatabaseBag = new PrivilegeBag();
+ addGrantPrivilegesToBag(policyProvider, grantDatabaseBag, HiveObjectType.DATABASE, catName, dbName, null, null,
+ authorizer);
+ hiveClient.refresh_privileges(dbToRefresh, authorizer, grantDatabaseBag);
+ LOG.debug("processing " + dbName);
- for (String tblName : hiveClient.getAllTables(dbName)) {
- numTbl++;
- LOG.debug("processing " + dbName + "." + tblName);
- HiveObjectRef tableToRefresh = getObjToRefresh(HiveObjectType.TABLE, dbName, tblName);
- PrivilegeBag grantTableBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantTableBag, HiveObjectType.TABLE,
- dbName, tblName, null, authorizer);
- hiveClient.refresh_privileges(tableToRefresh, authorizer, grantTableBag);
+ for (String tblName : hiveClient.getAllTables(catName, dbName)) {
+ numTbl++;
+ LOG.debug("processing " + dbName + "." + tblName);
+ HiveObjectRef tableToRefresh = getObjToRefresh(HiveObjectType.TABLE, catName, dbName, tblName);
+ PrivilegeBag grantTableBag = new PrivilegeBag();
+ addGrantPrivilegesToBag(policyProvider, grantTableBag, HiveObjectType.TABLE, catName, dbName, tblName, null,
+ authorizer);
+ hiveClient.refresh_privileges(tableToRefresh, authorizer, grantTableBag);
- HiveObjectRef tableOfColumnsToRefresh = getObjToRefresh(HiveObjectType.COLUMN, dbName, tblName);
- PrivilegeBag grantColumnBag = new PrivilegeBag();
- Table tbl = null;
- try {
- tbl = hiveClient.getTable(dbName, tblName);
- for (FieldSchema fs : tbl.getPartitionKeys()) {
- addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN,
- dbName, tblName, fs.getName(), authorizer);
- }
- for (FieldSchema fs : tbl.getSd().getCols()) {
- addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN,
- dbName, tblName, fs.getName(), authorizer);
+ HiveObjectRef tableOfColumnsToRefresh = getObjToRefresh(HiveObjectType.COLUMN, catName, dbName, tblName);
+ PrivilegeBag grantColumnBag = new PrivilegeBag();
+ Table tbl = null;
+ try {
+ tbl = hiveClient.getTable(catName, dbName, tblName);
+ for (FieldSchema fs : tbl.getPartitionKeys()) {
+ addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN, catName, dbName, tblName, fs.getName(), authorizer);
+ }
+ for (FieldSchema fs : tbl.getSd().getCols()) {
+ addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN, catName, dbName, tblName, fs.getName(), authorizer);
+ }
+ hiveClient.refresh_privileges(tableOfColumnsToRefresh, authorizer, grantColumnBag);
+ } catch (MetaException e) {
+ LOG.debug("Unable to synchronize " + tblName + ":" + e.getMessage());
}
- hiveClient.refresh_privileges(tableOfColumnsToRefresh, authorizer, grantColumnBag);
- } catch (MetaException e) {
- LOG.debug("Unable to synchronize " + tblName + ":" + e.getMessage());
}
}
}
- LOG.info("Success synchronize privilege " + policyProvider.getClass().getName() + ":" + numDb + " databases, "
- + numTbl + " tables");
+ LOG.info(String.format("Success synchronize privilege %s : %d catalogs, %d databases, %d tables.",
Review Comment:
Maybe just keep as the other part with concatenation so at least it is consistens inside the file?
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchronizer.java:
##########
@@ -199,57 +206,58 @@ public void run() {
LOG.info("Not selected as leader, skip");
continue;
}
- int numDc = 0, numDb = 0, numTbl = 0;
+ int numDc = 0, numCat = 0, numDb = 0, numTbl = 0;
for (String dcName : hiveClient.getAllDataConnectorNames()) {
numDc++;
- HiveObjectRef dcToRefresh = getObjToRefresh(HiveObjectType.DATACONNECTOR, null, dcName);
+ HiveObjectRef dcToRefresh = getObjToRefresh(HiveObjectType.DATACONNECTOR, null, null, dcName);
PrivilegeBag grantDataConnectorBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantDataConnectorBag, HiveObjectType.DATACONNECTOR,
- null, dcName, null, authorizer);
+ addGrantPrivilegesToBag(policyProvider, grantDataConnectorBag, HiveObjectType.DATACONNECTOR, null, null,
+ dcName, null, authorizer);
hiveClient.refresh_privileges(dcToRefresh, authorizer, grantDataConnectorBag);
LOG.debug("processing data connector: " + dcName);
}
LOG.info("Success synchronize privilege " + policyProvider.getClass().getName() + ":" + numDc + " dataconnectors");
- for (String dbName : hiveClient.getAllDatabases()) {
- numDb++;
- HiveObjectRef dbToRefresh = getObjToRefresh(HiveObjectType.DATABASE, dbName, null);
- PrivilegeBag grantDatabaseBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantDatabaseBag, HiveObjectType.DATABASE,
- dbName, null, null, authorizer);
- hiveClient.refresh_privileges(dbToRefresh, authorizer, grantDatabaseBag);
- LOG.debug("processing " + dbName);
+ for (String catName: hiveClient.getCatalogs()) {
+ numCat++;
+ for (String dbName : hiveClient.getAllDatabases(catName)) {
+ numDb++;
+ HiveObjectRef dbToRefresh = getObjToRefresh(HiveObjectType.DATABASE, catName, dbName, null);
+ PrivilegeBag grantDatabaseBag = new PrivilegeBag();
+ addGrantPrivilegesToBag(policyProvider, grantDatabaseBag, HiveObjectType.DATABASE, catName, dbName, null, null,
+ authorizer);
+ hiveClient.refresh_privileges(dbToRefresh, authorizer, grantDatabaseBag);
+ LOG.debug("processing " + dbName);
- for (String tblName : hiveClient.getAllTables(dbName)) {
- numTbl++;
- LOG.debug("processing " + dbName + "." + tblName);
- HiveObjectRef tableToRefresh = getObjToRefresh(HiveObjectType.TABLE, dbName, tblName);
- PrivilegeBag grantTableBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantTableBag, HiveObjectType.TABLE,
- dbName, tblName, null, authorizer);
- hiveClient.refresh_privileges(tableToRefresh, authorizer, grantTableBag);
+ for (String tblName : hiveClient.getAllTables(catName, dbName)) {
+ numTbl++;
+ LOG.debug("processing " + dbName + "." + tblName);
+ HiveObjectRef tableToRefresh = getObjToRefresh(HiveObjectType.TABLE, catName, dbName, tblName);
+ PrivilegeBag grantTableBag = new PrivilegeBag();
+ addGrantPrivilegesToBag(policyProvider, grantTableBag, HiveObjectType.TABLE, catName, dbName, tblName, null,
+ authorizer);
+ hiveClient.refresh_privileges(tableToRefresh, authorizer, grantTableBag);
- HiveObjectRef tableOfColumnsToRefresh = getObjToRefresh(HiveObjectType.COLUMN, dbName, tblName);
- PrivilegeBag grantColumnBag = new PrivilegeBag();
- Table tbl = null;
- try {
- tbl = hiveClient.getTable(dbName, tblName);
- for (FieldSchema fs : tbl.getPartitionKeys()) {
- addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN,
- dbName, tblName, fs.getName(), authorizer);
- }
- for (FieldSchema fs : tbl.getSd().getCols()) {
- addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN,
- dbName, tblName, fs.getName(), authorizer);
+ HiveObjectRef tableOfColumnsToRefresh = getObjToRefresh(HiveObjectType.COLUMN, catName, dbName, tblName);
+ PrivilegeBag grantColumnBag = new PrivilegeBag();
+ Table tbl = null;
+ try {
+ tbl = hiveClient.getTable(catName, dbName, tblName);
+ for (FieldSchema fs : tbl.getPartitionKeys()) {
+ addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN, catName, dbName, tblName, fs.getName(), authorizer);
+ }
+ for (FieldSchema fs : tbl.getSd().getCols()) {
+ addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN, catName, dbName, tblName, fs.getName(), authorizer);
+ }
+ hiveClient.refresh_privileges(tableOfColumnsToRefresh, authorizer, grantColumnBag);
+ } catch (MetaException e) {
+ LOG.debug("Unable to synchronize " + tblName + ":" + e.getMessage());
}
- hiveClient.refresh_privileges(tableOfColumnsToRefresh, authorizer, grantColumnBag);
- } catch (MetaException e) {
- LOG.debug("Unable to synchronize " + tblName + ":" + e.getMessage());
}
}
}
- LOG.info("Success synchronize privilege " + policyProvider.getClass().getName() + ":" + numDb + " databases, "
- + numTbl + " tables");
+ LOG.info(String.format("Success synchronize privilege %s : %d catalogs, %d databases, %d tables.",
Review Comment:
Maybe just keep as the other part with concatenation so at least it is consistent inside the file?
--
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] wecharyu commented on pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
wecharyu commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-1189229215
Gentle Ping @pvary @nrg4878
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r2425126935
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -144,20 +162,22 @@ public enum HivePrivObjectActionType {
// is applied to the table.
private String rowFilterExpression;
- public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
- this(type, dbname, objectName, HivePrivObjectActionType.OTHER);
+ public HivePrivilegeObject(HivePrivilegeObjectType type, String objectName) {
Review Comment:
The ranger or spark compile would fail only when upgrading the hive version. Is it acceptable to make the ranger and spark compatible with this change when they upgrade hive version?
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3391604028
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218) **Quality Gate passed**
Issues
 [54 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218)
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3409624974
Merged. Thanks @wecharyu ! Thanks @pvary for the 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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r861554859
##########
ql/src/test/results/clientnegative/authorization_grant_table_allpriv.q.out:
##########
@@ -15,4 +15,4 @@ POSTHOOK: Output: default@table_priv_allf
PREHOOK: query: GRANT ALL ON table_priv_allf TO USER user3
PREHOOK: type: GRANT_PRIVILEGE
PREHOOK: Output: default@table_priv_allf
-FAILED: Execution Error, return code 40000 from org.apache.hadoop.hive.ql.ddl.DDLTask. Permission denied: Principal [name=user2, type=USER] does not have following privileges for operation GRANT_PRIVILEGE [[SELECT with grant, UPDATE with grant, DELETE with grant] on Object [type=TABLE_OR_VIEW, name=default.table_priv_allf]]
+FAILED: Execution Error, return code 40000 from org.apache.hadoop.hive.ql.ddl.DDLTask. Permission denied: Principal [name=user2, type=USER] does not have following privileges for operation GRANT_PRIVILEGE [[SELECT with grant, UPDATE with grant, DELETE with grant] on Object [type=TABLE_OR_VIEW, name=null.default.table_priv_allf]]
Review Comment:
this `null` seems problematic
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3397672446
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218) **Quality Gate passed**
Issues
 [81 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218)
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r2425135876
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -144,20 +162,22 @@ public enum HivePrivObjectActionType {
// is applied to the table.
private String rowFilterExpression;
- public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
- this(type, dbname, objectName, HivePrivObjectActionType.OTHER);
+ public HivePrivilegeObject(HivePrivilegeObjectType type, String objectName) {
Review Comment:
If the Hive dependencies for Spark or Ranger haven't been upgraded, Ranger can still use lower-version(<4.0) Hive dependencies to authorize operations on higher-version(4.2) Hive, as long as the Hive APIs used remain backward compatible. However, if the higher-version(4.2) Hive API (such as HivePrivilegeObject) introduces compatibility-breaking changes, Ranger will no longer be able to use lower-version(<4.0) Hive dependencies to authorize higher-version Hive. In such cases, Ranger and Spark would have to upgrade their Hive version, but this upgrade process could be very slow (for example, as seen in https://github.com/apache/spark/pull/52099/files). During this period, users might be unable to use Ranger for authorization, leading to significant usage issues.
Therefore, it is necessary to retain the original API, which at least ensures that Ranger can authorize databases and tables under the default `hive `catalog without upgrading its Hive dependencies.
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r861554558
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java:
##########
@@ -430,7 +430,8 @@ public List<HivePrivilegeInfo> showPrivileges(HivePrincipal principal, HivePrivi
HivePrivilegeObject resPrivObj = new HivePrivilegeObject(
getPluginPrivilegeObjType(msObjRef.getObjectType()), msObjRef.getDbName(),
- msObjRef.getObjectName(), msObjRef.getPartValues(), msObjRef.getColumnName());
+ msObjRef.getObjectName(), msObjRef.getPartValues(),
Review Comment:
Catname?
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r862656055
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchronizer.java:
##########
@@ -89,18 +89,21 @@ private void addACLsToBag(
for (Map.Entry<HiveResourceACLs.Privilege, HiveResourceACLs.AccessResult> acl : principalAcls.getValue()
.entrySet()) {
if (acl.getValue() == HiveResourceACLs.AccessResult.ALLOWED) {
+ HiveObjectRef hiveObjectRef = null;
switch (objectType) {
case DATABASE:
+ hiveObjectRef = new HiveObjectRef(HiveObjectType.DATABASE, dbName, null, null, null);
Review Comment:
Shall we set the catalogname to the `HiveObjectRef`?
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchronizer.java:
##########
@@ -89,18 +89,21 @@ private void addACLsToBag(
for (Map.Entry<HiveResourceACLs.Privilege, HiveResourceACLs.AccessResult> acl : principalAcls.getValue()
.entrySet()) {
if (acl.getValue() == HiveResourceACLs.AccessResult.ALLOWED) {
+ HiveObjectRef hiveObjectRef = null;
switch (objectType) {
case DATABASE:
+ hiveObjectRef = new HiveObjectRef(HiveObjectType.DATABASE, dbName, null, null, null);
+ hiveObjectRef.setCatName(catName);
privBag.addToPrivileges(
- new HiveObjectPrivilege(new HiveObjectRef(HiveObjectType.DATABASE, dbName, null, null, null), principal,
- principalType, new PrivilegeGrantInfo(acl.getKey().toString(),
- (int) (System.currentTimeMillis() / 1000), GRANTOR, PrincipalType.USER, false), authorizer));
+ new HiveObjectPrivilege(hiveObjectRef, principal, principalType, new PrivilegeGrantInfo(acl.getKey().toString(),
Review Comment:
Same as above
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r2425142078
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -144,20 +162,22 @@ public enum HivePrivObjectActionType {
// is applied to the table.
private String rowFilterExpression;
- public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
- this(type, dbname, objectName, HivePrivObjectActionType.OTHER);
+ public HivePrivilegeObject(HivePrivilegeObjectType type, String objectName) {
Review Comment:
Make sense.
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3402209726
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218) **Quality Gate passed**
Issues
 [79 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218)
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3390799540
@zhangbutao pls take a look.
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r861552317
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchronizer.java:
##########
@@ -170,7 +170,8 @@ private void addGrantPrivilegesToBag(HivePolicyProvider policyProvider, Privileg
case COLUMN:
objectAcls = policyProvider
- .getResourceACLs(new HivePrivilegeObject(HivePrivilegeObjectType.COLUMN, dbName, objName, null, columnName));
+ .getResourceACLs(new HivePrivilegeObject(HivePrivilegeObjectType.COLUMN, dbName, objName, null,
Review Comment:
Shouldn't we need a catalog name here?
--
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] pvary commented on pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-1114629717
@wecharyu: Mostly LGTM. I would like to see @nrg4878's opinion as well, since he is more knowledgeable around HMS.
Thanks,
Peter
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r857332648
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/AuthorizationMetaStoreFilterHook.java:
##########
@@ -46,11 +46,18 @@ public AuthorizationMetaStoreFilterHook(Configuration conf) {
}
@Override
- public List<String> filterTableNames(String catName, String dbName, List<String> tableList)
+ public List<String> filterTableNames(String dbName, List<String> tableList)
throws MetaException {
List<HivePrivilegeObject> listObjs = getHivePrivObjects(dbName, tableList);
return getTableNames(getFilteredObjects(listObjs));
}
+
+ @Override
+ public List<String> filterTableNames(String catName, String dbName, List<String> tableList)
+ throws MetaException {
+ return filterTableNames(dbName, tableList);
Review Comment:
This seems problematic to me.
If we ignore the catalog name, that could become a serious hidden issue.
Do we have a better solution to this?
Maybe throw an exception if the catalog name is not the default? Or do the filtering correctly with the catalog info as well?
--
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] github-actions[bot] commented on pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-1250162094
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r861554295
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/filtercontext/DatabaseFilterContext.java:
##########
@@ -58,7 +58,7 @@ private List<HivePrivilegeObject> getInputHObjs() {
HivePrivilegeObjectType type = HivePrivilegeObjectType.DATABASE;
HivePrivObjectActionType objectActionType = HivePrivObjectActionType.OTHER;
HivePrivilegeObject hivePrivilegeObject =
- new HivePrivilegeObject(type, database, null, null, null, objectActionType, null, null);
+ new HivePrivilegeObject(type, database, null, null, null, objectActionType, null);
Review Comment:
Maybe adding a catalog here too?
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3396494437
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218) **Quality Gate passed**
Issues
 [79 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218)
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3401154536
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218) **Quality Gate passed**
Issues
 [79 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218)
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r2422620893
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -144,20 +162,22 @@ public enum HivePrivObjectActionType {
// is applied to the table.
private String rowFilterExpression;
- public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
- this(type, dbname, objectName, HivePrivObjectActionType.OTHER);
+ public HivePrivilegeObject(HivePrivilegeObjectType type, String objectName) {
Review Comment:
Apache Spark also uses `HivePrivilegeObject ` :
https://github.com/apache/spark/blob/c5ff48cc2b2c5a1526789ae414ff4c63b053d3ec/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/GetColumnsOperation.java#L223
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao merged PR #3218:
URL: https://github.com/apache/hive/pull/3218
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r862657620
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchronizer.java:
##########
@@ -199,57 +206,58 @@ public void run() {
LOG.info("Not selected as leader, skip");
continue;
}
- int numDc = 0, numDb = 0, numTbl = 0;
+ int numDc = 0, numCat = 0, numDb = 0, numTbl = 0;
for (String dcName : hiveClient.getAllDataConnectorNames()) {
numDc++;
- HiveObjectRef dcToRefresh = getObjToRefresh(HiveObjectType.DATACONNECTOR, null, dcName);
+ HiveObjectRef dcToRefresh = getObjToRefresh(HiveObjectType.DATACONNECTOR, null, null, dcName);
PrivilegeBag grantDataConnectorBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantDataConnectorBag, HiveObjectType.DATACONNECTOR,
- null, dcName, null, authorizer);
+ addGrantPrivilegesToBag(policyProvider, grantDataConnectorBag, HiveObjectType.DATACONNECTOR, null, null,
+ dcName, null, authorizer);
hiveClient.refresh_privileges(dcToRefresh, authorizer, grantDataConnectorBag);
LOG.debug("processing data connector: " + dcName);
}
LOG.info("Success synchronize privilege " + policyProvider.getClass().getName() + ":" + numDc + " dataconnectors");
- for (String dbName : hiveClient.getAllDatabases()) {
- numDb++;
- HiveObjectRef dbToRefresh = getObjToRefresh(HiveObjectType.DATABASE, dbName, null);
- PrivilegeBag grantDatabaseBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantDatabaseBag, HiveObjectType.DATABASE,
- dbName, null, null, authorizer);
- hiveClient.refresh_privileges(dbToRefresh, authorizer, grantDatabaseBag);
- LOG.debug("processing " + dbName);
+ for (String catName: hiveClient.getCatalogs()) {
+ numCat++;
+ for (String dbName : hiveClient.getAllDatabases(catName)) {
+ numDb++;
+ HiveObjectRef dbToRefresh = getObjToRefresh(HiveObjectType.DATABASE, catName, dbName, null);
+ PrivilegeBag grantDatabaseBag = new PrivilegeBag();
+ addGrantPrivilegesToBag(policyProvider, grantDatabaseBag, HiveObjectType.DATABASE, catName, dbName, null, null,
+ authorizer);
+ hiveClient.refresh_privileges(dbToRefresh, authorizer, grantDatabaseBag);
+ LOG.debug("processing " + dbName);
- for (String tblName : hiveClient.getAllTables(dbName)) {
- numTbl++;
- LOG.debug("processing " + dbName + "." + tblName);
- HiveObjectRef tableToRefresh = getObjToRefresh(HiveObjectType.TABLE, dbName, tblName);
- PrivilegeBag grantTableBag = new PrivilegeBag();
- addGrantPrivilegesToBag(policyProvider, grantTableBag, HiveObjectType.TABLE,
- dbName, tblName, null, authorizer);
- hiveClient.refresh_privileges(tableToRefresh, authorizer, grantTableBag);
+ for (String tblName : hiveClient.getAllTables(catName, dbName)) {
+ numTbl++;
+ LOG.debug("processing " + dbName + "." + tblName);
+ HiveObjectRef tableToRefresh = getObjToRefresh(HiveObjectType.TABLE, catName, dbName, tblName);
+ PrivilegeBag grantTableBag = new PrivilegeBag();
+ addGrantPrivilegesToBag(policyProvider, grantTableBag, HiveObjectType.TABLE, catName, dbName, tblName, null,
+ authorizer);
+ hiveClient.refresh_privileges(tableToRefresh, authorizer, grantTableBag);
- HiveObjectRef tableOfColumnsToRefresh = getObjToRefresh(HiveObjectType.COLUMN, dbName, tblName);
- PrivilegeBag grantColumnBag = new PrivilegeBag();
- Table tbl = null;
- try {
- tbl = hiveClient.getTable(dbName, tblName);
- for (FieldSchema fs : tbl.getPartitionKeys()) {
- addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN,
- dbName, tblName, fs.getName(), authorizer);
- }
- for (FieldSchema fs : tbl.getSd().getCols()) {
- addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN,
- dbName, tblName, fs.getName(), authorizer);
+ HiveObjectRef tableOfColumnsToRefresh = getObjToRefresh(HiveObjectType.COLUMN, catName, dbName, tblName);
+ PrivilegeBag grantColumnBag = new PrivilegeBag();
+ Table tbl = null;
+ try {
+ tbl = hiveClient.getTable(catName, dbName, tblName);
+ for (FieldSchema fs : tbl.getPartitionKeys()) {
+ addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN, catName, dbName, tblName, fs.getName(), authorizer);
+ }
+ for (FieldSchema fs : tbl.getSd().getCols()) {
+ addGrantPrivilegesToBag(policyProvider, grantColumnBag, HiveObjectType.COLUMN, catName, dbName, tblName, fs.getName(), authorizer);
+ }
+ hiveClient.refresh_privileges(tableOfColumnsToRefresh, authorizer, grantColumnBag);
+ } catch (MetaException e) {
+ LOG.debug("Unable to synchronize " + tblName + ":" + e.getMessage());
}
- hiveClient.refresh_privileges(tableOfColumnsToRefresh, authorizer, grantColumnBag);
- } catch (MetaException e) {
- LOG.debug("Unable to synchronize " + tblName + ":" + e.getMessage());
}
}
}
- LOG.info("Success synchronize privilege " + policyProvider.getClass().getName() + ":" + numDb + " databases, "
- + numTbl + " tables");
+ LOG.info(String.format("Success synchronize privilege %s : %d catalogs, %d databases, %d tables.",
Review Comment:
Maybe?
```
LOG.info("Success synchronize privilege {} :{} catalogs, {} databases, {} tables.",numCat, numDb, numTbl);
```
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r862684011
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java:
##########
@@ -1296,7 +1299,7 @@ public List<String> getDatabases(String databasePattern)
@Override
public List<String> getAllDatabases() throws MetaException {
try {
- return filterHook.filterDatabases(client.get_all_databases());
+ return filterHook.filterDatabases(null, client.get_all_databases());
Review Comment:
Should this be `getDefaultCatalog(conf)` instead off null?
--
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] wecharyu commented on pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
wecharyu commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-1101518363
@deniskuzZ @pvary Could you please review this PR?
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r2422617522
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -144,20 +162,22 @@ public enum HivePrivObjectActionType {
// is applied to the table.
private String rowFilterExpression;
- public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
- this(type, dbname, objectName, HivePrivObjectActionType.OTHER);
+ public HivePrivilegeObject(HivePrivilegeObjectType type, String objectName) {
Review Comment:
This change here introduces a breaking compatibility issue for downstream systems like Apache Ranger. Apache Ranger utilizes the `HivePrivilegeObject` class from Hive for initializing relevant authorization objects, and there are multiple places where the constructors of `HivePrivilegeObject` are used.
For example：
https://github.com/apache/ranger/blob/0e12d7317169d1e4c7f9712f4ae17baf6aeeed1e/hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java#L2587,
and https://github.com/apache/ranger/blob/0e12d7317169d1e4c7f9712f4ae17baf6aeeed1e/hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java#L2747.
We need to consider retaining these existing constructors to avoid compatibility issues for downstream systems like Apache Ranger.
My idea is to wait until Hive has fully implemented all the features of the Federated Catalog, and Apache Ranger has adopted the new `HivePrivilegeObject` constructors to implement Hive's catalog related authorization. Only then can we safely remove the original `HivePrivilegeObject` constructors.
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r2425804085
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/AuthorizationMetaStoreFilterHook.java:
##########
@@ -62,19 +65,24 @@ public List<Table> filterTables(List<Table> tableList) throws MetaException {
private List<Table> getFilteredTableList(List<HivePrivilegeObject> hivePrivilegeObjects, List<Table> tableList) {
List<Table> ret = new ArrayList<>();
for(HivePrivilegeObject hivePrivilegeObject:hivePrivilegeObjects) {
+ String catName = hivePrivilegeObject.getCatName();
String dbName = hivePrivilegeObject.getDbname();
String tblName = hivePrivilegeObject.getObjectName();
- Table table = getFilteredTable(dbName,tblName,tableList);
+ Table table = getFilteredTable(catName, dbName, tblName, tableList);
if (table != null) {
ret.add(table);
}
}
return ret;
}
- private Table getFilteredTable(String dbName, String tblName, List<Table> tableList) {
+ private Table getFilteredTable(String catName, String dbName, String tblName, List<Table> tableList) {
Table ret = null;
for (Table table: tableList) {
+ // do not check catalog name if catName is null
+ if (catName != null && table.getCatName() != null && !catName.equals(table.getCatName())) {
Review Comment:
I don't understand the change here. This line of code seems redundant. Under what circumstances would `!catName.equals(table.getCatName())` be true?
--
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] github-actions[bot] closed pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
URL: https://github.com/apache/hive/pull/3218
--
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] pvary commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r861553706
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -152,24 +151,31 @@ public enum HivePrivObjectActionType {
private String rowFilterExpression;
public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
Review Comment:
Shall we deprecate the old constructors?
--
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] wecharyu commented on a diff in pull request #3218: HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs.
Posted by GitBox <gi...@apache.org>.
wecharyu commented on code in PR #3218:
URL: https://github.com/apache/hive/pull/3218#discussion_r861731060
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java:
##########
@@ -152,24 +151,31 @@ public enum HivePrivObjectActionType {
private String rowFilterExpression;
public HivePrivilegeObject(HivePrivilegeObjectType type, String dbname, String objectName) {
Review Comment:
This constructor can be useful for `HivePrivilegeObjectType` like `DFS_URL`, `SERVICE_NAME` etc.
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3386200762
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218) **Quality Gate passed**
Issues
 [54 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218)
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3400111730
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218) **Quality Gate passed**
Issues
 [79 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218)
--
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
Re: [PR] HIVE-26148: Keep MetaStoreFilterHook interface compatibility after introducing catalogs. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3218:
URL: https://github.com/apache/hive/pull/3218#issuecomment-3392836576
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218) **Quality Gate passed**
Issues
 [54 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3218&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3218&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3218&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3218)
--
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