HIVE-28668: Hive should emit fewer events for truncate table operation #5582
Conversation
|
| } | ||
| alterHandler.alterPartition(ms, wh, catName, dbName, tableName, null, partition, | ||
| environmentContext, this, validWriteIds); | ||
| alterHandler.alterPartitions(ms, wh, catName, dbName, tableName, partitions, environmentContext, |
There was a problem hiding this comment.
since we know this is a truncation op, can we just call ms.alterPartitions(catName, dbname, name, partValsList, new_parts, writeId, writeIdList) and create the event directly? same for truncating the non-partitioned table.
| @@ -101,7 +101,7 @@ public void setConf(Configuration conf) { | |||
| @Override | |||
| public void alterTable(RawStore msdb, Warehouse wh, String catName, String dbname, | |||
| String name, Table newt, EnvironmentContext environmentContext, | |||
| IHMSHandler handler, String writeIdList) | |||
| IHMSHandler handler, String writeIdList, boolean isTruncateOp) | |||
There was a problem hiding this comment.
instead of adding a new isTruncateOp, can we put it into the environmentContext, like environmentContext.putToProperties("__isTruncateOp__", "true");
| context.dmd.getPayload()); | ||
| tName = TableName.fromString(singleMsg.getTable(), null, | ||
| context.isDbNameEmpty() ? singleMsg.getDB() : context.dbName); | ||
| tblObj = singleMsg.getTableObj(); |
There was a problem hiding this comment.
nit: can we create the tName from the tblObj?
| Iterator<String> afterIterator = afterIteratorMap.values().iterator(); | ||
| Map<String, String> partSpec = new LinkedHashMap<>(); | ||
| for (FieldSchema fs : tblObj.getPartitionKeys()) { | ||
| partSpec.put(fs.getName(), afterIterator.next()); |
There was a problem hiding this comment.
nit: afterIterator.next -> afterIteratorMap.get(fs.getName())
What changes were proposed in this pull request?
Currently, we emit twice the number of events for each table/partition for truncate operation. This patch removes this redundant events.
Why are the changes needed?
Having fewer events help external systems that consume these events to catch up with the quantum of events faster.
Does this PR introduce any user-facing change?
No.
Is the change a dependency upgrade?
No.
How was this patch tested?
Added unit test for the patch