[ 
https://issues.apache.org/jira/browse/DRILL-5547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16083201#comment-16083201
 ] 

ASF GitHub Bot commented on DRILL-5547:
---------------------------------------

Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/868#discussion_r126829118
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
 ---
    @@ -201,9 +240,35 @@
     
       public SystemOptionManager(LogicalPlanPersistence lpPersistence, final 
PersistentStoreProvider provider) {
         this.provider = provider;
    -    this.config =  
PersistentStoreConfig.newJacksonBuilder(lpPersistence.getMapper(), 
OptionValue.class)
    -        .name("sys.options")
    -        .build();
    +    this.config = 
PersistentStoreConfig.newJacksonBuilder(lpPersistence.getMapper(), 
OptionValue.class)
    --- End diff --
    
    A really handy trick to avoid code duplication is this:
    
    ```
    public SystemOptionManager(LogicalPlanPersistence lpPersistence, final 
PersistentStoreProvider provider) {
      this(lpPersistence, provider, null);
    }
    ```
    
    Then, the bulk of the constructor code appears only once: in the three-arg 
version.


> 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
>             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)

Reply via email to