Copilot commented on code in PR #6188:
URL: https://github.com/apache/hive/pull/6188#discussion_r2527959496
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -9246,85 +9246,93 @@ public Map<String, String>
updateTableColumnStatistics(ColumnStatistics colStats
}
}
- /**
- * Get partition's column stats
- *
- * @return Map of column name and its stats
- */
- private Map<String, MPartitionColumnStatistics> getPartitionColStats(Table
table, String partitionName,
- List<String> colNames, String engine) throws NoSuchObjectException,
MetaException {
- Map<String, MPartitionColumnStatistics> statsMap = Maps.newHashMap();
- List<MPartitionColumnStatistics> stats =
- getMPartitionColumnStatistics(table,
Lists.newArrayList(partitionName), colNames, engine);
- for (MPartitionColumnStatistics cStat : stats) {
- statsMap.put(cStat.getColName(), cStat);
- }
- return statsMap;
- }
-
@Override
public Map<String, String> updatePartitionColumnStatistics(Table table,
MTable mTable, ColumnStatistics colStats,
List<String> partVals, String validWriteIds, long writeId)
throws MetaException, NoSuchObjectException, InvalidObjectException,
InvalidInputException {
boolean committed = false;
-
+ long start = System.currentTimeMillis();
+ List<ColumnStatisticsObj> statsObjs = colStats.getStatsObj();
+ ColumnStatisticsDesc statsDesc = colStats.getStatsDesc();
+ String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() :
getDefaultCatalog(conf);
try {
openTransaction();
- List<ColumnStatisticsObj> statsObjs = colStats.getStatsObj();
- ColumnStatisticsDesc statsDesc = colStats.getStatsDesc();
- String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() :
getDefaultCatalog(conf);
- Partition partition = convertToPart(catName, statsDesc.getDbName(),
statsDesc.getTableName(), getMPartition(
- catName, statsDesc.getDbName(), statsDesc.getTableName(), partVals,
mTable), TxnUtils.isAcidTable(table));
- List<String> colNames = new ArrayList<>();
-
- for(ColumnStatisticsObj statsObj : statsObjs) {
- colNames.add(statsObj.getColName());
- }
-
- Map<String, MPartitionColumnStatistics> oldStats =
getPartitionColStats(table, statsDesc
- .getPartName(), colNames, colStats.getEngine());
-
MPartition mPartition = getMPartition(
catName, statsDesc.getDbName(), statsDesc.getTableName(), partVals,
mTable);
+ Partition partition = convertToPart(catName, statsDesc.getDbName(),
statsDesc.getTableName(),
+ mPartition, TxnUtils.isAcidTable(table));
Review Comment:
The `partition` object is created outside the retry loop (lines 9262-9263)
but used inside the retry loop (line 9298). After `pm.refresh(mPartition)` on
line 9287, the `partition` object may contain stale data since it was converted
from the old `mPartition` state before the refresh.
Consider converting the `Partition` object inside the retry loop after
refreshing `mPartition`, similar to how the table column statistics update
converts the `Table` object inside the retry loop (see line 9188).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
