nsivabalan commented on a change in pull request #4714:
URL: https://github.com/apache/hudi/pull/4714#discussion_r802141826
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -750,6 +757,22 @@ public PropertyBuilder
setCommitTimezone(HoodieTimelineTimeZone timelineTimeZone
return this;
}
+ public PropertyBuilder set(String key, Object value) {
+ if (HoodieTableConfig.PERSISTED_CONFIG_LIST.contains(key)) {
+ this.others.put(key, value);
+ }
+ return this;
+ }
+
+ public PropertyBuilder set(Map<String, Object> props) {
+ for (Map.Entry<String, Object> entry : props.entrySet()) {
Review comment:
can we go through PERSISTED_CONFIG_LIST here in this for loop and in
next line check if its part of incoming props list. iterating through a smaller
list is better :)
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -183,6 +185,14 @@
public static final ConfigProperty<String> URL_ENCODE_PARTITIONING =
KeyGeneratorOptions.URL_ENCODE_PARTITIONING;
public static final ConfigProperty<String> HIVE_STYLE_PARTITIONING_ENABLE =
KeyGeneratorOptions.HIVE_STYLE_PARTITIONING_ENABLE;
+ public static final List<String> PERSISTED_CONFIG_LIST = Arrays.asList(
+ Config.DATE_TIME_PARSER_PROP,
Review comment:
yes, please do open one.
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -643,6 +644,12 @@ public static PropertyBuilder withPropertyBuilder() {
private Boolean urlEncodePartitioning;
private HoodieTimelineTimeZone commitTimeZone;
+ /**
+ * Persist the configs that is written at the first time, and should not
be changed.
+ * Like KeyGenerator's configs.
+ */
+ private Properties others = new Properties();
Review comment:
So far we have never added loose configs (random key value pairs) to our
tableConfig. But these could be empty if not for timestamp based key gen. Can't
think of better ways.
btw, @xushiyan @codope @yihua : Adding new properties to tableConfig does
not warrant a new table version right? wanted to confirm w/ you folks.
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -759,6 +780,11 @@ public PropertyBuilder
fromMetaClient(HoodieTableMetaClient metaClient) {
public PropertyBuilder fromProperties(Properties properties) {
HoodieConfig hoodieConfig = new HoodieConfig(properties);
+
+ for (String key : hoodieConfig.getProps().stringPropertyNames()) {
Review comment:
do you mean to say, hoodieConfig.getProps().stringPropertyNames() will
be same as PERSISTED_CONFIG_LIST. Just trying to see if we can iterate through
smaller list among PERSISTED_CONFIG_LIST and
hoodieConfig.getProps().stringPropertyNames()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
