sankarh commented on a change in pull request #2153:
URL: https://github.com/apache/hive/pull/2153#discussion_r619441006
##########
File path:
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);
+ GetTableRequest getTableRequest = new GetTableRequest(dbname,name);
Review comment:
nit: space after ,
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
##########
@@ -262,11 +260,25 @@ public void truncateTable(String dbName, String tableName,
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);
Review comment:
nit: space after ,
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
##########
@@ -3686,67 +3687,75 @@ private Table getTableInternal(String catName, String
dbname, String name,
}
@Override
+ @Deprecated
public Table get_table_core(
final String catName,
final String dbname,
final String name)
throws MetaException, NoSuchObjectException {
- return get_table_core(catName, dbname, name, null);
+ GetTableRequest getTableRequest = new GetTableRequest(dbname,name);
+ getTableRequest.setCatName(catName);
+ return get_table_core(getTableRequest);
}
@Override
+ @Deprecated
public Table get_table_core(
final String catName,
final String dbname,
final String name,
final String writeIdList)
throws MetaException, NoSuchObjectException {
- return get_table_core(catName, dbname, name, writeIdList, false, null);
+ GetTableRequest getTableRequest = new GetTableRequest(dbname,name);
+ getTableRequest.setCatName(catName);
+ getTableRequest.setValidWriteIdList(writeIdList);
+ return get_table_core(getTableRequest);
}
/**
* This function retrieves table from metastore. If getColumnStats flag is
true,
* then engine should be specified so the table is retrieve with the column
stats
* for that engine.
*/
- public Table get_table_core(final String catName,
- final String dbname,
- final String name,
- final String writeIdList,
- boolean getColumnStats, String engine)
- throws MetaException, NoSuchObjectException {
- Preconditions.checkArgument(!getColumnStats || engine != null,
+ @Override
+ public Table get_table_core(GetTableRequest getTableRequest) throws
MetaException, NoSuchObjectException {
+ Preconditions.checkArgument(!getTableRequest.isGetColumnStats() ||
getTableRequest.getEngine() != null,
"To retrieve column statistics with a table, engine parameter cannot
be null");
Database db = null;
Table t = null;
try {
- db = get_database_core(catName, dbname);
+ db = get_database_core(getTableRequest.getCatName(),
getTableRequest.getDbName());
Review comment:
Shall store cat, db and tbl names in local variables.
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
##########
@@ -2079,11 +2091,21 @@ 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.get(cacheKeyTableId) != null)
Review comment:
nit: need space before ( and also need to use {} for single statement
block as well.
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##########
@@ -1256,7 +1256,11 @@ private void validateTableType(Table tbl) {
return getTable(catName, dbName, tblName, null);
}
- @Override public Table getTable(String catName, String dbName, String
tblName, String validWriteIds)
+ @Override public Table getTable(String catName, String dbName, String
tblName, String validWriteIds) throws MetaException {
Review comment:
nit: Keep annotations and method definition in separate lines.
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
##########
@@ -2079,11 +2091,21 @@ 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.get(cacheKeyTableId) != null)
+ 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);
- queryCache.put(cacheKey, v);
+ if (tableId == -1)
+ queryCache.put(cacheKeyTableId, v.getTable().getId());
+ else
+ queryCache.put(cacheKey, v);
Review comment:
Aren't we suppose to add v into queryCache even if tableId == -1?
Probably need to set id in req object before updating cache entry.
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
##########
@@ -2079,11 +2091,21 @@ 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.get(cacheKeyTableId) != null)
+ 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);
- queryCache.put(cacheKey, v);
+ if (tableId == -1)
Review comment:
nit: need {} for single statement blocks
##########
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java
##########
@@ -3312,6 +3312,12 @@ public Table getTable(String catName, String dbName,
String tableName,
}
@Override
+ public Table getTable(GetTableRequest getTableRequest) throws MetaException,
TException, NoSuchObjectException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+
Review comment:
nit: Remove extra line.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
