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

    https://github.com/apache/drill/pull/859#discussion_r123396761
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
    @@ -166,7 +166,7 @@
       String DEFAULT_TEMPORARY_WORKSPACE = 
"drill.exec.default_temporary_workspace";
     
       String OUTPUT_FORMAT_OPTION = "store.format";
    -  OptionValidator OUTPUT_FORMAT_VALIDATOR = new 
StringValidator(OUTPUT_FORMAT_OPTION, "parquet");
    +  OptionValidator OUTPUT_FORMAT_VALIDATOR = new 
EnumeratedStringValidator(OUTPUT_FORMAT_OPTION, "parquet", "parquet", "json", 
"psv", "csv", "tsv", "csvh");
    --- End diff --
    
    Is this the right approach? This validator means that anyone who adds an 
output file format must modify this file and rebuild all of Drill in order to 
use that format by default. Not sure we want such tight binding.
    
    Seems a better idea would be to register all known output formats at 
runtime, validate the option against that list, warn if the format is unknown, 
and default to Parquet.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to