diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java index e4da3cb4e3..bc4159cd8a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java @@ -24,7 +24,6 @@ import org.apache.doris.analysis.AlterTableStmt; import org.apache.doris.analysis.AlterViewStmt; import org.apache.doris.analysis.ColumnRenameClause; import org.apache.doris.analysis.CreateMaterializedViewStmt; -import org.apache.doris.analysis.DateLiteral; import org.apache.doris.analysis.DropMaterializedViewStmt; import org.apache.doris.analysis.DropPartitionClause; import org.apache.doris.analysis.ModifyColumnCommentClause; @@ -52,7 +51,6 @@ import org.apache.doris.catalog.PartitionInfo; import org.apache.doris.catalog.ReplicaAllocation; import org.apache.doris.catalog.Table; import org.apache.doris.catalog.TableIf.TableType; -import org.apache.doris.catalog.Type; import org.apache.doris.catalog.View; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.DdlException; @@ -61,7 +59,6 @@ import org.apache.doris.common.UserException; import org.apache.doris.common.util.DynamicPartitionUtil; import org.apache.doris.common.util.MetaLockUtils; import org.apache.doris.common.util.PropertyAnalyzer; -import org.apache.doris.common.util.TimeUtils; import org.apache.doris.persist.AlterViewInfo; import org.apache.doris.persist.BatchModifyPartitionsInfo; import org.apache.doris.persist.ModifyCommentOperationLog; @@ -81,7 +78,6 @@ import org.apache.logging.log4j.Logger; import java.util.Arrays; import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Map; @@ -684,17 +680,10 @@ public class Alter { // 4. data property // 4.1 get old data property from partition DataProperty dataProperty = partitionInfo.getDataProperty(partition.getId()); - // 4.2 combine the old properties with new ones - Map newProperties = new HashMap<>(); - newProperties.put(PropertyAnalyzer.PROPERTIES_STORAGE_MEDIUM, dataProperty.getStorageMedium().name()); - DateLiteral dateLiteral = new DateLiteral(dataProperty.getCooldownTimeMs(), - TimeUtils.getTimeZone(), Type.DATETIME); - newProperties.put(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TIME, dateLiteral.getStringValue()); - newProperties.put(PropertyAnalyzer.PROPERTIES_REMOTE_STORAGE_POLICY, dataProperty.getRemoteStoragePolicy()); - newProperties.putAll(properties); - // 4.3 analyze new properties - DataProperty newDataProperty = - PropertyAnalyzer.analyzeDataProperty(newProperties, DataProperty.DEFAULT_DATA_PROPERTY); + Map modifiedProperties = Maps.newHashMap(); + modifiedProperties.putAll(properties); + // 4.2 analyze new properties + DataProperty newDataProperty = PropertyAnalyzer.analyzeDataProperty(modifiedProperties, dataProperty); // 1. date property if (newDataProperty != null) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java index 84b29b9803..144576af6d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java @@ -44,6 +44,7 @@ import org.apache.doris.thrift.TTabletType; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.Sets; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -111,24 +112,28 @@ public class PropertyAnalyzer { private static final double MAX_FPP = 0.05; private static final double MIN_FPP = 0.0001; - public static DataProperty analyzeDataProperty(Map properties, DataProperty oldDataProperty) + /** + * check and replace members of DataProperty by properties. + * + * @param properties key->value for members to change. + * @param oldDataProperty old DataProperty + * @return new DataProperty + * @throws AnalysisException property has invalid key->value + */ + public static DataProperty analyzeDataProperty(Map properties, final DataProperty oldDataProperty) throws AnalysisException { if (properties == null || properties.isEmpty()) { return oldDataProperty; } - TStorageMedium storageMedium = null; - long cooldownTimeStamp = DataProperty.MAX_COOLDOWN_TIME_MS; - String remoteStoragePolicy = ""; + TStorageMedium storageMedium = oldDataProperty.getStorageMedium(); + long cooldownTimeStamp = oldDataProperty.getCooldownTimeMs(); + String remoteStoragePolicy = oldDataProperty.getRemoteStoragePolicy(); - boolean hasMedium = false; - boolean hasCooldown = false; - boolean hasRemoteStoragePolicy = false; for (Map.Entry entry : properties.entrySet()) { String key = entry.getKey(); String value = entry.getValue(); - if (!hasMedium && key.equalsIgnoreCase(PROPERTIES_STORAGE_MEDIUM)) { - hasMedium = true; + if (key.equalsIgnoreCase(PROPERTIES_STORAGE_MEDIUM)) { if (value.equalsIgnoreCase(TStorageMedium.SSD.name())) { storageMedium = TStorageMedium.SSD; } else if (value.equalsIgnoreCase(TStorageMedium.HDD.name())) { @@ -136,40 +141,26 @@ public class PropertyAnalyzer { } else { throw new AnalysisException("Invalid storage medium: " + value); } - } else if (!hasCooldown && key.equalsIgnoreCase(PROPERTIES_STORAGE_COOLDOWN_TIME)) { + } else if (key.equalsIgnoreCase(PROPERTIES_STORAGE_COOLDOWN_TIME)) { DateLiteral dateLiteral = new DateLiteral(value, Type.DATETIME); cooldownTimeStamp = dateLiteral.unixTimestamp(TimeUtils.getTimeZone()); - if (cooldownTimeStamp != DataProperty.MAX_COOLDOWN_TIME_MS) { - hasCooldown = true; - } - } else if (!hasRemoteStoragePolicy && key.equalsIgnoreCase(PROPERTIES_REMOTE_STORAGE_POLICY)) { - if (!Strings.isNullOrEmpty(value)) { - hasRemoteStoragePolicy = true; - remoteStoragePolicy = value; - } + } else if (key.equalsIgnoreCase(PROPERTIES_REMOTE_STORAGE_POLICY)) { + remoteStoragePolicy = value; } } // end for properties - // Check properties - - if (!hasCooldown && !hasMedium && !hasRemoteStoragePolicy) { - return oldDataProperty; - } - properties.remove(PROPERTIES_STORAGE_MEDIUM); properties.remove(PROPERTIES_STORAGE_COOLDOWN_TIME); properties.remove(PROPERTIES_REMOTE_STORAGE_POLICY); - if (hasCooldown && !hasMedium) { - throw new AnalysisException("Invalid data property. storage medium property is not found"); - } - - if (storageMedium == TStorageMedium.HDD && hasCooldown) { + if (storageMedium == TStorageMedium.HDD) { cooldownTimeStamp = DataProperty.MAX_COOLDOWN_TIME_MS; LOG.info("Can not assign cool down timestamp to HDD storage medium, ignore user setting."); - hasCooldown = false; } + boolean hasCooldown = cooldownTimeStamp != DataProperty.MAX_COOLDOWN_TIME_MS; + boolean hasRemoteStoragePolicy = StringUtils.isNotEmpty(remoteStoragePolicy); + long currentTimeMs = System.currentTimeMillis(); if (storageMedium == TStorageMedium.SSD && hasCooldown) { if (cooldownTimeStamp <= currentTimeMs) { @@ -183,7 +174,7 @@ public class PropertyAnalyzer { } if (hasRemoteStoragePolicy) { - // check remote resource + // check remote storage policy StoragePolicy checkedPolicy = new StoragePolicy(PolicyTypeEnum.STORAGE, remoteStoragePolicy); Policy policy = Catalog.getCurrentCatalog().getPolicyMgr().getPolicy(checkedPolicy); if (!(policy instanceof StoragePolicy)) { @@ -195,7 +186,7 @@ public class PropertyAnalyzer { if (storagePolicy.getCooldownDatetime().getTime() <= currentTimeMs) { throw new AnalysisException("Remote storage cool down time should later than now"); } - if (hasCooldown && (storagePolicy.getCooldownDatetime().getTime() <= cooldownTimeStamp)) { + if (hasCooldown && storagePolicy.getCooldownDatetime().getTime() <= cooldownTimeStamp) { throw new AnalysisException("`remote_storage_cooldown_time`" + " should later than `storage_cooldown_time`."); }