difin commented on code in PR #6662:
URL: https://github.com/apache/hive/pull/6662#discussion_r3708654150


##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -207,38 +197,73 @@ private static Statistics collectStatistics(HiveConf 
conf, PrunedPartitionList p
   public static long getNumRows(HiveConf conf, List<ColumnInfo> schema, Table 
table, PrunedPartitionList partitionList, 
       AtomicInteger noColsMissingStats) {
 
-    List<Partish> inputs = new ArrayList<>();
-    if (table.isPartitioned()) {
-      for (Partition part : partitionList.getNotDeniedPartns()) {
-        inputs.add(Partish.buildFor(table, part));
-      }
-    } else {
-      inputs.add(Partish.buildFor(table));
-    }
-
     Factory basicStatsFactory = new BasicStats.Factory();
 
     if (HiveConf.getBoolVar(conf, ConfVars.HIVE_STATS_ESTIMATE_STATS)) {
       basicStatsFactory.addEnhancer(new BasicStats.DataSizeEstimator(conf));
       basicStatsFactory.addEnhancer(new 
BasicStats.RowNumEstimator(estimateRowSizeFromSchema(conf, schema)));
     }
-    
-    for (Partish pi : inputs) {
-      BasicStats bStats = new BasicStats(pi);
-      long nr = bStats.getNumRows();
-      // FIXME: this point will be lost after the factory; check that it's 
really a warning....cleanup/etc
-      if (nr <= 0) {
-        // log warning if row count is missing
-        noColsMissingStats.getAndIncrement();
-      }
+
+    // when partition-level statistics are unavailable (e.g. non-native table 
with an external stats
+    // source) fall back to the table-level statistics rather than 
per-partition minimums
+    List<BasicStats> results;
+    if (table.isPartitioned() && checkCanProvidePartitionStats(table)) {
+      results = buildPartitionStats(conf, table, partitionList, 
basicStatsFactory);
+    } else {
+      results = List.of(buildTableStats(table, basicStatsFactory));
     }
-    List<BasicStats> results = basicStatsFactory.buildAll(conf, inputs);
+    // count the entries with missing row counts (estimated rows do not count 
as provided)
+    noColsMissingStats.addAndGet((int) results.stream()
+        .filter(bStats -> bStats.getRawNumRows() <= 0)
+        .count());
     BasicStats aggregateStat = BasicStats.buildFrom(results);
 
     aggregateStat.apply(new BasicStats.SetMinRowNumber());
     return aggregateStat.getNumRows();
   }
 
+  /**
+   * Builds the per-partition basic stats. When the storage handler provides 
them, one batched read
+   * (see {@link HiveStorageHandler#getAggrBasicStatsFor}) serves the whole 
partition list; otherwise each
+   * partition is read individually (see {@link BasicStats.Factory#buildAll}).
+   */
+  private static List<BasicStats> buildPartitionStats(HiveConf conf, Table 
table, PrunedPartitionList partList,
+      BasicStats.Factory factory) {
+    List<Partish> inputs = partList.getNotDeniedPartns().stream()
+        .map(part -> Partish.buildFor(table, part))
+        .toList();
+    HiveStorageHandler storageHandler = table.isNonNative() ? 
table.getStorageHandler() : null;
+    if (storageHandler != null && storageHandler.canProvideBasicStatistics()) {
+      if (partList.getReferredPartCols().isEmpty() && !inputs.isEmpty()) {
+        // no partition predicate: a non-empty list covers every partition, so 
the table-level statistics
+        // are exactly their aggregate - skip the per-partition read
+        return List.of(buildTableStats(table, factory));
+      }
+      List<String> partNames = inputs.stream()
+          .map(partish -> partish.getPartition().getName())
+          .toList();
+      Map<String, Map<String, String>> aggrBasicStats = partNames.isEmpty() ? 
Map.of() :
+          storageHandler.getAggrBasicStatsFor(table, partNames);
+      if (!aggrBasicStats.isEmpty()) {
+        return inputs.stream()
+            .map(pi -> factory.build(pi,
+                aggrBasicStats.getOrDefault(pi.getPartition().getName(), 
Map.of())))
+            .toList();
+      }

Review Comment:
   This omits partitions missing from the stats file, but the caller treats any 
non-empty map as a full success. Can it lead to wrong partition estimates?



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

Reply via email to