zhangbutao commented on code in PR #6380:
URL: https://github.com/apache/hive/pull/6380#discussion_r3037822412
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -707,6 +707,13 @@ private boolean writeColStats(List<ColumnStatistics>
colStats, Table tbl) {
return false;
}
+ @Override
+ public long getSnapshotId(org.apache.hadoop.hive.ql.metadata.Table hmsTable)
{
+ Table table = IcebergTableUtil.getTable(conf, hmsTable.getTTable());
+ Snapshot snapshot = IcebergTableUtil.getTableSnapshot(table, hmsTable);
Review Comment:
Should we handle the exception case where the table is null?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/RelOptHiveTable.java:
##########
@@ -622,7 +619,7 @@ private void updateColStats(Set<Integer> projIndxLst,
boolean allowMissingStats)
}
}
- if (hiveColStats != null && hiveColStats.size() ==
nonPartColNamesThatRqrStats.size()) {
Review Comment:
Why was the null check removed?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java:
##########
@@ -184,8 +184,11 @@ public static PrunedPartitionList prune(Table tab,
ExprNodeDesc prunerExpr,
String key = tab.getFullyQualifiedName() + ";";
if (tab.getMetaTable() != null) {
key = tab.getFullyQualifiedName() + "." + tab.getMetaTable() + ";";
- } else if (tab.getSnapshotRef() != null) {
- key = tab.getFullyQualifiedName() + "." + tab.getSnapshotRef() + ";";
+ } else if (tab.isNonNative()) {
Review Comment:
Should we currently limit ourselves to only Iceberg tables, rather than all
non-native tables?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/RelOptHiveTable.java:
##########
@@ -586,22 +584,21 @@ private void updateColStats(Set<Integer> projIndxLst,
boolean allowMissingStats)
if (partitionList.getNotDeniedPartns().isEmpty()) {
// no need to make a metastore call
rowCount = 0;
- hiveColStats = new ArrayList<ColStatistics>();
+ hiveColStats = new ArrayList<>();
for (int i = 0; i < nonPartColNamesThatRqrStats.size(); i++) {
// add empty stats object for each column
hiveColStats.add(
new ColStatistics(
nonPartColNamesThatRqrStats.get(i),
hiveNonPartitionColsMap.get(nonPartColIndxsThatRqrStats.get(i)).getTypeName()));
}
- colNamesFailedStats.clear();
Review Comment:
Why was it removed?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/RelOptHiveTable.java:
##########
@@ -514,19 +512,19 @@ private void updateColStats(Set<Integer> projIndxLst,
boolean allowMissingStats)
if (null == partitionList) {
// We could be here either because its an unpartitioned table or because
// there are no pruning predicates on a partitioned table.
- computePartitionList(hiveConf, null, new HashSet<Integer>());
+ computePartitionList(hiveConf, null, new HashSet<>());
}
- String partitionListKey = partitionList.getKey().orElse(null);
Review Comment:
Why was the null key check removed?
--
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]
