[GitHub] drill pull request #1045: DRILL-5730 Test Mocking Improvements

[paul-rogers]

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

    https://github.com/apache/drill/pull/1045#discussion_r156177751
  
    --- Diff: 
contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/MaprDBJsonRecordReader.java
 ---
    @@ -117,7 +116,7 @@ public MaprDBJsonRecordReader(MapRDBSubScanSpec 
subScanSpec,
     
         disableCountOptimization = 
formatPluginConfig.disableCountOptimization();
         setColumns(projectedColumns);
    -    unionEnabled = 
context.getOptions().getOption(ExecConstants.ENABLE_UNION_TYPE);
    +    unionEnabled = 
context.getOptionSet().getOption(ExecConstants.ENABLE_UNION_TYPE);
    --- End diff --
    
    If you are cleaning up this code, consider using a newly added feature of 
the option manager, the ability to get the typed result. Rather than:
    
    ```
      public static final BooleanValidator ENABLE_UNION_TYPE = new 
BooleanValidator(ENABLE_UNION_TYPE_KEY);
    ...
    unionEnabled = 
context.getOptionSet().getOption(ExecConstants.ENABLE_UNION_TYPE);
    ```
    
    We can now do:
    ```
      public static final String ENABLE_UNION_TYPE_KEY = 
"exec.enable_union_type";
    ...
    unionEnabled = 
context.getOptionSet().getBoolean(ExecConstants.ENABLE_UNION_TYPE_KEY);
    ```
    
    This method handles value lookup, but also enforces types. (The existing 
code will just trigger an NPE if types don't match.) This new form also hides 
the details of the option validator as recent changes have ensured that the 
validators are already visible to the option manager internals.


---