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

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

Github user ilooner commented on a diff in the pull request:

    https://github.com/apache/drill/pull/923#discussion_r138132892
  
    --- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java ---
    @@ -56,7 +56,7 @@ public void checkChangedColumn() throws Exception {
         test("ALTER session SET `%s` = %d;", SLICE_TARGET,
           ExecConstants.SLICE_TARGET_DEFAULT);
         testBuilder()
    -        .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
    +        .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND 
optionScope = 'SESSION'", SLICE_TARGET)
    --- End diff --
    
    Yeah we are in a tough spot because **type** was not well defined 
previously, so the tests implicitly assumed **type** represented where an 
option was set. Now that we have settled on a definition for **type**, which is 
the set of scopes where an option can be set, we have deviated from the meaning 
**type** was given in the tests. One possible way out of this situation is to 
change the definition of **type** and **optionScope** again by swapping their 
meanings:
    
    * **type**: Would become where an option was set.
    * **optionScope**: Would become the set of scopes where an option could be 
set.
    
    This would minimize the changes required to the unit tests. It's hard to 
say how it would impact other user's scripts though because **type** was 
treated inconsistently in the code base, so I'm not sure how someone could have 
used the **type** information productively except to write unit tests, which 
verified incorrect behavior.
    
    Long story short, I'll swap the definitions of **type** and 
**optionScope**. I think that would minimize the impact on unit tests and 
users, but we cannot provide any guarantees for users who depended on type when 
it was inconsistently defined.



> Support System/Session Internal Options And Additional Option System Fixes
> --------------------------------------------------------------------------
>
>                 Key: DRILL-5723
>                 URL: https://issues.apache.org/jira/browse/DRILL-5723
>             Project: Apache Drill
>          Issue Type: New Feature
>            Reporter: Timothy Farkas
>            Assignee: Timothy Farkas
>
> This is a feature proposed by [~ben-zvi].
> Currently all the options are accessible by the user in sys.options. We would 
> like to add internal options which can be altered, but are not visible in the 
> sys.options table. These internal options could be seen by another alias 
> select * from internal.options. The intention would be to put new options we 
> weren't comfortable with exposing to the end user in this table.
> After the options and their corresponding features are considered stable they 
> could be changed to appear in the sys.option table.
> A bunch of other fixes to the Option system have been clubbed into this:
> * OptionValidators no longer hold default values. Default values are 
> contained in the SystemOptionManager
> * Options have an OptionDefinition. The option definition includes:
>   * A validator
>   * Metadata about the options visibility, required permissions, and the 
> scope in which it can be set.
> * The Option Manager interface has been cleaned up so that a Type is not 
> required to be passed in in order to set and delete options



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to