[HUDI-5096] Upgrade jcommander to 1.78

[xicm]

Change Logs

Upgrade jcommander 1.72 -> 1.78 to resolve NPE issue for Boolean param without default value.

Impact

• Affects all main program args parsing
• resolves security vulnerabilities; see fix(sec): upgrade com.beust:jcommander to 1.75 #6961

Risk level (write none, low medium or high below)

low

Documentation Update

NA

Contributor's checklist

• Read through contributor's guide
• Change Logs and Impact were stated clearly
• Adequate tests were added if applicable
• CI passed

[xushiyan]

we can't just add default true to the variables and leave others false. what if some config changed default from false to true? we should point these to the properties' defaults

[xicm]

@xushiyan The change of the default value is a problem.
Double check with you, is it ok to do like this
boolean useJdbc = Boolean.parseBoolean(HIVE_USE_JDBC.defaultValue());

That means for boolean params we must pass "--key true/false" instead of "--key" if we want to overwrite the default value.

[xushiyan]

@xicm thanks for making further changes. Making it work like --key true/false is a breaking change for users, so we have to avoid. But this is an existing problem, correct? from the beginning, regardless of users setting --use-jdbc or not, because useJdbc defaults to true, users won't be able to overwrite it at all. Let's keep the behavior unchanged (not ideal but it's not an urgent problem) and focus on fixing the reported issue.

[xicm]

@xushiyan I reverted the latest commit.
For the params default to true, userJdbc, autoCreateDatabase,syncAsSparkDataSourceTable , I changed the arity =1 and default vaue to true. So user can overwrite them and other UTs can be successful.

[xushiyan]

@xicm this leads to inconsistency: some boolean args are arity=0 and some arity=1, which will cause confusion and won't be a good usage pattern. Besides, this basically ties usage with default values: what if an option default change from true to false, then we need to change arity and it will break. Please avoid any usage change in this PR and only tackle the NPE problem. As i mentioned, this is an existing problem and should be addressed separately and with more user-experience consideration.

[xicm]

Thanks for your explanation. I updated the pr.

[matthiasdg]

@xicm @xushiyan I am a bit late to the party, but just wanted to point out cbeust/jcommander#378
This means you can still override parameters that default to true unlike what you seem to believe. Worse, if they're set to true by default, adding them in the cli tool will toggle them to false. Or adding them multiple times will also change the effect (easy to verify now that a test was added).

Therefore, the behavior in hudi has already changed since #4175; before that the Boolean JCommander parameters had default values, such as public Boolean syncAsSparkDataSourceTable = true;, which meant adding --spark-datasource actually set this to false. Now on the other hand, since the default value comes from HIVE_SYNC_AS_DATA_SOURCE_TABLE and not through JCommander, adding --spark-datasource would also set this to true. So it is already quite confusing. With no defaults set in JCommander like currently is the case, it may make more sense (at least it's consistent), but then to disable spark-datasource, I think I'll actually have to add --spark-datasource --spark-datasource; if I don't add it, the default HIVE_SYNC_AS_DATA_SOURCE_TABLE value (=true) is used, if I add it once -> JCommander also sets it to true, twice -> JCommander flips it to false.

Based on this I'm personally in favor of enforcing arity=1 for all booleans, makes it clear what will happen and will throw an error if someone does not specify an arity, so then it's at least clear that the behavior changed (which now isn't).
But this probably deserves a separate issue?

[xicm]

@hudi-bot run azure

[xushiyan]

@xicm i found upgrading to 1.78 resolves the original NPE issue

[hudi-bot]

CI report:

• 36b07fc Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

• @hudi-bot run azure re-run the last Azure build

[xicm]

@xicm i found upgrading to 1.78 resolves the original NPE issue

Experts tell the truth. 👍