HIVE-23820: [HS2] Send tableId in request for get_table_request API

itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java

@@ -322,6 +322,12 @@ public Table getTable(String catName, String dbName, String tableName,
     return objectStore.getTable(catName, dbName, tableName, writeIdList);
   }
 
+  @Override
+  public Table getTable(String catalogName, String dbName, String tableName, String writeIdList, long tableId)
+      throws MetaException {
+    return objectStore.getTable(catalogName, dbName, tableName, writeIdList, tableId);
+  }
+
   @Override
   public boolean addPartition(Partition part)
       throws InvalidObjectException, MetaException {

ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java

@@ -101,6 +101,7 @@
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
 import org.apache.hadoop.hive.metastore.api.GetPartitionsByNamesRequest;
+import org.apache.hadoop.hive.metastore.api.GetTableRequest;
 import org.apache.hadoop.hive.ql.io.HdfsUtils;
 import org.apache.hadoop.hive.metastore.HiveMetaException;
 import org.apache.hadoop.hive.metastore.HiveMetaHook;
@@ -1526,19 +1527,19 @@ public Table getTable(final String dbName, final String tableName, boolean throw
     org.apache.hadoop.hive.metastore.api.Table tTable = null;
     try {
       // Note: this is currently called w/true from StatsOptimizer only.
+      GetTableRequest request = new GetTableRequest(dbName, tableName);
+      request.setCatName(getDefaultCatalog(conf));
+      request.setGetColumnStats(getColumnStats);
+      request.setEngine(Constants.HIVE_ENGINE);
       if (checkTransactional) {
         ValidWriteIdList validWriteIdList = null;
-        long txnId = SessionState.get().getTxnMgr() != null ?
-            SessionState.get().getTxnMgr().getCurrentTxnId() : 0;
+        long txnId = SessionState.get().getTxnMgr() != null ? SessionState.get().getTxnMgr().getCurrentTxnId() : 0;
         if (txnId > 0) {
-          validWriteIdList = AcidUtils.getTableValidWriteIdListWithTxnList(conf,
-              dbName, tableName);
+          validWriteIdList = AcidUtils.getTableValidWriteIdListWithTxnList(conf, dbName, tableName);
         }
-        tTable = getMSC().getTable(getDefaultCatalog(conf), dbName, tableName,
-            validWriteIdList != null ? validWriteIdList.toString() : null, getColumnStats, Constants.HIVE_ENGINE);
-      } else {
-        tTable = getMSC().getTable(dbName, tableName, getColumnStats, Constants.HIVE_ENGINE);
+        request.setValidWriteIdList(validWriteIdList != null ? validWriteIdList.toString() : null);
       }
+      tTable = getMSC().getTable(request);
     } catch (NoSuchObjectException e) {
       if (throwException) {
         throw new InvalidTableException(tableName);

ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java

@@ -233,19 +233,17 @@ public void truncateTable(String dbName, String tableName,
   @Override
   public org.apache.hadoop.hive.metastore.api.Table getTable(String dbname, String name) throws MetaException,
   TException, NoSuchObjectException {
-    return getTable(dbname, name, false, null);
+    GetTableRequest getTableRequest = new GetTableRequest(dbname,name);
+    return getTable(getTableRequest);
   }
 
   @Override
-  public org.apache.hadoop.hive.metastore.api.Table getTable(String dbname, String name,
-      boolean getColStats, String engine) throws MetaException, TException, NoSuchObjectException {
-    // First check temp tables
-    org.apache.hadoop.hive.metastore.api.Table table = getTempTable(dbname, name);
-    if (table != null) {
-      return deepCopy(table);  // Original method used deepCopy(), do the same here.
-    }
-    // Try underlying client
-    return super.getTable(getDefaultCatalog(conf), dbname, name, getColStats, engine);
+  public org.apache.hadoop.hive.metastore.api.Table getTable(String dbname, String name, boolean getColStats,
+      String engine) throws MetaException, TException, NoSuchObjectException {
+    GetTableRequest getTableRequest = new GetTableRequest(dbname, name);
+    getTableRequest.setGetColumnStats(getColStats);
+    getTableRequest.setEngine(engine);
+    return getTable(getTableRequest);
   }
 
   // Need to override this one too or dropTable breaks because it doesn't find the table when checks
@@ -259,16 +257,29 @@ public org.apache.hadoop.hive.metastore.api.Table getTable(String catName, Strin
   // Need to override this one too or dropTable breaks because it doesn't find the table when checks
   // before the drop.
   @Override
-  public org.apache.hadoop.hive.metastore.api.Table getTable(String catName, String dbName,
-          String tableName, boolean getColStats, String engine)
-          throws TException {
+  public org.apache.hadoop.hive.metastore.api.Table getTable(String catName, String dbName, String tableName,
+      boolean getColStats, String engine) throws TException {
+    GetTableRequest getTableRequest = new GetTableRequest(dbName, tableName);
+    getTableRequest.setGetColumnStats(getColStats);
+    getTableRequest.setEngine(engine);
     if (!DEFAULT_CATALOG_NAME.equals(catName)) {
-      return super.getTable(catName, dbName, tableName, getColStats, engine);
+      getTableRequest.setCatName(catName);
+      return super.getTable(getTableRequest);
     } else {
-      return getTable(dbName, tableName, getColStats, engine);
+      return getTable(getTableRequest);
     }
   }
 
+  public org.apache.hadoop.hive.metastore.api.Table getTable(GetTableRequest getTableRequest) throws MetaException, TException, NoSuchObjectException {
+    // First check temp tables
+    org.apache.hadoop.hive.metastore.api.Table table = getTempTable(getTableRequest.getDbName(), getTableRequest.getTblName());
+    if (table != null) {
+      return deepCopy(table);  // Original method used deepCopy(), do the same here.
+    }
+    // Try underlying client
+    return super.getTable(getTableRequest);
+  }
+
   @Override
   public List<String> getAllTables(String dbName) throws MetaException {
     List<String> tableNames = super.getAllTables(dbName);
@@ -2079,15 +2090,26 @@ protected GetTableResult getTableInternal(GetTableRequest req) throws TException
     Map<Object, Object> queryCache = getQueryCache();
     if (queryCache != null) {
       // Retrieve or populate cache
+      CacheKey cacheKeyTableId = new CacheKey(KeyType.TABLE_ID, req.getCatName(), req.getDbName(), req.getTblName());
+      long tableId = -1;
+
+      if (queryCache.containsKey(cacheKeyTableId))
+        tableId = (long) queryCache.get(cacheKeyTableId);
+
+      req.setId(tableId);
       CacheKey cacheKey = new CacheKey(KeyType.TABLE, req);
       GetTableResult v = (GetTableResult) queryCache.get(cacheKey);
       if (v == null) {
         v = super.getTableInternal(req);
+        if (tableId == -1) {
+          queryCache.put(cacheKeyTableId, v.getTable().getId());
+          req.setId(v.getTable().getId());
+          cacheKey = new CacheKey(KeyType.TABLE, req);
+        }
         queryCache.put(cacheKey, v);
       } else {
-        LOG.debug(
-            "Query level HMS cache: method=getTableInternal, dbName={}, tblName={}",
-            req.getDbName(), req.getTblName());
+        LOG.debug("Query level HMS cache: method=getTableInternal, dbName={}, tblName={}", req.getDbName(),
+            req.getTblName());
       }
       return v;
     }

standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java

@@ -2381,82 +2381,141 @@ public Partition getPartitionWithAuthInfo(String catName, String dbName, String
     return deepCopy(FilterUtils.filterPartitionIfEnabled(isClientFilterEnabled, filterHook, p));
   }
 
+  /**
+   * @deprecated use getTable(GetTableRequest getTableRequest)
+   * @param dbname
+   * @param name
+   * @return
+   * @throws TException
+   */
   @Override
+  @Deprecated
   public Table getTable(String dbname, String name) throws TException {
-    return getTable(getDefaultCatalog(conf), dbname, name);
+    GetTableRequest req = new GetTableRequest(dbname, name);
+    req.setCatName(getDefaultCatalog(conf));
+    return getTable(req);
   }
 
+  /**
+   * @deprecated use getTable(GetTableRequest getTableRequest)
+   * @param dbname
+   * @param name
+   * @param getColumnStats
+   *          get the column stats, if available, when true
+   * @param engine engine sending the request
+   * @return
+   * @throws TException
+   */
   @Override
+  @Deprecated
   public Table getTable(String dbname, String name, boolean getColumnStats, String engine) throws TException {
-    return getTable(getDefaultCatalog(conf), dbname, name, getColumnStats, engine);
+    GetTableRequest req = new GetTableRequest(dbname, name);
+    req.setCatName(getDefaultCatalog(conf));
+    req.setGetColumnStats(getColumnStats);
+    if (getColumnStats) {
+      req.setEngine(engine);
+    }
+    return getTable(req);
   }
 
+  /**
+   * @deprecated use getTable(GetTableRequest getTableRequest)
+   * @param catName catalog the table is in.
+   * @param dbName database the table is in.
+   * @param tableName table name.
+   * @return
+   * @throws TException
+   */
   @Override
+  @Deprecated
   public Table getTable(String catName, String dbName, String tableName) throws TException {
-    return getTable(catName, dbName, tableName, false, null);
+    GetTableRequest req = new GetTableRequest(dbName, tableName);
+    req.setCatName(catName);
+    return getTable(req);
   }
 
+  /**
+   * @deprecated use getTable(GetTableRequest getTableRequest)
+   * @param catName
+   * @param dbName
+   * @param tableName
+   * @param getColumnStats
+   * @param engine
+   * @return
+   * @throws TException
+   */
+  @Deprecated
   public Table getTable(String catName, String dbName, String tableName,
       boolean getColumnStats, String engine) throws TException {
-    long t1 = System.currentTimeMillis();
-
-    try {
       GetTableRequest req = new GetTableRequest(dbName, tableName);
       req.setCatName(catName);
-      req.setCapabilities(version);
       req.setGetColumnStats(getColumnStats);
       if (getColumnStats) {
         req.setEngine(engine);
       }
-      if (processorCapabilities != null)
-        req.setProcessorCapabilities(new ArrayList<String>(Arrays.asList(processorCapabilities)));
-      if (processorIdentifier != null)
-        req.setProcessorIdentifier(processorIdentifier);
-
-      Table t = getTableInternal(req).getTable();
-
-      return deepCopy(FilterUtils.filterTableIfEnabled(isClientFilterEnabled, filterHook, t));
-    } finally {
-      long diff = System.currentTimeMillis() - t1;
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("class={}, method={}, duration={}, comments={}", CLASS_NAME, "getTable",
-            diff, "HMS client");
-      }
-    }
+      return getTable(req);
   }
 
   protected GetTableResult getTableInternal(GetTableRequest req) throws TException {
     return client.get_table_req(req);
   }
 
+  /**
+   * @deprecated use getTable(GetTableRequest getTableRequest)
+   * @param catName catalog the table is in.
+   * @param dbName database the table is in.
+   * @param tableName table name.
+   * @param validWriteIdList applicable snapshot
+   * @return
+   * @throws TException
+   */
   @Override
+  @Deprecated
   public Table getTable(String catName, String dbName, String tableName,
       String validWriteIdList) throws TException {
-    return getTable(catName, dbName, tableName, validWriteIdList, false, null);
+    GetTableRequest req = new GetTableRequest(dbName, tableName);
+    req.setCatName(catName);
+    req.setValidWriteIdList(validWriteIdList);
+    return getTable(req);
   }
 
-
+  /**
+   * @deprecated use getTable(GetTableRequest getTableRequest)
+   * @param catName catalog the table is in.
+   * @param dbName database the table is in.
+   * @param tableName table name.
+   * @param validWriteIdList applicable snapshot
+   * @param getColumnStats get the column stats, if available, when true
+   * @param engine engine sending the request
+   * @return
+   * @throws TException
+   */
   @Override
+  @Deprecated
   public Table getTable(String catName, String dbName, String tableName, String validWriteIdList,
       boolean getColumnStats, String engine) throws TException {
+    GetTableRequest req = new GetTableRequest(dbName, tableName);
+    req.setCatName(catName);
+    req.setValidWriteIdList(validWriteIdList);
+    req.setGetColumnStats(getColumnStats);
+    if (getColumnStats) {
+      req.setEngine(engine);
+    }
+    return getTable(req);
+  }
+
+  @Override
+  public Table getTable(GetTableRequest getTableRequest) throws MetaException, TException, NoSuchObjectException {
     long t1 = System.currentTimeMillis();
 
     try {
-      GetTableRequest req = new GetTableRequest(dbName, tableName);
-      req.setCatName(catName);
-      req.setCapabilities(version);
-      req.setValidWriteIdList(validWriteIdList);
-      req.setGetColumnStats(getColumnStats);
-      if (getColumnStats) {
-        req.setEngine(engine);
-      }
+      getTableRequest.setCapabilities(version);
       if (processorCapabilities != null)
-        req.setProcessorCapabilities(new ArrayList<String>(Arrays.asList(processorCapabilities)));
+        getTableRequest.setProcessorCapabilities(new ArrayList<String>(Arrays.asList(processorCapabilities)));
       if (processorIdentifier != null)
-        req.setProcessorIdentifier(processorIdentifier);
-
-      Table t = getTableInternal(req).getTable();
+        getTableRequest.setProcessorIdentifier(processorIdentifier);
 
+      Table t = getTableInternal(getTableRequest).getTable();
       return deepCopy(FilterUtils.filterTableIfEnabled(isClientFilterEnabled, filterHook, t));
     } finally {
       long diff = System.currentTimeMillis() - t1;

standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientWithLocalCache.java

@@ -182,7 +182,9 @@ public enum KeyType {
     // GetValidWriteIdsResponse <-- getValidWriteIdsInternal(GetValidWriteIdsRequest rqst)
     // Stored individually as:
     // TableValidWriteIds <-- String fullTableName, String validTxnList, long writeId
-    VALID_WRITE_IDS(TableValidWriteIds.class, String.class, long.class);
+    VALID_WRITE_IDS(TableValidWriteIds.class, String.class, long.class),
+    // TableId <- String fullTableName
+    TABLE_ID(Long.class, String.class);
 
     private final List<Class<?>> keyClasses;
     private final Class<?> valueClass;