From 03b9b5f3226fc2606a9fe272d7ab0850164862bf Mon Sep 17 00:00:00 2001 From: yagagagaga Date: Tue, 6 May 2025 15:13:10 +0800 Subject: [PATCH 1/5] [fix](vault) fix UnsupportedOperationException if no `use_path_style` in CreateStorageVaultStmt --- .../apache/doris/analysis/CreateStorageVaultStmt.java | 11 ++++++++++- .../java/org/apache/doris/catalog/StorageVault.java | 4 ---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java index 4ce1e3cd1d7c08..43e2b76d0e6150 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java @@ -28,6 +28,7 @@ import org.apache.doris.common.FeNameFormat; import org.apache.doris.common.UserException; import org.apache.doris.common.util.PrintableMap; +import org.apache.doris.datasource.property.PropertyConverter; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.qe.ConnectContext; @@ -55,7 +56,15 @@ public class CreateStorageVaultStmt extends DdlStmt implements NotFallbackInPars public CreateStorageVaultStmt(boolean ifNotExists, String vaultName, Map properties) { this.ifNotExists = ifNotExists; this.vaultName = vaultName; - this.properties = ImmutableMap.copyOf(properties); + + if (!properties.containsKey(PropertyConverter.USE_PATH_STYLE)) { + this.properties = ImmutableMap.builder() + .putAll(properties) + .put(PropertyConverter.USE_PATH_STYLE, "true") + .build(); + } else { + this.properties = ImmutableMap.copyOf(properties); + } this.vaultType = vaultType.UNKNOWN; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java index 0584d10dc327d2..0d7641aa047022 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java @@ -147,10 +147,6 @@ public void setId(String id) { vault.modifyProperties(stmt.getProperties()); break; case S3: - if (!stmt.getProperties().containsKey(PropertyConverter.USE_PATH_STYLE)) { - stmt.getProperties().put(PropertyConverter.USE_PATH_STYLE, "true"); - } - CreateResourceStmt resourceStmt = new CreateResourceStmt(false, ifNotExists, name, stmt.getProperties()); resourceStmt.analyzeResourceType(); From 0bb4b38eb23e0207418e7abb00fb52e37f875168 Mon Sep 17 00:00:00 2001 From: yagagagaga Date: Tue, 6 May 2025 15:19:13 +0800 Subject: [PATCH 2/5] update --- .../src/main/java/org/apache/doris/catalog/StorageVault.java | 1 - 1 file changed, 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java index 0d7641aa047022..f71560c012ee22 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java @@ -22,7 +22,6 @@ import org.apache.doris.cloud.proto.Cloud; import org.apache.doris.common.DdlException; import org.apache.doris.common.UserException; -import org.apache.doris.datasource.property.PropertyConverter; import org.apache.doris.qe.ShowResultSetMetaData; import com.google.common.base.Preconditions; From c24c5ccaca799f5311181c2c65c2fd90261e8afc Mon Sep 17 00:00:00 2001 From: yagagagaga Date: Tue, 6 May 2025 15:30:02 +0800 Subject: [PATCH 3/5] update --- .../java/org/apache/doris/analysis/CreateStorageVaultStmt.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java index 43e2b76d0e6150..8b52e999a4ff2e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java @@ -57,7 +57,8 @@ public CreateStorageVaultStmt(boolean ifNotExists, String vaultName, Mapbuilder() .putAll(properties) .put(PropertyConverter.USE_PATH_STYLE, "true") From e5b55760d517abea03c546ffc1d3a0619c9d7b9b Mon Sep 17 00:00:00 2001 From: yagagagaga Date: Tue, 6 May 2025 15:37:19 +0800 Subject: [PATCH 4/5] update --- .../analysis/CreateStorageVaultStmt.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java index 8b52e999a4ff2e..3a4e8ff9cb59b3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java @@ -47,7 +47,7 @@ public class CreateStorageVaultStmt extends DdlStmt implements NotFallbackInPars private final boolean ifNotExists; private final String vaultName; - private final ImmutableMap properties; + private ImmutableMap properties; private boolean setAsDefault; private int pathVersion = 0; private int numShard = 0; @@ -56,16 +56,7 @@ public class CreateStorageVaultStmt extends DdlStmt implements NotFallbackInPars public CreateStorageVaultStmt(boolean ifNotExists, String vaultName, Map properties) { this.ifNotExists = ifNotExists; this.vaultName = vaultName; - - if ("s3".equalsIgnoreCase(properties.get(StorageVault.PropertyKey.TYPE)) - && !properties.containsKey(PropertyConverter.USE_PATH_STYLE)) { - this.properties = ImmutableMap.builder() - .putAll(properties) - .put(PropertyConverter.USE_PATH_STYLE, "true") - .build(); - } else { - this.properties = ImmutableMap.copyOf(properties); - } + this.properties = ImmutableMap.copyOf(properties); this.vaultType = vaultType.UNKNOWN; } @@ -155,6 +146,14 @@ public void analyze(Analyzer analyzer) throws UserException { } setAsDefault = Boolean.parseBoolean(properties.getOrDefault(SET_AS_DEFAULT, "false")); setStorageVaultType(StorageVault.StorageVaultType.fromString(type)); + + if (vaultType == StorageVault.StorageVaultType.S3 + && !properties.containsKey(PropertyConverter.USE_PATH_STYLE)) { + properties = ImmutableMap.builder() + .putAll(properties) + .put(PropertyConverter.USE_PATH_STYLE, "true") + .build(); + } } @Override From 47bf13d632de95aa94ae3d128d68e67b0f6cdbf9 Mon Sep 17 00:00:00 2001 From: yagagagaga Date: Tue, 6 May 2025 15:40:46 +0800 Subject: [PATCH 5/5] add case --- .../vault_p0/create/test_create_vault.groovy | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/regression-test/suites/vault_p0/create/test_create_vault.groovy b/regression-test/suites/vault_p0/create/test_create_vault.groovy index 502782a6cc1a36..c0128291f82f33 100644 --- a/regression-test/suites/vault_p0/create/test_create_vault.groovy +++ b/regression-test/suites/vault_p0/create/test_create_vault.groovy @@ -233,6 +233,21 @@ suite("test_create_vault", "nonConcurrent") { ); """ + sql """ + CREATE STORAGE VAULT IF NOT EXISTS ${s3VaultName} + PROPERTIES ( + "type"="S3", + "s3.endpoint"="${getS3Endpoint()}", + "s3.region" = "${getS3Region()}", + "s3.access_key" = "${getS3AK()}", + "s3.secret_key" = "${getS3SK()}", + "s3.root.path" = "${s3VaultName}", + "s3.bucket" = "${getS3BucketName()}", + "s3.external_endpoint" = "", + "provider" = "${getS3Provider()}" + ); + """ + sql """ CREATE TABLE ${s3VaultName} ( C_CUSTKEY INTEGER NOT NULL,