kgyrtkirk commented on a change in pull request #734: Hive 22028 Clean up Add
Partition
URL: https://github.com/apache/hive/pull/734#discussion_r306287318
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/AlterTableAddPartitionOperation.java
##########
@@ -37,10 +52,181 @@ public
AlterTableAddPartitionOperation(DDLOperationContext context, AlterTableAd
@Override
public int execute() throws HiveException {
- List<Partition> parts = context.getDb().createPartitions(desc);
- for (Partition part : parts) {
- DDLUtils.addIfAbsentByName(new WriteEntity(part,
WriteEntity.WriteType.INSERT), context);
- }
+ // TODO: catalog name everywhere in this method
+ Table table = context.getDb().getTable(desc.getDbName(),
desc.getTableName());
+ long writeId = getWriteId(table);
+
+ List<Partition> partitions = getPartitions(table, writeId);
+ addPartitions(table, partitions, writeId);
return 0;
}
+
+ private long getWriteId(Table table) throws LockException {
+ // In case of replication, get the writeId from the source and use valid
write Id list for replication.
+ if (desc.getReplicationSpec().isInReplicationScope() &&
desc.getPartitions().get(0).getWriteId() > 0) {
+ return desc.getPartitions().get(0).getWriteId();
+ } else {
+ AcidUtils.TableSnapshot tableSnapshot =
AcidUtils.getTableSnapshot(context.getConf(), table, true);
+ if (tableSnapshot != null && tableSnapshot.getWriteId() > 0) {
+ return tableSnapshot.getWriteId();
+ } else {
+ return -1;
+ }
+ }
+ }
+
+ private List<Partition> getPartitions(Table table, long writeId) throws
HiveException {
+ List<Partition> partitions = new ArrayList<>(desc.getPartitions().size());
+ for (AlterTableAddPartitionDesc.PartitionDesc partitionDesc :
desc.getPartitions()) {
+ Partition partition = convertPartitionSpecToMetaPartition(table,
partitionDesc);
+ if (partition != null && writeId > 0) {
+ partition.setWriteId(writeId);
+ }
+ partitions.add(partition);
+ }
+
+ return partitions;
+ }
+
+ private Partition convertPartitionSpecToMetaPartition(Table table,
+ AlterTableAddPartitionDesc.PartitionDesc partitionSpec) throws
HiveException {
+ Path location = partitionSpec.getLocation() != null ? new
Path(table.getPath(), partitionSpec.getLocation()) : null;
+ if (location != null) {
+ // Ensure that it is a full qualified path (in most cases it will be
since tbl.getPath() is full qualified)
+ location = new Path(Utilities.getQualifiedPath(context.getConf(),
location));
+ }
+
+ Partition partition =
org.apache.hadoop.hive.ql.metadata.Partition.createMetaPartitionObject(
+ table, partitionSpec.getPartSpec(), location);
+
+ if (partitionSpec.getPartParams() != null) {
+ partition.setParameters(partitionSpec.getPartParams());
+ }
+ if (partitionSpec.getInputFormat() != null) {
+ partition.getSd().setInputFormat(partitionSpec.getInputFormat());
+ }
+ if (partitionSpec.getOutputFormat() != null) {
+ partition.getSd().setOutputFormat(partitionSpec.getOutputFormat());
+ }
+ if (partitionSpec.getNumBuckets() != -1) {
+ partition.getSd().setNumBuckets(partitionSpec.getNumBuckets());
+ }
+ if (partitionSpec.getCols() != null) {
+ partition.getSd().setCols(partitionSpec.getCols());
+ }
+ if (partitionSpec.getSerializationLib() != null) {
+
partition.getSd().getSerdeInfo().setSerializationLib(partitionSpec.getSerializationLib());
+ }
+ if (partitionSpec.getSerdeParams() != null) {
+
partition.getSd().getSerdeInfo().setParameters(partitionSpec.getSerdeParams());
+ }
+ if (partitionSpec.getBucketCols() != null) {
+ partition.getSd().setBucketCols(partitionSpec.getBucketCols());
+ }
+ if (partitionSpec.getSortCols() != null) {
+ partition.getSd().setSortCols(partitionSpec.getSortCols());
+ }
+ if (partitionSpec.getColStats() != null) {
+ partition.setColStats(partitionSpec.getColStats());
+ // Statistics will have an associated write Id for a transactional
table. We need it to update column statistics.
+ partition.setWriteId(partitionSpec.getWriteId());
+ }
+ return partition;
+ }
+
+ private void addPartitions(Table table, List<Partition> partitions, long
writeId) throws HiveException {
+ List<org.apache.hadoop.hive.ql.metadata.Partition> outPartitions = null;
+ if (!desc.getReplicationSpec().isInReplicationScope()) {
Review comment:
I think it might worth considering to instead of branching based on a
replication flag; the replication stuff could be some kind of listener for the
changes - it seems to me that the "replication counterpart" only kills the
stats for every existing partition
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
