xushiyan commented on code in PR #7068:
URL: https://github.com/apache/hudi/pull/7068#discussion_r1006874024
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java:
##########
@@ -108,45 +108,45 @@ public static class HiveSyncConfigParams {
+ "instead of org.apache.hudi package. Use this when you are in
the process of migrating from "
+ "com.uber.hoodie to org.apache.hudi. Stop using this after you
migrated the table definition to "
+ "org.apache.hudi input format.")
- public Boolean usePreApacheInputFormat;
+ public boolean usePreApacheInputFormat;
Review Comment:
this will make the member variables default to `false` which may differ from
the config property 's default value.
##########
hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/replication/TestHiveSyncGlobalCommitTool.java:
##########
@@ -69,6 +70,7 @@ private HiveSyncGlobalCommitParams
getGlobalCommitConfig(String commitTime) thro
params.loadedProps.setProperty(LOCAL_BASE_PATH,
localCluster.tablePath(DB_NAME, TBL_NAME));
params.loadedProps.setProperty(REMOTE_BASE_PATH,
remoteCluster.tablePath(DB_NAME, TBL_NAME));
params.loadedProps.setProperty(META_SYNC_GLOBAL_REPLICATE_TIMESTAMP.key(),
commitTime);
+ params.loadedProps.setProperty(HIVE_USE_JDBC.key(), "true");
Review Comment:
should not be needed if default value fixed
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/replication/HiveSyncGlobalCommitParams.java:
##########
@@ -85,7 +85,9 @@ public void load() throws IOException {
Properties mkGlobalHiveSyncProps(boolean forRemote) {
TypedProperties props = new TypedProperties(loadedProps);
- props.putAll(globalHiveSyncConfigParams.toProps());
+ globalHiveSyncConfigParams.toProps().entrySet().forEach(
+ e -> props.putIfAbsent(e.getKey(), e.getValue())
+ );
Review Comment:
why this change? if anything not working, it should be
`globalHiveSyncConfigParams.toProps()` to be fixed, not making a workaround
here, which masks the root issue.
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java:
##########
@@ -108,45 +108,45 @@ public static class HiveSyncConfigParams {
+ "instead of org.apache.hudi package. Use this when you are in
the process of migrating from "
+ "com.uber.hoodie to org.apache.hudi. Stop using this after you
migrated the table definition to "
+ "org.apache.hudi input format.")
- public Boolean usePreApacheInputFormat;
+ public boolean usePreApacheInputFormat;
@Deprecated
@Parameter(names = {"--use-jdbc"}, description = "Hive jdbc connect url")
- public Boolean useJdbc;
+ public boolean useJdbc;
Review Comment:
the member var should just be assigned to the config's default value, to be
consistent
--
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: commits-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
