[
https://issues.apache.org/jira/browse/DRILL-5547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16142346#comment-16142346
]
ASF GitHub Bot commented on DRILL-5547:
---------------------------------------
Github user paul-rogers commented on the issue:
https://github.com/apache/drill/pull/868
From a note sent by @dvjyothsna:
All the system/session option default values are externalized to the conf
file. Now the options are looked up in the hierarchical order of SESSION -
SYSTEM - CONFIG, the session being given the highest precedence. So, if the
user doesn't set system/session option by issuing alter system/session, option
manager fetches the value from the config. Now we need not bother about
getting the value from config separately if it is not in the system/session
level. For example:
```
OptionValue value = sessionOptions.getOption(JAVA_COMPILER_OPTION);
policy = CompilerPolicy.valueOf((value != null)
? value.string_val.toUpperCase()
: config.getString(JAVA_COMPILER_CONFIG)
```
In the above code snippet we try to get option value and if it is not set
(if it is null) we try to get it from Config. With my code changes if the value
is not set at system/session level, value from the config is picked
automatically.For example, the above can be simplified to:
```
policy =
CompilerPolicy.valueOf(sessionOptions.getOption(JAVA_COMPILER_OPTION));
```
I have already externalized all the existing options in ExecConstants,
PlannerSettings, etc.,
to the drill-module.conf. Following are the steps to add a new
system/session option.
Steps to add a new system/session option:
1. Add a validator in the ExecConstants.java or PlannerSettings.java or
whichever is appropriate for the option. For example:
```
String NEW_OPTION_KEY = "drill.exec.new_option";
OptionValidator NEW_OPTION= new DoubleValidator(NEW_OPTION_KEY);
```
2. Add the validator from the above step to the validators list in
`SystemOptionManager.java`.
3. Add the option in the conf file (for example, `drill-module.conf` or
`drill-override.conf` file ) under the name space `drill.exec.options`. For
example:
```
drill.exec.options : {
drill : {
exec : {
new_option : 1.5
}
}
}
```
4. Set the default value for the option in the conf file as shown above.
5. OptionManager loads default values from the conf file, User can override
the default values by issuing `ALTER SYSTEM/SESSION`.
> Drill config options and session options do not work as intended
> ----------------------------------------------------------------
>
> Key: DRILL-5547
> URL: https://issues.apache.org/jira/browse/DRILL-5547
> Project: Apache Drill
> Issue Type: Bug
> Components: Server
> Affects Versions: 1.10.0
> Reporter: Karthikeyan Manivannan
> Assignee: Venkata Jyothsna Donapati
> Labels: ready-to-commit
> Fix For: Future
>
>
> In Drill, session options should take precedence over config options. But
> several of these session options are assigned hard-coded default values when
> the option validators are initialized. Because of this config options will
> never be read and honored even if the user did not specify the session
> option.
> ClassCompilerSelector.JAVA_COMPILER_VALIDATOR uses CompilerPolicy.DEFAULT as
> the default value. This default value gets into the session options map via
> the initialization of validators in SystemOptionManager.
> Now any piece of code that tries to check if a session option is set will
> never see a null, so it will always use that value and never try to look into
> the config options. For example, in the following piece of code from
> ClassCompilerSelector (), the policy will never be read from the config file.
> policy = CompilerPolicy.valueOf((value != null) ?
> value.string_val.toUpperCase() :
> config.getString(JAVA_COMPILER_CONFIG).toUpperCase());
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
