[ 
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)

Reply via email to