sankarh commented on a change in pull request #579: HIVE-21109 : Support stats 
replication for ACID tables.
URL: https://github.com/apache/hive/pull/579#discussion_r270652633
 
 

 ##########
 File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/ColumnStatsUpdateTask.java
 ##########
 @@ -297,21 +303,34 @@ private ColumnStatisticsDesc getColumnStatsDesc(String 
dbName,
 
   private int persistColumnStats(Hive db) throws HiveException, MetaException, 
IOException {
     ColumnStatistics colStats = constructColumnStatsFromInput();
-    ColumnStatisticsDesc colStatsDesc = colStats.getStatsDesc();
-    // We do not support stats replication for a transactional table yet. If 
we are converting
-    // a non-transactional table to a transactional table during replication, 
we might get
-    // column statistics but we shouldn't update those.
-    if (work.getColStats() != null &&
-        
AcidUtils.isTransactionalTable(getHive().getTable(colStatsDesc.getDbName(),
-                                                          
colStatsDesc.getTableName()))) {
-      LOG.debug("Skipped updating column stats for table " +
-                TableName.getDbTable(colStatsDesc.getDbName(), 
colStatsDesc.getTableName()) +
-                " because it is converted to a transactional table during 
replication.");
-      return 0;
-    }
-
     SetPartitionsStatsRequest request =
             new SetPartitionsStatsRequest(Collections.singletonList(colStats));
+
+    // Set writeId and validWriteId list for replicated statistics.
+    if (work.getColStats() != null) {
+      String dbName = colStats.getStatsDesc().getDbName();
+      String tblName = colStats.getStatsDesc().getTableName();
+      Table tbl = db.getTable(dbName, tblName);
+      long writeId = work.getWriteId();
+      // If it's a transactional table on source and target, we will get a 
valid writeId
+      // associated with it. Otherwise it's a non-transactional table on 
source migrated to a
+      // transactional table on target, we need to craft a valid writeId here.
+      if (AcidUtils.isTransactionalTable(tbl)) {
+        ValidWriteIdList writeIds;
+        if (writeId <= 0) {
 
 Review comment:
   Instead of having this assumption of "writeId <= 0 means migration case", it 
is better if the caller sets the correct writeId in ColumnStatsUpdateWork 
itself. 
   If it is non-migration case and there is a bug in the caller and passes 
wrong writeId, then we throw incorrect exception message.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to