[ https://issues.apache.org/jira/browse/HIVE-24769?focusedWorklogId=572127&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-572127 ]
ASF GitHub Bot logged work on HIVE-24769: ----------------------------------------- Author: ASF GitHub Bot Created on: 25/Mar/21 17:58 Start Date: 25/Mar/21 17:58 Worklog Time Spent: 10m Work Description: nrg4878 commented on a change in pull request #1970: URL: https://github.com/apache/hive/pull/1970#discussion_r601701019 ########## File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java ########## @@ -3585,31 +3585,35 @@ public GetTablesResult get_table_objects_by_name_req(GetTablesRequest req) throw if (dbName == null || dbName.isEmpty()) { throw new UnknownDBException("DB name is null or empty"); } - if (tableNames == null) { - throw new InvalidOperationException(dbName + " cannot find null tables"); - } - - // The list of table names could contain duplicates. RawStore.getTableObjectsByName() - // only guarantees returning no duplicate table objects in one batch. If we need - // to break into multiple batches, remove duplicates first. - List<String> distinctTableNames = tableNames; - if (distinctTableNames.size() > tableBatchSize) { - List<String> lowercaseTableNames = new ArrayList<>(); - for (String tableName : tableNames) { - lowercaseTableNames.add(org.apache.hadoop.hive.metastore.utils.StringUtils.normalizeIdentifier(tableName)); + RawStore ms = getMS(); + if(tablePattern != null){ Review comment: This if condition seems unnecessary now given that we support both tableNames and tablePattern if provided. Just the code in the else block seems sufficient. No? ########## File path: standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java ########## @@ -2435,8 +2435,24 @@ public Type getType(String name) throws NoSuchObjectException, MetaException, TE @Override public List<String> getTables(String catName, String dbName, String tablePattern) throws TException { - List<String> tables = client.get_tables(prependCatalogToDbName(catName, dbName, conf), tablePattern); - return FilterUtils.filterTableNamesIfEnabled(isClientFilterEnabled, filterHook, catName, dbName, tables); + List<String> tables = new ArrayList<>(); + GetProjectionsSpec projectionsSpec = new GetProjectionsSpec(); + List<String> projectedFields = Arrays.asList("dbName", "tableName", "owner", "ownerType"); + projectionsSpec.setFieldList(projectedFields); + GetTablesRequest req = new GetTablesRequest(dbName); + req.setCatName(catName); + req.setCapabilities(version); + req.setTblNames(tables); + req.setTablesPattern(tablePattern); + if (processorCapabilities != null) Review comment: processorIdentifier also needs to be passed in if set. ########## File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java ########## @@ -1896,7 +1917,12 @@ private MTable getMTable(String catName, String db, String table) { } } else if (projectionFields.size() == 1) { // Execute the query to fetch the partial results. - List<Object> results = (List<Object>) query.execute(db, catName, lowered_tbl_names); + List<Object[]> results = new ArrayList<>(); + if(tablePattern == null) { Review comment: Shouldn't there be 3 possiblities here? tablePattern != null && !lowered_tbl_names.isEmpty() --> execute() needs 4 arguments lowered_tbl_names.isEmpty() && tablePattern != null --> execute() needs 3 arguments !lowered_tbl_names.isEmpty() && tablePattern == null --> execute() needs 3 arguments ########## File path: standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java ########## @@ -2435,8 +2435,24 @@ public Type getType(String name) throws NoSuchObjectException, MetaException, TE @Override public List<String> getTables(String catName, String dbName, String tablePattern) throws TException { - List<String> tables = client.get_tables(prependCatalogToDbName(catName, dbName, conf), tablePattern); - return FilterUtils.filterTableNamesIfEnabled(isClientFilterEnabled, filterHook, catName, dbName, tables); + List<String> tables = new ArrayList<>(); + GetProjectionsSpec projectionsSpec = new GetProjectionsSpec(); + List<String> projectedFields = Arrays.asList("dbName", "tableName", "owner", "ownerType"); Review comment: We should use GetTableProjectionSpecBuilder instead of hard coding the fieldnames. builder.includeDatabase().includeTableName().includeOwner().includeOwnerType() ########## File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java ########## @@ -1877,11 +1889,20 @@ private MTable getMTable(String catName, String db, String table) { } if (projectionFields == null) { - mtables = (List<MTable>) query.execute(db, catName, lowered_tbl_names); + if(tablePattern == null) { + mtables = (List<MTable>) query.execute(db, catName, lowered_tbl_names); + }else{ + mtables = (List<MTable>) query.execute(db, catName, tablePattern); Review comment: Shouldn't there be 3 possiblities here? tablePattern != null && !lowered_tbl_names.isEmpty() --> execute() needs 4 arguments lowered_tbl_names.isEmpty() && tablePattern != null --> execute() needs 3 arguments !lowered_tbl_names.isEmpty() && tablePattern == null --> execute() needs 3 arguments ########## File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java ########## @@ -1853,14 +1853,26 @@ private MTable getMTable(String catName, String db, String table) { db = normalizeIdentifier(db); catName = normalizeIdentifier(catName); - List<String> lowered_tbl_names = new ArrayList<>(tbl_names.size()); - for (String t : tbl_names) { - lowered_tbl_names.add(normalizeIdentifier(t)); + List<String> lowered_tbl_names = new ArrayList<>(); + if(tbl_names != null) { + lowered_tbl_names = new ArrayList<>(tbl_names.size()); + for (String t : tbl_names) { + lowered_tbl_names.add(normalizeIdentifier(t)); + } } query = pm.newQuery(MTable.class); - query.setFilter("database.name == db && database.catalogName == cat && tbl_names.contains(tableName)"); - query.declareParameters("java.lang.String db, java.lang.String cat, java.util.Collection tbl_names"); + + if(tbl_names != null && !tbl_names.isEmpty() && tablePattern != null) { Review comment: Can we simplify these conditions to something like this? filterString = "database.name == db && database.catalogName == cat"; filterParams = "java.lang.String db, java.lang.String cat"; if(tbl_names != null && !tbl_names.isEmpty()) { filterString = filterString + " " + "&& tbl_names.contains(tableName)"; filterParams = filterParams + " " + "java.util.Collection tbl_names"; } if (tablePattern != null) { filterString = filterString + " " + "&& tableName.matches(tablePattern)"); filterParams = filterParams + " " + "java.lang.String tablePattern"; } ########## File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java ########## @@ -1877,11 +1889,20 @@ private MTable getMTable(String catName, String db, String table) { } if (projectionFields == null) { - mtables = (List<MTable>) query.execute(db, catName, lowered_tbl_names); + if(tablePattern == null) { + mtables = (List<MTable>) query.execute(db, catName, lowered_tbl_names); + }else{ + mtables = (List<MTable>) query.execute(db, catName, tablePattern); + } } else { if (projectionFields.size() > 1) { // Execute the query to fetch the partial results. - List<Object[]> results = (List<Object[]>) query.execute(db, catName, lowered_tbl_names); + List<Object[]> results = new ArrayList<>(); + if(tablePattern == null) { Review comment: Shouldn't there be 3 possiblities here? tablePattern != null && !lowered_tbl_names.isEmpty() --> execute() needs 4 arguments lowered_tbl_names.isEmpty() && tablePattern != null --> execute() needs 3 arguments !lowered_tbl_names.isEmpty() && tablePattern == null --> execute() needs 3 arguments -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking ------------------- Worklog Id: (was: 572127) Time Spent: 0.5h (was: 20m) > HiveMetaStore getTables() doesn't have Owner information to filter on it > ------------------------------------------------------------------------ > > Key: HIVE-24769 > URL: https://issues.apache.org/jira/browse/HIVE-24769 > Project: Hive > Issue Type: Improvement > Reporter: Sai Hemanth Gantasala > Assignee: Sai Hemanth Gantasala > Priority: Major > Labels: pull-request-available > Fix For: 4.0.0 > > Time Spent: 0.5h > Remaining Estimate: 0h > > HiveMetaStoreClient#getTables() api should have table owner information so > that they can be used while authorizing in Apache Ranger/Sentry. > -- This message was sent by Atlassian Jira (v8.3.4#803005)