Skip to content

Commit

Permalink
change data-file.external-paths.specific-fs to string
Browse files Browse the repository at this point in the history
  • Loading branch information
neuyilan committed Jan 3, 2025
1 parent bcb2be1 commit f3303c1
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 66 deletions.
4 changes: 2 additions & 2 deletions docs/layouts/shortcodes/generated/core_configuration.html
Original file line number Diff line number Diff line change
Expand Up @@ -1029,8 +1029,8 @@
<tr>
<td><h5>data-file.external-paths.specific-fs</h5></td>
<td style="word-wrap: break-word;">(none)</td>
<td><p>Enum</p></td>
<td>The specific file system of the external path when data-file.external-paths.strategy is set to specific-fs<br /><br />Possible values:<ul><li>"S3": Select S3 as the write path for the external path.</li><li>"OSS": Select OSS as the write path for the external path.</li></ul></td>
<td>String</td>
<td>The specific file system of the external path when data-file.external-paths.strategy is set to specific-fs, should be the prefix scheme of the external path, now supported are s3 and oss.</td>
</tr>
</tbody>
</table>
35 changes: 5 additions & 30 deletions paimon-common/src/main/java/org/apache/paimon/CoreOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,16 @@ public class CoreOptions implements Serializable {
.withDescription(
"The strategy of selecting an external path when writing data.");

public static final ConfigOption<ExternalFSStrategy> DATA_FILE_EXTERNAL_PATHS_SPECIFIC_FS =
public static final ConfigOption<String> DATA_FILE_EXTERNAL_PATHS_SPECIFIC_FS =
key("data-file.external-paths.specific-fs")
.enumType(ExternalFSStrategy.class)
.stringType()
.noDefaultValue()
.withDescription(
"The specific file system of the external path when "
+ DATA_FILE_EXTERNAL_PATHS_STRATEGY.key()
+ " is set to "
+ ExternalPathStrategy.SPECIFIC_FS);
+ ExternalPathStrategy.SPECIFIC_FS
+ ", should be the prefix scheme of the external path, now supported are s3 and oss.");

// todo, this path is the table schema path, the name will be changed in the later PR.
@ExcludeFromDocumentation("Internal use only")
Expand Down Expand Up @@ -2215,7 +2216,7 @@ public ExternalPathStrategy externalPathStrategy() {
return options.get(DATA_FILE_EXTERNAL_PATHS_STRATEGY);
}

public ExternalFSStrategy externalSpecificFSStrategy() {
public String externalSpecificFSStrategy() {
return options.get(DATA_FILE_EXTERNAL_PATHS_SPECIFIC_FS);
}

Expand Down Expand Up @@ -3060,32 +3061,6 @@ public InlineElement getDescription() {
}
}

/** Specifies the strategy for selecting specific filesystem storage paths. */
public enum ExternalFSStrategy implements DescribedEnum {
S3("S3", "Select S3 as the write path for the external path."),

OSS("OSS", "Select OSS as the write path for the external path.");

private final String value;

private final String description;

ExternalFSStrategy(String value, String description) {
this.value = value;
this.description = description;
}

@Override
public String toString() {
return value;
}

@Override
public InlineElement getDescription() {
return text(description);
}
}

/** Specifies the local file type for lookup. */
public enum LookupLocalFileType implements DescribedEnum {
SORT("sort", "Construct a sorted file for lookup."),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

package org.apache.paimon.fs;

import org.apache.paimon.CoreOptions.ExternalFSStrategy;
import org.apache.paimon.CoreOptions.ExternalPathStrategy;
import org.apache.paimon.annotation.VisibleForTesting;

Expand All @@ -32,11 +31,14 @@

/** Provider for external paths. */
public class ExternalPathProvider implements Serializable {
private final Map<ExternalFSStrategy, Path> externalPathsMap;
private static final String S3 = "s3";
private static final String OSS = "oss";

private final Map<String, Path> externalPathsMap;
private final List<Path> externalPathsList;

private final ExternalPathStrategy externalPathStrategy;
private final ExternalFSStrategy externalFSStrategy;
private final String externalFSStrategy;
private int currentIndex;
private boolean externalPathExists;
private final String dbAndTableRelativePath;
Expand All @@ -53,7 +55,7 @@ public ExternalPathProvider() {
public ExternalPathProvider(
String externalPaths,
ExternalPathStrategy externalPathStrategy,
ExternalFSStrategy externalFSStrategy,
String externalFSStrategy,
String dbAndTableRelativePath) {
this.externalPathsMap = new HashMap<>();
this.externalPathsList = new ArrayList<>();
Expand All @@ -68,14 +70,22 @@ private void initExternalPaths(String externalPaths) {
if (externalPaths == null) {
return;
}

if (externalPathStrategy != null
&& externalPathStrategy.equals(ExternalPathStrategy.SPECIFIC_FS)) {
if (externalFSStrategy == null) {
throw new IllegalArgumentException("external fs strategy should not be null: ");
}
}

String[] tmpArray = externalPaths.split(",");
for (String part : tmpArray) {
String path = part.trim();
if (path.toLowerCase().startsWith("oss")) {
externalPathsMap.put(ExternalFSStrategy.OSS, new Path(path));
if (path.toLowerCase().startsWith(OSS)) {
externalPathsMap.put(OSS, new Path(path));
externalPathsList.add(new Path(path));
} else if (path.toLowerCase().startsWith("s3")) {
externalPathsMap.put(ExternalFSStrategy.S3, new Path(path));
} else if (path.toLowerCase().startsWith(S3)) {
externalPathsMap.put(S3, new Path(path));
externalPathsList.add(new Path(path));
} else {
throw new IllegalArgumentException("Unsupported external path: " + path);
Expand Down Expand Up @@ -112,23 +122,17 @@ public Optional<Path> getNextExternalPath() {
}

private Optional<Path> getSpecificFSExternalPath() {
switch (externalFSStrategy) {
switch (externalFSStrategy.toLowerCase()) {
case S3:
if (!externalPathsMap.containsKey(ExternalFSStrategy.S3)) {
if (!externalPathsMap.containsKey(S3)) {
return Optional.empty();
}
return Optional.of(
new Path(
externalPathsMap.get(ExternalFSStrategy.S3),
dbAndTableRelativePath));
return Optional.of(new Path(externalPathsMap.get(S3), dbAndTableRelativePath));
case OSS:
if (!externalPathsMap.containsKey(ExternalFSStrategy.OSS)) {
if (!externalPathsMap.containsKey(OSS)) {
return Optional.empty();
}
return Optional.of(
new Path(
externalPathsMap.get(ExternalFSStrategy.OSS),
dbAndTableRelativePath));
return Optional.of(new Path(externalPathsMap.get(OSS), dbAndTableRelativePath));
default:
throw new IllegalArgumentException(
"Unsupported external fs strategy: " + externalFSStrategy);
Expand All @@ -145,7 +149,7 @@ public boolean externalPathExists() {
}

@VisibleForTesting
public Map<ExternalFSStrategy, Path> getExternalPathsMap() {
public Map<String, Path> getExternalPathsMap() {
return externalPathsMap;
}

Expand All @@ -169,21 +173,22 @@ public boolean equals(Object o) {
&& externalPathsMap.equals(that.externalPathsMap)
&& externalPathsList.equals(that.externalPathsList)
&& externalPathStrategy == that.externalPathStrategy
&& externalFSStrategy == that.externalFSStrategy
&& Objects.equals(externalFSStrategy, that.externalFSStrategy)
&& Objects.equals(dbAndTableRelativePath, that.dbAndTableRelativePath);
}

@Override
public String toString() {
return "ExternalPathProvider{"
+ " externalPathsMap="
+ ", externalPathsMap="
+ externalPathsMap
+ ", externalPathsList="
+ externalPathsList
+ ", externalPathStrategy="
+ externalPathStrategy
+ ", externalFSStrategy="
+ ", externalFSStrategy='"
+ externalFSStrategy
+ '\''
+ ", currentIndex="
+ currentIndex
+ ", externalPathExists="
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

package org.apache.paimon.fs;

import org.apache.paimon.CoreOptions.ExternalFSStrategy;
import org.apache.paimon.CoreOptions.ExternalPathStrategy;

import org.assertj.core.api.Assertions;
Expand All @@ -31,7 +30,6 @@

/** Test for {@link ExternalPathProvider}. */
public class ExternalPathProviderTest {

private ExternalPathProvider provider;

@BeforeEach
Expand Down Expand Up @@ -72,7 +70,7 @@ public void testGetNextExternalPathSpecificFS() {
new ExternalPathProvider(
"oss://bucket1/path1,s3://bucket2/path2",
ExternalPathStrategy.SPECIFIC_FS,
ExternalFSStrategy.OSS,
"OSS",
"db/table");

Optional<Path> path = provider.getNextExternalPath();
Expand All @@ -86,7 +84,7 @@ public void testGetNextExternalPathNone() {
new ExternalPathProvider(
"oss://bucket1/path1,s3://bucket2/path2",
ExternalPathStrategy.NONE,
ExternalFSStrategy.OSS,
"OSS",
"db/table");

Optional<Path> path = provider.getNextExternalPath();
Expand All @@ -100,7 +98,7 @@ public void testUnsupportedExternalPath() {
new ExternalPathProvider(
"hdfs://bucket1/path1",
ExternalPathStrategy.ROUND_ROBIN,
ExternalFSStrategy.OSS,
"oss",
"db/table");
})
.isInstanceOf(IllegalArgumentException.class);
Expand All @@ -110,11 +108,21 @@ public void testUnsupportedExternalPath() {
public void testUnsupportedExternalFSStrategy() {
provider =
new ExternalPathProvider(
"oss://bucket1/path1",
ExternalPathStrategy.SPECIFIC_FS,
ExternalFSStrategy.S3,
"db/table");
"oss://bucket1/path1", ExternalPathStrategy.SPECIFIC_FS, "S3", "db/table");
Optional<Path> path = provider.getNextExternalPath();
assertThat(path.isPresent()).isFalse();
}

@Test
public void testExternalFSStrategyNull() {
Assertions.assertThatThrownBy(
() -> {
new ExternalPathProvider(
"oss://bucket1/path1",
ExternalPathStrategy.SPECIFIC_FS,
null,
"db/table");
})
.isInstanceOf(IllegalArgumentException.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.apache.paimon.table;

import org.apache.paimon.CoreOptions;
import org.apache.paimon.CoreOptions.ExternalFSStrategy;
import org.apache.paimon.CoreOptions.ExternalPathStrategy;
import org.apache.paimon.catalog.CatalogContext;
import org.apache.paimon.fs.ExternalPathProvider;
Expand Down Expand Up @@ -84,7 +83,7 @@ private static ExternalPathProvider getExternalPathProvider(
CoreOptions coreOptions = CoreOptions.fromMap(tableSchema.options());
String externalPaths = coreOptions.dataFileExternalPaths();
ExternalPathStrategy externalPathStrategy = coreOptions.externalPathStrategy();
ExternalFSStrategy externalSpecificFSStrategy = coreOptions.externalSpecificFSStrategy();
String externalSpecificFSStrategy = coreOptions.externalSpecificFSStrategy();
String dbAndTablePath = tablePath.getParent().getName() + "/" + tablePath.getName();
return new ExternalPathProvider(
externalPaths, externalPathStrategy, externalSpecificFSStrategy, dbAndTablePath);
Expand Down

0 comments on commit f3303c1

Please sign in to comment.