[GitHub] [hive] deniskuzZ commented on a diff in pull request #4319: HIVE-25366: Reduce number of Table calls in updatePartitonColStatsInt…

["deniskuzZ (via GitHub)" <gi...@apache.org>]

deniskuzZ commented on code in PR #4319:
URL: https://github.com/apache/hive/pull/4319#discussion_r1196474537


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java:
##########
@@ -1150,10 +1151,14 @@ Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats, Strin
    * @throws InvalidObjectException the stats object is invalid
    * @throws InvalidInputException unable to record the stats for the table
    */
-  Map<String, String> updatePartitionColumnStatistics(ColumnStatistics statsObj,
+  Map<String, String> updatePartitionColumnStatistics(Table table, MTable mTable, ColumnStatistics statsObj,

Review Comment:
   You can't just change the signature for public interface methods. 
   Also looks like in most places we do not reuse mTable. Could you please change only where it's really needed? 



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java:
##########
@@ -1276,7 +1277,7 @@ public Table getTable(String catName, String dbName, String tblName, String vali
 
   @Override public Table getTable(String catName, String dbName, String tblName, String validWriteIds, long tableId)
       throws MetaException {
-    catName = normalizeIdentifier(catName);
+    catName = normalizeIdentifier(catName != null ? catName : getDefaultCatalog(conf));

Review Comment:
   Optional.ofNullable(catName).orElse(getDefaultCatalog(conf))



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java:
##########
@@ -2225,18 +2226,27 @@ private void updatePartitionColumnStatisticsInCache(ColumnStatistics colStats, M
     sharedCache.updatePartitionColStatsInCache(catName, dbName, tblName, partVals, colStats.getStatsObj());
   }
 
-  @Override public Map<String, String> updatePartitionColumnStatistics(ColumnStatistics colStats, List<String> partVals,
+  @Override public Map<String, String> updatePartitionColumnStatistics(Table table, MTable mTable,
+      ColumnStatistics colStats, List<String> partVals,
       String validWriteIds, long writeId)
       throws NoSuchObjectException, MetaException, InvalidObjectException, InvalidInputException {
     Map<String, String> newParams =
-        rawStore.updatePartitionColumnStatistics(colStats, partVals, validWriteIds, writeId);
+        rawStore.updatePartitionColumnStatistics(table, mTable, colStats, partVals, validWriteIds, writeId);
     // in case of event based cache update, cache is updated during commit txn
     if (newParams != null && !canUseEvents) {
       updatePartitionColumnStatisticsInCache(colStats, newParams, partVals);
     }
     return newParams;
   }
 
+  @Override public Map<String, String> updatePartitionColumnStatistics(ColumnStatistics statsObj, List<String> partVals,
+      String validWriteIds, long writeId)
+      throws NoSuchObjectException, MetaException, InvalidObjectException, InvalidInputException {
+    MTable mTable = ensureGetMTable(statsObj.getStatsDesc().getCatName(), statsObj.getStatsDesc().getDbName(), statsObj.getStatsDesc().getTableName());

Review Comment:
   can we create var for statsObj.getStatsDesc(), so it could be more readable (not so long lines)



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -4109,7 +4110,7 @@ private List<Partition> add_partitions_core(final RawStore ms, String catName,
             + TableName.getQualified(catName, dbName, tblName) +
             " does not exist");
       }
-
+      MTable mTable = getMS().ensureGetMTable(catName, dbName, tblName);

Review Comment:
   should we do an evaluation even if later conditions won't trigger updatePartitonColStatsInternal?



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