[
https://issues.apache.org/jira/browse/DRILL-2478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16058507#comment-16058507
]
ASF GitHub Bot commented on DRILL-2478:
---------------------------------------
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.
> Validating values assigned to SYSTEM/SESSION configuration parameters
> ---------------------------------------------------------------------
>
> Key: DRILL-2478
> URL: https://issues.apache.org/jira/browse/DRILL-2478
> Project: Apache Drill
> Issue Type: Bug
> Components: Execution - Flow
> Affects Versions: 1.11.0
> Environment: {code}
> 0: jdbc:drill:> select * from sys.version;
> +------------+----------------+-------------+-------------+------------+
> | commit_id | commit_message | commit_time | build_email | build_time |
> +------------+----------------+-------------+-------------+------------+
> | f658a3c513ddf7f2d1b0ad7aa1f3f65049a594fe | DRILL-2209 Insert
> ProjectOperator with MuxExchange | 09.03.2015 @ 01:49:18 EDT | Unknown |
> 09.03.2015 @ 04:50:05 EDT |
> +------------+----------------+-------------+-------------+------------+
> 1 row selected (0.046 seconds)
> {code}
> Reporter: Khurram Faraaz
> Fix For: Future
>
>
> Values that are assigned to configuration parameters of type SYSTEM and
> SESSION must be validated. Currently any value can be assigned to some of the
> SYSTEM/SESSION type parameters.
> Here are two examples where assignment of invalid values to store.format does
> not result in any error.
> {code}
> 0: jdbc:drill:> alter session set `store.format`='1';
> +------------+------------+
> | ok | summary |
> +------------+------------+
> | true | store.format updated. |
> +------------+------------+
> 1 row selected (0.02 seconds)
> {code}
> {code}
> 0: jdbc:drill:> alter session set `store.format`='foo';
> +------------+------------+
> | ok | summary |
> +------------+------------+
> | true | store.format updated. |
> +------------+------------+
> 1 row selected (0.039 seconds)
> {code}
> In some cases values to some of the configuration parameters are validated,
> like in this example, where trying to assign an invalid value to parameter
> store.parquet.compression results in an error, which is correct. However,
> this kind of validation is not performed for every configuration parameter of
> SYSTEM/SESSION type. These values that are assigned to parameters must be
> validated, and report errors if incorrect values are assigned by users.
> {code}
> 0: jdbc:drill:> alter session set `store.parquet.compression`='anything';
> Query failed: ExpressionParsingException: Option store.parquet.compression
> must be one of: [snappy, gzip, none]
> Error: exception while executing query: Failure while executing query.
> (state=,code=0)
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
