okumin commented on code in PR #4731:
URL: https://github.com/apache/hive/pull/4731#discussion_r1333203974
##########
common/src/test/org/apache/hadoop/hive/conf/TestHiveConf.java:
##########
@@ -176,6 +176,20 @@ public void testHiddenConfig() throws Exception {
}
}
+ @Test
+ public void testLockedConfig() throws Exception {
+ HiveConf conf = new HiveConf();
+ // Get the default value of the config
+ String defaultVal = conf.get("hive.execution.engine");
Review Comment:
I'd say we should explicitly set `hive.execution.engine=mr` here because it
could be possible that tez will be the default engine someday.
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -850,6 +851,10 @@ public static enum ConfVars {
HIVEIGNOREMAPJOINHINT("hive.ignore.mapjoin.hint", true, "Ignore the
mapjoin hint"),
+ HIVE_CONF_LOCKED_LIST("hive.conf.locked.list", "", "Comma separated " +
+ "list of configuration options which are locked and can not be
changed at runtime. Warning is logged and the " +
+ "change is ignored when user try to set these configs during
runtime"),
Review Comment:
I guess this variable should be listed in the restricted list. Otherwise,
users can bypass this option.
https://github.com/apache/hive/blob/539da19dc48740cb9c21cea0f8bff7ba0763e31b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java#L6996-L6999
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -6001,18 +6006,24 @@ public void verifyAndSet(String name, String value)
throws IllegalArgumentExcept
throw new IllegalArgumentException("Cannot modify " + name + " at
runtime. It is in the list"
+ " of parameters that can't be modified at runtime or is prefixed
by a restricted variable");
}
- String oldValue = name != null ? get(name) : null;
- if (name == null || value == null || !value.equals(oldValue)) {
- // When either name or value is null, the set method below will fail,
- // and throw IllegalArgumentException
- set(name, value);
+ if (!isLockedConfig(name)) {
Review Comment:
I prefer early return because it could be possible to add another
restriction in the future.
--
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: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
