From 1b3aab1daf35f3bbc52865dd235f50b2d1621756 Mon Sep 17 00:00:00 2001 From: Kaijie Chen Date: Tue, 24 Dec 2024 18:25:03 +0800 Subject: [PATCH 1/4] [fix](tvf) fix azure tvf: can not build s3() --- .../org/apache/doris/tablefunction/S3TableValuedFunction.java | 2 ++ 1 file changed, 2 insertions(+) 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 7a9566c13e0882..f62beb8e2f177c 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 @@ -59,6 +59,8 @@ public S3TableValuedFunction(Map properties) throws AnalysisExce final boolean isAzureTvf = AzureProperties.checkAzureProviderPropertyExist(properties); // Azure could run without region if (isAzureTvf) { + // We have to copy properties because the map is immutable + properties = Maps.newHashMap(properties); properties.put(S3Properties.REGION, "DUMMY-REGION"); } // 1. analyze common properties From abc56fdf7eddb82ecd48c030a871e76fcd6c3f38 Mon Sep 17 00:00:00 2001 From: Kaijie Chen Date: Tue, 24 Dec 2024 19:36:20 +0800 Subject: [PATCH 2/4] fixup --- .../doris/tablefunction/S3TableValuedFunction.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 f62beb8e2f177c..9647b1cd8bbe5b 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 @@ -59,9 +59,13 @@ public S3TableValuedFunction(Map properties) throws AnalysisExce final boolean isAzureTvf = AzureProperties.checkAzureProviderPropertyExist(properties); // Azure could run without region if (isAzureTvf) { - // We have to copy properties because the map is immutable - properties = Maps.newHashMap(properties); - properties.put(S3Properties.REGION, "DUMMY-REGION"); + try { + properties.put(S3Properties.REGION, "DUMMY-REGION"); + } catch (UnsupportedOperationException e) { + // copy properties if the map is immutable + properties = Maps.newHashMap(properties); + properties.put(S3Properties.REGION, "DUMMY-REGION"); + } } // 1. analyze common properties Map otherProps = super.parseCommonProperties(properties); From e1e228a39dff473459a442463532ba1d87b470b7 Mon Sep 17 00:00:00 2001 From: Kaijie Chen Date: Wed, 25 Dec 2024 10:45:18 +0800 Subject: [PATCH 3/4] fixup --- .../tablefunction/S3TableValuedFunction.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) 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 9647b1cd8bbe5b..2e563ba1196481 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 @@ -32,6 +32,7 @@ import org.apache.doris.thrift.TFileType; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import java.util.Map; @@ -58,14 +59,12 @@ public class S3TableValuedFunction extends ExternalFileTableValuedFunction { public S3TableValuedFunction(Map properties) throws AnalysisException { final boolean isAzureTvf = AzureProperties.checkAzureProviderPropertyExist(properties); // Azure could run without region - if (isAzureTvf) { - try { - properties.put(S3Properties.REGION, "DUMMY-REGION"); - } catch (UnsupportedOperationException e) { - // copy properties if the map is immutable - properties = Maps.newHashMap(properties); - properties.put(S3Properties.REGION, "DUMMY-REGION"); - } + if (isAzureTvf && !properties.containsKey(S3Properties.REGION)) { + // copy properties because it's immutable + properties = new ImmutableMap.Builder() + .putAll(properties) + .put(S3Properties.REGION, "DUMMY-REGION") + .build(); } // 1. analyze common properties Map otherProps = super.parseCommonProperties(properties); From 326379c68edb6401d7d37f47381d6c1cb2f3d858 Mon Sep 17 00:00:00 2001 From: Kaijie Chen Date: Wed, 25 Dec 2024 12:52:19 +0800 Subject: [PATCH 4/4] improve --- .../tablefunction/S3TableValuedFunction.java | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) 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 2e563ba1196481..3defb171a9f0cf 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 @@ -32,7 +32,6 @@ import org.apache.doris.thrift.TFileType; import com.google.common.base.Strings; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import java.util.Map; @@ -57,15 +56,6 @@ public class S3TableValuedFunction extends ExternalFileTableValuedFunction { "ACCESS_KEY", "SECRET_KEY", "SESSION_TOKEN", "REGION"); public S3TableValuedFunction(Map properties) throws AnalysisException { - final boolean isAzureTvf = AzureProperties.checkAzureProviderPropertyExist(properties); - // Azure could run without region - if (isAzureTvf && !properties.containsKey(S3Properties.REGION)) { - // copy properties because it's immutable - properties = new ImmutableMap.Builder() - .putAll(properties) - .put(S3Properties.REGION, "DUMMY-REGION") - .build(); - } // 1. analyze common properties Map otherProps = super.parseCommonProperties(properties); @@ -89,8 +79,14 @@ public S3TableValuedFunction(Map properties) throws AnalysisExce // If endpoint is missing, exception will be thrown. String endpoint = constructEndpoint(otherProps, s3uri); if (!otherProps.containsKey(S3Properties.REGION)) { - String region = s3uri.getRegion().orElseThrow(() -> - new AnalysisException(String.format("Properties '%s' is required.", S3Properties.REGION))); + String region; + if (AzureProperties.checkAzureProviderPropertyExist(properties)) { + // Azure could run without region + region = s3uri.getRegion().orElse("DUMMY-REGION"); + } else { + region = s3uri.getRegion().orElseThrow(() -> new AnalysisException( + String.format("Properties '%s' is required.", S3Properties.REGION))); + } otherProps.put(S3Properties.REGION, region); } checkNecessaryS3Properties(otherProps); @@ -104,7 +100,7 @@ public S3TableValuedFunction(Map properties) throws AnalysisExce locationProperties = S3Properties.credentialToMap(credential); locationProperties.put(PropertyConverter.USE_PATH_STYLE, usePathStyle); - if (isAzureTvf) { + if (AzureProperties.checkAzureProviderPropertyExist(properties)) { // For Azure's compatibility, we need bucket to connect to the blob storage's container locationProperties.put(S3Properties.BUCKET, s3uri.getBucket()); }