kgyrtkirk commented on a change in pull request #2636:
URL: https://github.com/apache/hive/pull/2636#discussion_r766527736
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
##########
@@ -346,11 +346,12 @@ public void alterTable(RawStore msdb, Warehouse wh,
String catName, String dbnam
}
}
Deadline.checkTimeout();
+ Table table = msdb.getTable(catName, newDbName, newTblName);
for (Entry<Partition, ColumnStatistics> partColStats :
columnStatsNeedUpdated.entries()) {
ColumnStatistics newPartColStats = partColStats.getValue();
newPartColStats.getStatsDesc().setDbName(newDbName);
newPartColStats.getStatsDesc().setTableName(newTblName);
- msdb.updatePartitionColumnStatistics(newPartColStats,
partColStats.getKey().getValues(),
+ msdb.updatePartitionColumnStatistics(table, newPartColStats,
partColStats.getKey().getValues(),
Review comment:
looking at the above code - I'm wondering why we need this at all; I
believe `alterPartitions` clears the stat data - but I've not seen it
explicitly - and this added logic here adds it back after that was done
* old values are really removed ?
* can't we simply retain the old stat values - because at the end of the day
that's what happens here...or I've missed something? - doing this would reduce
the number of calls drastically; since we would simply retain things
It also seems like the `alterPartitions` / `alterTable` is also killing the
basic stat state - after a rename I don't think we must do that....
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##########
@@ -9687,31 +9687,35 @@ private void writeMPartitionColumnStatistics(Table
table, Partition partition,
List<ColumnStatisticsObj> statsObjs = colStats.getStatsObj();
ColumnStatisticsDesc statsDesc = colStats.getStatsDesc();
String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() :
getDefaultCatalog(conf);
- MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(),
statsDesc.getTableName());
- Table table = convertToTable(mTable);
- Partition partition = convertToPart(getMPartition(
- catName, statsDesc.getDbName(), statsDesc.getTableName(), partVals,
mTable), false);
- List<String> colNames = new ArrayList<>();
+ MTable mTable = null;
+ if(table == null) {
Review comment:
let's not play with `null` -s and alternate code paths
you decided to change the method signature and added `table` ; fill it out
everywhere or remove the parameter.
--
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]
