You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@paimon.apache.org by lz...@apache.org on 2023/12/27 08:02:53 UTC
(incubator-paimon) branch master updated: [core] Remove ConfigOptions toList (#2578)
This is an automated email from the ASF dual-hosted git repository.
lzljs3620320 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-paimon.git
The following commit(s) were added to refs/heads/master by this push:
new 3be4f97c6 [core] Remove ConfigOptions toList (#2578)
3be4f97c6 is described below
commit 3be4f97c68e568d0d96ff74c8a5c4f1b4f08073e
Author: Jingsong Lee <ji...@gmail.com>
AuthorDate: Wed Dec 27 16:02:49 2023 +0800
[core] Remove ConfigOptions toList (#2578)
---
docs/content/concepts/primary-key-table.md | 2 +-
.../org/apache/paimon/options/ConfigOption.java | 19 ++------
.../org/apache/paimon/options/ConfigOptions.java | 54 ++--------------------
.../java/org/apache/paimon/options/Options.java | 6 +--
.../org/apache/paimon/options/OptionsUtils.java | 14 +-----
.../configuration/ConfigOptionsDocGenerator.java | 39 +++-------------
.../ConfigOptionsDocGeneratorTest.java | 28 -----------
7 files changed, 16 insertions(+), 146 deletions(-)
diff --git a/docs/content/concepts/primary-key-table.md b/docs/content/concepts/primary-key-table.md
index 2fa2ed87a..772c403e3 100644
--- a/docs/content/concepts/primary-key-table.md
+++ b/docs/content/concepts/primary-key-table.md
@@ -329,7 +329,7 @@ Current supported aggregate functions and data types are:
* `nested-update`:
The nested-update function collects multiple rows into one array<row> (so-called 'nested table'). It supports ARRAY<ROW> data types.
- Use `fields.<field-name>.nested-key=pk0,pk1;...` to specify the primary keys of the nested table. If no keys, row will be appended to array<row>.
+ Use `fields.<field-name>.nested-key=pk0,pk1,...` to specify the primary keys of the nested table. If no keys, row will be appended to array<row>.
An example:
diff --git a/paimon-common/src/main/java/org/apache/paimon/options/ConfigOption.java b/paimon-common/src/main/java/org/apache/paimon/options/ConfigOption.java
index d6a0c0ed3..399683aa7 100644
--- a/paimon-common/src/main/java/org/apache/paimon/options/ConfigOption.java
+++ b/paimon-common/src/main/java/org/apache/paimon/options/ConfigOption.java
@@ -70,23 +70,16 @@ public class ConfigOption<T> {
* <ul>
* <li>typeClass == atomic class (e.g. {@code Integer.class}) -> {@code ConfigOption<Integer>}
* <li>typeClass == {@code Map.class} -> {@code ConfigOption<Map<String, String>>}
- * <li>typeClass == atomic class and isList == true for {@code ConfigOption<List<Integer>>}
* </ul>
*/
private final Class<?> clazz;
- private final boolean isList;
-
// ------------------------------------------------------------------------
Class<?> getClazz() {
return clazz;
}
- boolean isList() {
- return isList;
- }
-
/**
* Creates a new config option with fallback keys.
*
@@ -94,8 +87,6 @@ public class ConfigOption<T> {
* @param clazz describes type of the ConfigOption, see description of the clazz field
* @param description Description for that option
* @param defaultValue The default value for this option
- * @param isList tells if the ConfigOption describes a list option, see description of the clazz
- * field
* @param fallbackKeys The list of fallback keys, in the order to be checked
*/
ConfigOption(
@@ -103,14 +94,12 @@ public class ConfigOption<T> {
Class<?> clazz,
Description description,
T defaultValue,
- boolean isList,
FallbackKey... fallbackKeys) {
this.key = checkNotNull(key);
this.description = description;
this.defaultValue = defaultValue;
this.fallbackKeys = fallbackKeys == null || fallbackKeys.length == 0 ? EMPTY : fallbackKeys;
this.clazz = checkNotNull(clazz);
- this.isList = isList;
}
// ------------------------------------------------------------------------
@@ -134,8 +123,7 @@ public class ConfigOption<T> {
// put fallback keys first so that they are prioritized
final FallbackKey[] mergedAlternativeKeys =
Stream.concat(newFallbackKeys, currentAlternativeKeys).toArray(FallbackKey[]::new);
- return new ConfigOption<>(
- key, clazz, description, defaultValue, isList, mergedAlternativeKeys);
+ return new ConfigOption<>(key, clazz, description, defaultValue, mergedAlternativeKeys);
}
/**
@@ -158,8 +146,7 @@ public class ConfigOption<T> {
final FallbackKey[] mergedAlternativeKeys =
Stream.concat(currentAlternativeKeys, newDeprecatedKeys)
.toArray(FallbackKey[]::new);
- return new ConfigOption<>(
- key, clazz, description, defaultValue, isList, mergedAlternativeKeys);
+ return new ConfigOption<>(key, clazz, description, defaultValue, mergedAlternativeKeys);
}
/**
@@ -181,7 +168,7 @@ public class ConfigOption<T> {
* @return A new config option, with given description.
*/
public ConfigOption<T> withDescription(final Description description) {
- return new ConfigOption<>(key, clazz, description, defaultValue, isList, fallbackKeys);
+ return new ConfigOption<>(key, clazz, description, defaultValue, fallbackKeys);
}
// ------------------------------------------------------------------------
diff --git a/paimon-common/src/main/java/org/apache/paimon/options/ConfigOptions.java b/paimon-common/src/main/java/org/apache/paimon/options/ConfigOptions.java
index 72f561335..caef58946 100644
--- a/paimon-common/src/main/java/org/apache/paimon/options/ConfigOptions.java
+++ b/paimon-common/src/main/java/org/apache/paimon/options/ConfigOptions.java
@@ -22,8 +22,6 @@ import org.apache.paimon.annotation.Public;
import org.apache.paimon.options.description.Description;
import java.time.Duration;
-import java.util.Arrays;
-import java.util.List;
import java.util.Map;
import static org.apache.paimon.utils.Preconditions.checkNotNull;
@@ -181,8 +179,7 @@ public class ConfigOptions {
@Deprecated
public <T> ConfigOption<T> defaultValue(T value) {
checkNotNull(value);
- return new ConfigOption<>(
- key, value.getClass(), ConfigOption.EMPTY_DESCRIPTION, value, false);
+ return new ConfigOption<>(key, value.getClass(), ConfigOption.EMPTY_DESCRIPTION, value);
}
/**
@@ -195,8 +192,7 @@ public class ConfigOptions {
*/
@Deprecated
public ConfigOption<String> noDefaultValue() {
- return new ConfigOption<>(
- key, String.class, ConfigOption.EMPTY_DESCRIPTION, null, false);
+ return new ConfigOption<>(key, String.class, ConfigOption.EMPTY_DESCRIPTION, null);
}
}
@@ -214,11 +210,6 @@ public class ConfigOptions {
this.clazz = clazz;
}
- /** Defines that the option's type should be a list of previously defined atomic type. */
- public ListConfigOptionBuilder<T> asList() {
- return new ListConfigOptionBuilder<>(key, clazz);
- }
-
/**
* Creates a ConfigOption with the given default value.
*
@@ -226,7 +217,7 @@ public class ConfigOptions {
* @return The config option with the default value.
*/
public ConfigOption<T> defaultValue(T value) {
- return new ConfigOption<>(key, clazz, ConfigOption.EMPTY_DESCRIPTION, value, false);
+ return new ConfigOption<>(key, clazz, ConfigOption.EMPTY_DESCRIPTION, value);
}
/**
@@ -235,44 +226,7 @@ public class ConfigOptions {
* @return The config option without a default value.
*/
public ConfigOption<T> noDefaultValue() {
- return new ConfigOption<>(
- key, clazz, Description.builder().text("").build(), null, false);
- }
- }
-
- /**
- * Builder for {@link ConfigOption} of list of type {@link E}.
- *
- * @param <E> list element type of the option
- */
- public static class ListConfigOptionBuilder<E> {
- private final String key;
- private final Class<E> clazz;
-
- ListConfigOptionBuilder(String key, Class<E> clazz) {
- this.key = key;
- this.clazz = clazz;
- }
-
- /**
- * Creates a ConfigOption with the given default value.
- *
- * @param values The list of default values for the config option
- * @return The config option with the default value.
- */
- @SafeVarargs
- public final ConfigOption<List<E>> defaultValues(E... values) {
- return new ConfigOption<>(
- key, clazz, ConfigOption.EMPTY_DESCRIPTION, Arrays.asList(values), true);
- }
-
- /**
- * Creates a ConfigOption without a default value.
- *
- * @return The config option without a default value.
- */
- public ConfigOption<List<E>> noDefaultValue() {
- return new ConfigOption<>(key, clazz, ConfigOption.EMPTY_DESCRIPTION, null, true);
+ return new ConfigOption<>(key, clazz, Description.builder().text("").build(), null);
}
}
diff --git a/paimon-common/src/main/java/org/apache/paimon/options/Options.java b/paimon-common/src/main/java/org/apache/paimon/options/Options.java
index ee8a249df..4510dc8a0 100644
--- a/paimon-common/src/main/java/org/apache/paimon/options/Options.java
+++ b/paimon-common/src/main/java/org/apache/paimon/options/Options.java
@@ -98,11 +98,7 @@ public class Options implements Serializable {
Class<?> clazz = option.getClazz();
try {
- if (option.isList()) {
- return rawValue.map(v -> OptionsUtils.convertToList(v, clazz));
- } else {
- return rawValue.map(v -> OptionsUtils.convertValue(v, clazz));
- }
+ return rawValue.map(v -> OptionsUtils.convertValue(v, clazz));
} catch (Exception e) {
throw new IllegalArgumentException(
String.format(
diff --git a/paimon-common/src/main/java/org/apache/paimon/options/OptionsUtils.java b/paimon-common/src/main/java/org/apache/paimon/options/OptionsUtils.java
index 36d05ef63..47eb45007 100644
--- a/paimon-common/src/main/java/org/apache/paimon/options/OptionsUtils.java
+++ b/paimon-common/src/main/java/org/apache/paimon/options/OptionsUtils.java
@@ -77,18 +77,6 @@ public class OptionsUtils {
throw new IllegalArgumentException("Unsupported type: " + clazz);
}
- @SuppressWarnings("unchecked")
- static <T> T convertToList(Object rawValue, Class<?> atomicClass) {
- if (rawValue instanceof List) {
- return (T) rawValue;
- } else {
- return (T)
- StructuredOptionsSplitter.splitEscaped(rawValue.toString(), ';').stream()
- .map(s -> convertValue(s, atomicClass))
- .collect(Collectors.toList());
- }
- }
-
@SuppressWarnings("unchecked")
static Map<String, String> convertToProperties(Object o) {
if (o instanceof Map) {
@@ -273,7 +261,7 @@ public class OptionsUtils {
* </pre>
*/
public static boolean canBePrefixMap(ConfigOption<?> configOption) {
- return configOption.getClazz() == Map.class && !configOption.isList();
+ return configOption.getClazz() == Map.class;
}
/** Filter condition for prefix map keys. */
diff --git a/paimon-docs/src/main/java/org/apache/paimon/docs/configuration/ConfigOptionsDocGenerator.java b/paimon-docs/src/main/java/org/apache/paimon/docs/configuration/ConfigOptionsDocGenerator.java
index 209447457..bef414915 100644
--- a/paimon-docs/src/main/java/org/apache/paimon/docs/configuration/ConfigOptionsDocGenerator.java
+++ b/paimon-docs/src/main/java/org/apache/paimon/docs/configuration/ConfigOptionsDocGenerator.java
@@ -449,49 +449,22 @@ public class ConfigOptionsDocGenerator {
}
}
- private static boolean isList(ConfigOption<?> option) {
- try {
- Method getClazzMethod = ConfigOption.class.getDeclaredMethod("isList");
- getClazzMethod.setAccessible(true);
- boolean isList = (boolean) getClazzMethod.invoke(option);
- getClazzMethod.setAccessible(false);
- return isList;
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- }
-
@VisibleForTesting
static String typeToHtml(OptionWithMetaInfo optionWithMetaInfo) {
ConfigOption<?> option = optionWithMetaInfo.option;
Class<?> clazz = getClazz(option);
- boolean isList = isList(option);
if (clazz.isEnum()) {
- return enumTypeToHtml(isList);
+ return enumTypeToHtml();
}
- return atomicTypeToHtml(clazz, isList);
+ return atomicTypeToHtml(clazz);
}
- private static String atomicTypeToHtml(Class<?> clazz, boolean isList) {
- String typeName = clazz.getSimpleName();
-
- final String type;
- if (isList) {
- type = String.format("List<%s>", typeName);
- } else {
- type = typeName;
- }
- return escapeCharacters(type);
+ private static String atomicTypeToHtml(Class<?> clazz) {
+ return escapeCharacters(clazz.getSimpleName());
}
- private static String enumTypeToHtml(boolean isList) {
- final String type;
- if (isList) {
- type = "List<Enum>";
- } else {
- type = "Enum";
- }
- return String.format("<p>%s</p>", escapeCharacters(type));
+ private static String enumTypeToHtml() {
+ return String.format("<p>%s</p>", escapeCharacters("Enum"));
}
@VisibleForTesting
diff --git a/paimon-docs/src/test/java/org/apache/paimon/docs/configuration/ConfigOptionsDocGeneratorTest.java b/paimon-docs/src/test/java/org/apache/paimon/docs/configuration/ConfigOptionsDocGeneratorTest.java
index f46cf4c15..96d72c9aa 100644
--- a/paimon-docs/src/test/java/org/apache/paimon/docs/configuration/ConfigOptionsDocGeneratorTest.java
+++ b/paimon-docs/src/test/java/org/apache/paimon/docs/configuration/ConfigOptionsDocGeneratorTest.java
@@ -94,13 +94,6 @@ class ConfigOptionsDocGeneratorTest {
.defaultValue(TestEnum.VALUE_1)
.withDescription("Description");
- public static ConfigOption<List<TestEnum>> enumListOption =
- ConfigOptions.key("option.enum.list")
- .enumType(TestEnum.class)
- .asList()
- .defaultValues(TestEnum.VALUE_1, TestEnum.VALUE_2)
- .withDescription("Description");
-
public static ConfigOption<DescribedTestEnum> describedEnum =
ConfigOptions.key("option.enum.described")
.enumType(DescribedTestEnum.class)
@@ -119,15 +112,6 @@ class ConfigOptionsDocGeneratorTest {
.defaultValue(Collections.singletonMap("key1", "value1"))
.withDescription("Description");
- public static ConfigOption<List<Map<String, String>>> mapListOption =
- ConfigOptions.key("option.map.list")
- .mapType()
- .asList()
- .defaultValues(
- Collections.singletonMap("key1", "value1"),
- Collections.singletonMap("key2", "value2"))
- .withDescription("Description");
-
public static ConfigOption<Duration> durationOption =
ConfigOptions.key("option.duration")
.durationType()
@@ -167,24 +151,12 @@ class ConfigOptionsDocGeneratorTest {
+ " <td>Description<br /><br />Possible values:<ul><li>\"A\": First letter of the alphabet</li><li>\"B\": Second letter of the alphabet</li></ul></td>\n"
+ " </tr>\n"
+ " <tr>\n"
- + " <td><h5>option.enum.list</h5></td>\n"
- + " <td style=\"word-wrap: break-word;\">VALUE_1;<wbr>VALUE_2</td>\n"
- + " <td><p>List<Enum></p></td>\n"
- + " <td>Description<br /><br />Possible values:<ul><li>\"VALUE_1\"</li><li>\"VALUE_2\"</li><li>\"VALUE_3\"</li></ul></td>\n"
- + " </tr>\n"
- + " <tr>\n"
+ " <td><h5>option.map</h5></td>\n"
+ " <td style=\"word-wrap: break-word;\">key1:value1</td>\n"
+ " <td>Map</td>\n"
+ " <td>Description</td>\n"
+ " </tr>\n"
+ " <tr>\n"
- + " <td><h5>option.map.list</h5></td>\n"
- + " <td style=\"word-wrap: break-word;\">key1:value1;<wbr>key2:value2</td>\n"
- + " <td>List<Map></td>\n"
- + " <td>Description</td>\n"
- + " </tr>\n"
- + " <tr>\n"
+ " <td><h5>option.memory</h5></td>\n"
+ " <td style=\"word-wrap: break-word;\">1 kb</td>\n"
+ " <td>MemorySize</td>\n"