[ https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14900989#comment-14900989 ]
ASF GitHub Bot commented on DRILL-1065: --------------------------------------- Github user adeneche commented on a diff in the pull request: https://github.com/apache/drill/pull/159#discussion_r39996054 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java --- @@ -49,45 +53,67 @@ public SetOptionHandler(QueryContext context) { } @Override - public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConversionException, IOException, ForemanSetupException { + public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, ForemanSetupException { final SqlSetOption option = unwrap(sqlNode, SqlSetOption.class); - final String scope = option.getScope(); - final String name = option.getName(); final SqlNode value = option.getValue(); - OptionValue.OptionType type; - if (value instanceof SqlLiteral) { + if (value != null && !(value instanceof SqlLiteral)) { + throw UserException.validationError() + .message("Drill does not support assigning non-literal values in SET statements.") + .build(logger); + } + + final String scope = option.getScope(); + final OptionValue.OptionType type; + if (scope == null) { // No scope mentioned assumed SESSION + type = OptionType.SESSION; + } else { switch (scope.toLowerCase()) { - case "session": - type = OptionValue.OptionType.SESSION; - break; - case "system": - type = OptionValue.OptionType.SYSTEM; - break; -// case "query": -// type = OptionValue.OptionType.QUERY; -// break; - default: - throw new ValidationException("Invalid OPTION scope. Scope must be SESSION or SYSTEM."); + case "session": + type = OptionType.SESSION; + break; + case "system": + type = OptionType.SYSTEM; + break; + default: + throw UserException.validationError() + .message("Invalid OPTION scope %s. Scope must be SESSION or SYSTEM.", scope) + .build(logger); } + } - if (type == OptionType.SYSTEM) { - // If the user authentication is enabled, make sure the user who is trying to change the system option has - // administrative privileges. - if (context.isUserAuthenticationEnabled() && - !ImpersonationUtil.hasAdminPrivileges( - context.getQueryUserName(), - context.getOptions().getOption(ExecConstants.ADMIN_USERS_KEY).string_val, - context.getOptions().getOption(ExecConstants.ADMIN_USER_GROUPS_KEY).string_val)) { - throw UserException.permissionError() - .message("Not authorized to change SYSTEM options.") - .build(logger); - } + if (type == OptionType.SYSTEM) { + // If the user authentication is enabled, make sure the user who is trying to change the system option has + // administrative privileges. + if (context.isUserAuthenticationEnabled() && + !ImpersonationUtil.hasAdminPrivileges( + context.getQueryUserName(), + context.getOptions().getOption(ExecConstants.ADMIN_USERS_KEY).string_val, --- End diff -- you should reuse `context.getOptions()` as a local variable > Provide a reset command to reset an option to its default value > --------------------------------------------------------------- > > Key: DRILL-1065 > URL: https://issues.apache.org/jira/browse/DRILL-1065 > Project: Apache Drill > Issue Type: Improvement > Components: Execution - Flow > Reporter: Aman Sinha > Assignee: Sudheesh Katkam > Priority: Minor > Fix For: 1.2.0 > > > Within a session, currently we set configuration options and it would be very > useful to have a 'reset' command to reset the value of an option to its > default system value: > ALTER SESSION RESET <option name> > If we don't want to add a new keyword for RESET, we could potentially > overload the SET command and allow the user to set to the 'default' value. -- This message was sent by Atlassian JIRA (v6.3.4#6332)