This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push:
new 26750144912 [fix](s3) fix invalid s3 properties checking logic (#35757)
26750144912 is described below
commit 2675014491263696f6a3f717d13f444bdca6982e
Author: Mingyu Chen <[email protected]>
AuthorDate: Sat Jun 1 23:30:22 2024 +0800
[fix](s3) fix invalid s3 properties checking logic (#35757)
Introduced from #35747
pick part of #35762
---
.../main/java/org/apache/doris/catalog/S3Resource.java | 15 +++------------
.../java/org/apache/doris/fs/remote/S3FileSystem.java | 5 ++++-
.../nereids/trees/expressions/functions/table/Hdfs.java | 3 +--
.../nereids/trees/expressions/functions/table/Local.java | 3 +--
.../nereids/trees/expressions/functions/table/S3.java | 3 +--
.../apache/doris/tablefunction/S3TableValuedFunction.java | 7 +++++--
6 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java
index a2603897047..8b9b5f6af37 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java
@@ -23,7 +23,6 @@ import org.apache.doris.common.FeConstants;
import org.apache.doris.common.proc.BaseProcResult;
import org.apache.doris.common.util.PrintableMap;
import org.apache.doris.datasource.credentials.CloudCredentialWithEndpoint;
-import org.apache.doris.datasource.property.PropertyConverter;
import org.apache.doris.datasource.property.constants.S3Properties;
import org.apache.doris.fs.remote.S3FileSystem;
@@ -121,15 +120,6 @@ public class S3Resource extends Resource {
private static void pingS3(CloudCredentialWithEndpoint credential, String
bucketName, String rootPath,
Map<String, String> properties) throws DdlException {
String bucket = "s3://" + bucketName + "/";
- Map<String, String> propertiesPing = new HashMap<>();
- propertiesPing.put(S3Properties.Env.ACCESS_KEY,
credential.getAccessKey());
- propertiesPing.put(S3Properties.Env.SECRET_KEY,
credential.getSecretKey());
- propertiesPing.put(S3Properties.Env.TOKEN,
credential.getSessionToken());
- propertiesPing.put(S3Properties.Env.ENDPOINT,
credential.getEndpoint());
- propertiesPing.put(S3Properties.Env.REGION, credential.getRegion());
- propertiesPing.put(PropertyConverter.USE_PATH_STYLE,
- properties.getOrDefault(PropertyConverter.USE_PATH_STYLE,
"false"));
- properties.putAll(propertiesPing);
S3FileSystem fileSystem = new S3FileSystem(properties);
String testFile = bucket + rootPath + "/test-object-valid.txt";
String content = "doris will be better";
@@ -142,14 +132,14 @@ public class S3Resource extends Resource {
if (status != Status.OK) {
throw new DdlException(
"ping s3 failed(upload), status: " + status + ",
properties: " + new PrintableMap<>(
- propertiesPing, "=", true, false, true,
false));
+ properties, "=", true, false, true, false));
}
} finally {
if (status.ok()) {
Status delete = fileSystem.delete(testFile);
if (delete != Status.OK) {
LOG.warn("delete test file failed, status: {}, properties:
{}", delete, new PrintableMap<>(
- propertiesPing, "=", true, false, true, false));
+ properties, "=", true, false, true, false));
}
}
}
@@ -250,3 +240,4 @@ public class S3Resource extends Resource {
readUnlock();
}
}
+
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/fs/remote/S3FileSystem.java
b/fe/fe-core/src/main/java/org/apache/doris/fs/remote/S3FileSystem.java
index 3869824de55..2b94d2195da 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/fs/remote/S3FileSystem.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/fs/remote/S3FileSystem.java
@@ -60,7 +60,10 @@ public class S3FileSystem extends ObjFileSystem {
if (dfsFileSystem == null) {
Configuration conf = new Configuration();
System.setProperty("com.amazonaws.services.s3.enableV4", "true");
-
PropertyConverter.convertToHadoopFSProperties(properties).forEach(conf::set);
+ // the entry value in properties may be null, and
+
PropertyConverter.convertToHadoopFSProperties(properties).entrySet().stream()
+ .filter(entry -> entry.getKey() != null &&
entry.getValue() != null)
+ .forEach(entry -> conf.set(entry.getKey(),
entry.getValue()));
try {
dfsFileSystem = FileSystem.get(new Path(remotePath).toUri(),
conf);
} catch (Exception e) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/Hdfs.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/Hdfs.java
index 6c32de50eea..5f8651ed61c 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/Hdfs.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/Hdfs.java
@@ -45,8 +45,7 @@ public class Hdfs extends TableValuedFunction {
Map<String, String> arguments = getTVFProperties().getMap();
return new HdfsTableValuedFunction(arguments);
} catch (Throwable t) {
- throw new AnalysisException("Can not build HdfsTableValuedFunction
by "
- + this + ": " + t.getMessage(), t);
+ throw new AnalysisException("Can not build hdfs(): " +
t.getMessage(), t);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/Local.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/Local.java
index d45a4c93943..4330980ee86 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/Local.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/Local.java
@@ -46,8 +46,7 @@ public class Local extends TableValuedFunction {
Map<String, String> arguments = getTVFProperties().getMap();
return new LocalTableValuedFunction(arguments);
} catch (Throwable t) {
- throw new AnalysisException("Can not build
LocalTableValuedFunction by "
- + this + ": " + t.getMessage(), t);
+ throw new AnalysisException("Can not build local(): " +
t.getMessage(), t);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/S3.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/S3.java
index 1f3d7cb805e..d2623d8fd3c 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/S3.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/table/S3.java
@@ -44,8 +44,7 @@ public class S3 extends TableValuedFunction {
Map<String, String> arguments = getTVFProperties().getMap();
return new S3TableValuedFunction(arguments);
} catch (Throwable t) {
- throw new AnalysisException("Can not build S3TableValuedFunction
by "
- + this + ": " + t.getMessage(), t);
+ throw new AnalysisException("Can not build s3(): " +
t.getMessage(), t);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/tablefunction/S3TableValuedFunction.java
b/fe/fe-core/src/main/java/org/apache/doris/tablefunction/S3TableValuedFunction.java
index 98b35de7d3e..8476f1c978b 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/tablefunction/S3TableValuedFunction.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/tablefunction/S3TableValuedFunction.java
@@ -73,8 +73,10 @@ public class S3TableValuedFunction extends
ExternalFileTableValuedFunction {
S3URI s3uri = getS3Uri(uriStr,
Boolean.parseBoolean(usePathStyle.toLowerCase()),
Boolean.parseBoolean(forceParsingByStandardUri.toLowerCase()));
- String endpoint = getOrDefaultAndRemove(otherProps,
S3Properties.ENDPOINT, s3uri.getEndpoint().orElseThrow(() ->
- new AnalysisException(String.format("Properties '%s' is
required.", S3Properties.ENDPOINT))));
+ String endpoint = getOrDefaultAndRemove(otherProps,
S3Properties.ENDPOINT, s3uri.getEndpoint().orElse(""));
+ if (Strings.isNullOrEmpty(endpoint)) {
+ throw new AnalysisException(String.format("Properties '%s' is
required.", S3Properties.ENDPOINT));
+ }
if (!otherProps.containsKey(S3Properties.REGION)) {
String region = s3uri.getRegion().orElseThrow(() ->
new AnalysisException(String.format("Properties '%s' is
required.", S3Properties.REGION)));
@@ -151,3 +153,4 @@ public class S3TableValuedFunction extends
ExternalFileTableValuedFunction {
return "S3TableValuedFunction";
}
}
+
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
