drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From paul-rogers <...@git.apache.org>
Subject [GitHub] drill pull request #859: DRILL-2478: Validating values assigned to SYSTEM/SE...
Date Thu, 22 Jun 2017 00:23:03 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message