subject:"\[jira\] \[Commented\] \(DRILL\-1065\) Provide a reset command to reset an option to its default value"

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-10-01 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14940736#comment-14940736
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user asfgit closed the pull request at:

https://github.com/apache/drill/pull/159


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939174#comment-14939174
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40873420
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
 ---
@@ -179,6 +181,7 @@ public void validate(final OptionValue v) {
 
 public TypeValidator(final String name, final Kind kind, final 
OptionValue defValue) {
   super(name);
+  checkArgument(defValue.type == OptionType.SYSTEM, "Default value 
must be SYSTEM type.");
--- End diff --

Can you modify all of these to return UserException? Maybe create a static 
method ideally reporting the specific option and value as additional 
UserException context. 


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939177#comment-14939177
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40873825
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
 ---
@@ -179,6 +181,7 @@ public void validate(final OptionValue v) {
 
 public TypeValidator(final String name, final Kind kind, final 
OptionValue defValue) {
   super(name);
+  checkArgument(defValue.type == OptionType.SYSTEM, "Default value 
must be SYSTEM type.");
--- End diff --

This check is when a TypeValidator (static instance) is created, so 
developers know not to create a validator with non SYSTEM level option type. 
Nothing to do with usage. Right?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939190#comment-14939190
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40874245
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
 ---
@@ -179,6 +181,7 @@ public void validate(final OptionValue v) {
 
 public TypeValidator(final String name, final Kind kind, final 
OptionValue defValue) {
   super(name);
+  checkArgument(defValue.type == OptionType.SYSTEM, "Default value 
must be SYSTEM type.");
--- End diff --

I am confused.. what does "these" refer to? I do not see any other 
precondition checks.

That change was addressing Hakim's comment 
[here](https://github.com/apache/drill/pull/159/files#diff-767c81796fcc968c436bd67655218f1aR83).
 The issue was when ControlsOptionValidator was being registered with the 
SystemOptionManager, the default option was being stored within 
SystemOptionManager as a SESSION option.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939289#comment-14939289
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40877406
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
 ---
@@ -102,6 +125,29 @@ public void setOption(OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, final OptionType type) {
+try {
+  SystemOptionManager.getValidator(name); // ensure the option exists
+} catch (final IllegalArgumentException e) {
+  throw UserException.validationError(e)
+.build(logger);
--- End diff --

Initially, I made that change in the ```getValidator``` function, but it 
seemed like UserException was one level too deep (if that makes sense). This 
way the calling function can do whatever it wants with the exception e.g. wrap 
it in a UserException for the user, or ignore it in case of a unit test.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939202#comment-14939202
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40875160
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
 ---
@@ -179,6 +181,7 @@ public void validate(final OptionValue v) {
 
 public TypeValidator(final String name, final Kind kind, final 
OptionValue defValue) {
   super(name);
+  checkArgument(defValue.type == OptionType.SYSTEM, "Default value 
must be SYSTEM type.");
--- End diff --

There are three other checkArguments in this changeset. However, I think 
they also refer to dev mistakes instead of user errors.  Probably fine as is.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939180#comment-14939180
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40873899
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
 ---
@@ -179,6 +181,7 @@ public void validate(final OptionValue v) {
 
 public TypeValidator(final String name, final Kind kind, final 
OptionValue defValue) {
   super(name);
+  checkArgument(defValue.type == OptionType.SYSTEM, "Default value 
must be SYSTEM type.");
--- End diff --

Are all of these? I may have just missed the context. 


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939203#comment-14939203
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-144601466
  
Looks good. One small change above. Otherwise +1.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939328#comment-14939328
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40878882
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
 ---
@@ -102,6 +125,29 @@ public void setOption(OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, final OptionType type) {
+try {
+  SystemOptionManager.getValidator(name); // ensure the option exists
+} catch (final IllegalArgumentException e) {
+  throw UserException.validationError(e)
+.build(logger);
--- End diff --

The function call does not have to be within the context of a query e.g. 
unit tests can use ```SystemOptionManager.getValidator(...)```. But callers, 
within the context of a query, will wrap the exception in a UserException.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939153#comment-14939153
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-144591640
  
Rebased, squashed, and edited the commit message. Also, see 
[DRILL-3875](https://issues.apache.org/jira/browse/DRILL-3875).


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939201#comment-14939201
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40875088
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
 ---
@@ -102,6 +125,29 @@ public void setOption(OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, final OptionType type) {
+try {
+  SystemOptionManager.getValidator(name); // ensure the option exists
+} catch (final IllegalArgumentException e) {
+  throw UserException.validationError(e)
+.build(logger);
--- End diff --

Can we simply fix the lower level exception here. We should be throwing 
UserException (which mean we wouldn't have to catch here).


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939352#comment-14939352
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40880950
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
 ---
@@ -102,6 +125,29 @@ public void setOption(OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, final OptionType type) {
+try {
+  SystemOptionManager.getValidator(name); // ensure the option exists
+} catch (final IllegalArgumentException e) {
+  throw UserException.validationError(e)
+.build(logger);
--- End diff --

My main issue here is that we have an unfriendly message. One of the things 
that we should be doing is creating a user friendly message at the point of 
user exception creation. If you think it was too deep one level deeper, then 
you should make a friendly user message at this level. Trusting that the system 
level exception message is meaningful to the user should be avoided.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-26 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14909549#comment-14909549
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user adeneche commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-143511508
  
@sudheeshkatkam can you please rebase, and create a separate JIRA for 
removing *exec* from reserved words.

+1, LGTM except `CompoundIdentifierConverter.java` I don't have enough 
knowledge to review this class. @jacques-n can you please review the change in 
this file ?

Thanks


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-26 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14909554#comment-14909554
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-143511727
  
The CompoundIdentifierConverter.java changes look sound. There is no reason 
to identifier expansion for the purpose of SetOptions. +1 


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-26 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14909393#comment-14909393
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-143477295
  
Okay, let's skip this for now.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-25 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14908353#comment-14908353
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-143294316
  
Hey @julianhyde, looks like the 
[keyword](https://github.com/apache/incubator-calcite/blob/master/core/src/main/codegen/templates/Parser.jj#L5094)
 list can be extended but non-reserved keywords cannot be. Should I open a JIRA?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-25 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14908429#comment-14908429
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user julianhyde commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-143310179
  
Yes, open a JIRA.

We don't need NonReservedKeyWord() and CommonNonReservedKeyWord() to be 
separate anymore, so you can generate code into whichever one is most 
convenient.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-25 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14908764#comment-14908764
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-143367166
  
Here's the list of keywords that we need to add to the non-reserved list: 
*["EXEC", "JOIN", "WINDOW", "LARGE", "PARTITION", "OLD", "COLUMN"]*.

Given Calcite's 
[note](https://github.com/apache/incubator-calcite/blob/master/core/src/main/codegen/templates/Parser.jj#L5114)
 about such a list and the 
[standard](http://savage.net.au/SQL/sql-2003-2.bnf.html#xref-keywords), I am 
against making this change. There are a lot of unit test failures (PARSE ERROR) 
in TestWindowFrame and TestWindowFunctions with this change.

@julianhyde What are the implications of adding a keyword to the 
non-reserved list (say "JOIN")?

Also the option *"store.parquet.block-size"* has a "-" in it which is not 
allowed. So do we are escape the word? Or change the name?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902922#comment-14902922
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-142342234
  
We use "exec" as the first part of many option names (e.g. 
```exec.min_hash_table_size```), and unfortunately, "exec" is a keyword, and 
the parser throws an error (```... PARSE ERROR: Encountered "SET exec" ...```). 
We still have to escape keywords.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14903869#comment-14903869
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-142477062
  
(commenting here and not on the commit)

There are various ```getOption(...)``` convenient methods in the 
OptionManager interface with typed validators as parameters (like 
```StringValidator```) which return the value with that type (like 
```String```).


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14903669#comment-14903669
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-142454472
  
Let's fix this as part of the patch. Since we don't actually support EXEC 
(as far as I can remember), let's make this work nicely by removing it as a 
reserved word.  (I think there is an unreserved reserved word list in the 
parser). @julianhyde may be able to help point us in the write direction.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902810#comment-14902810
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40101415
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/CompoundIdentifierConverter.java
 ---
@@ -107,7 +117,7 @@ public SqlNode visitChild(
   }
   SqlNode newOperand = 
operand.accept(CompoundIdentifierConverter.this);
   enableComplex = localEnableComplex;
-  if (newOperand != operand) {
+  if (! newOperand.equalsDeep(operand, false)) {
--- End diff --

I'm a little confused by this change. Operands are expected to be 
immutable. As such, identity comparison should be sufficient. Are you trying to 
avoid excessive rewrites?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902811#comment-14902811
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jacques-n commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-142325962
  
As an aside, I'm really excited about this commit. Should make things much 
easier for end users. We should probably create a new jira to update the docs 
to remove escaping for all the options. 


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902816#comment-14902816
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40101930
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/CompoundIdentifierConverter.java
 ---
@@ -107,7 +117,7 @@ public SqlNode visitChild(
   }
   SqlNode newOperand = 
operand.accept(CompoundIdentifierConverter.this);
   enableComplex = localEnableComplex;
-  if (newOperand != operand) {
+  if (! newOperand.equalsDeep(operand, false)) {
--- End diff --

I reverted this change. Will post a new patch soon.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901579#comment-14901579
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40037509
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
 ---
@@ -83,14 +84,30 @@ public OptionValue getOption(final String name) {
*/
   abstract boolean setLocalOption(OptionValue value);
 
+  /**
+   * Deletes the option with given name for this manager without falling 
back.
+   *
+   * @param type option type
+   * @return true iff the option was successfully deleted
--- End diff --

I will clarify the docs in both places.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14900825#comment-14900825
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user jinfengni commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r39984380
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/CompoundIdentifierConverter.java
 ---
@@ -107,7 +117,7 @@ public SqlNode visitChild(
   }
   SqlNode newOperand = 
operand.accept(CompoundIdentifierConverter.this);
   enableComplex = localEnableComplex;
-  if (newOperand != operand) {
+  if (! newOperand.equalsDeep(operand, false)) {
--- End diff --

Is it necessary to call equalsDeep()?  If the expression has an identifier 
which is rewritten by CompoundIdentifierConverter, is it true that the new 
expression would be different in reference from then original one?



> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901021#comment-14901021
 ] 

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_r39997945
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
 ---
@@ -241,6 +243,30 @@ public void setOption(final OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, OptionType type) {
+assert type == OptionType.SYSTEM;
+try { // ensure option exists
--- End diff --

you can extract this try block as a static method `ensureOptionExists`, 
this same code is duplicated in multiple places


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901000#comment-14901000
 ] 

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_r39996854
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
 ---
@@ -83,14 +84,30 @@ public OptionValue getOption(final String name) {
*/
   abstract boolean setLocalOption(OptionValue value);
 
+  /**
+   * Deletes the option with given name for this manager without falling 
back.
+   *
+   * @param type option type
+   * @return true iff the option was successfully deleted
--- End diff --

this javadoc can have multiple meanings: _successfully deleted_ could 
either mean the implementation couldn't find the option, or it doesn't support 
the option type. It should actually return false if it doesn't support the 
option type


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901009#comment-14901009
 ] 

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_r39997204
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/InMemoryOptionManager.java
 ---
@@ -54,12 +56,32 @@ boolean setLocalOption(final OptionValue value) {
 return options.values();
   }
 
+  @Override
+  boolean deleteLocalOptions(final OptionType type) {
+if (supportsOption(type)) {
+  options.clear();
+  return true;
+} else {
+  return false;
+}
+  }
+
+  @Override
+  boolean deleteLocalOption(final String name, final OptionType type) {
+if (supportsOption(type)) {
+  options.remove(name);
+  return true;
+} else {
+  return false;
+}
+  }
+
   /**
-   * Check to see if implementations of this manager support the given 
option value (e.g. check for option type).
+   * Check to see if implementations of this manager support the given 
option type.
*
-   * @param value the option value
-   * @return true iff the option value is supported
+   * @param type option type
+   * @return true iff the type is supported
*/
-  abstract boolean supportsOption(OptionValue value);
+  abstract boolean supportsOption(OptionType type);
--- End diff --

you should probably rename this to `supportsOptionType` because we are 
effectively checking if the option type is supported. This will help the method 
be self explanatory


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901010#comment-14901010
 ] 

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_r39997250
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java
 ---
@@ -31,8 +33,33 @@
   void setOption(OptionValue value);
 
   /**
+   * Deletes the option. Unfortunately, the type is required given the 
fallback structure of option managers.
--- End diff --

why _unfortunately_ ?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901018#comment-14901018
 ] 

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_r39997752
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
 ---
@@ -241,6 +243,30 @@ public void setOption(final OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, OptionType type) {
+assert type == OptionType.SYSTEM;
+try { // ensure option exists
+  getValidator(name);
+} catch (final IllegalArgumentException e) {
+  throw UserException.validationError(e)
+.build(logger);
+}
+options.delete(name.toLowerCase());
+  }
+
+  @Override
+  public void deleteAllOptions(OptionType type) {
+assert type == OptionType.SYSTEM;
--- End diff --

same here


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901029#comment-14901029
 ] 

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_r39998405
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java ---
@@ -46,19 +48,207 @@ public void testOptions() throws Exception{
   @Test
   public void checkValidationException() throws Exception {
 thrownException.expect(new UserExceptionMatcher(VALIDATION));
-test(String.format("ALTER session SET `%s` = '%s';", 
ExecConstants.SLICE_TARGET, "fail"));
+test("ALTER session SET `%s` = '%s';", SLICE_TARGET, "fail");
   }
 
   @Test // DRILL-3122
   public void checkChangedColumn() throws Exception {
-test(String.format("ALTER session SET `%s` = %d;", 
ExecConstants.SLICE_TARGET,
+test(String.format("ALTER session SET `%s` = %d;", SLICE_TARGET,
   ExecConstants.SLICE_TARGET_DEFAULT));
 testBuilder()
-.sqlQuery(String.format("SELECT status FROM sys.options WHERE name 
= '%s' AND type = 'SESSION'", ExecConstants.SLICE_TARGET))
+.sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
 .unOrdered()
 .baselineColumns("status")
 .baselineValues("DEFAULT")
 .build()
 .run();
   }
+
+  @Test
+  public void setAndResetSessionOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+
+// change option
+test("SET `%s` = %d;", SLICE_TARGET, 10);
+// check changed
+test("SELECT status, type, name FROM sys.options WHERE type = 
'SESSION';");
+testBuilder()
+  .sqlQuery("SELECT num_val FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .baselineColumns("num_val")
+  .baselineValues((long) 10)
+  .build()
+  .run();
+
+// reset option
+test("RESET `%s`;", SLICE_TARGET);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+  }
+
+  @Test
+  public void setAndResetSystemOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("status")
+  .baselineValues("DEFAULT")
+  .build()
+  .run();
+
+// change option
+test("ALTER system SET `%s` = %b;", ENABLE_VERBOSE_ERRORS_KEY, true);
+// check changed
+testBuilder()
+  .sqlQuery("SELECT bool_val FROM sys.options WHERE name = '%s' AND 
type = 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("bool_val")
+  .baselineValues(true)
+  .build()
+  .run();
+
+// reset option
+test("ALTER system RESET `%s`;", ENABLE_VERBOSE_ERRORS_KEY);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("status")
+  .baselineValues("DEFAULT")
+  .build()
+  .run();
+  }
+
+  @Test
+  public void resetAllSessionOptions() throws Exception {
--- End diff --

also change _SESSION_ and _SYSTEM_ options then reset all _SESSION_ options


> 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  
> 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

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14900994#comment-14900994
 ] 

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_r39996432
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/CompoundIdentifierConverter.java
 ---
@@ -161,6 +171,7 @@ public SqlNode visitChild(
 rules.put(SqlJoin.class, R(D, D, D, D, D, E));
 rules.put(SqlOrderBy.class, R(D, E, D, D));
 rules.put(SqlDropTable.class, R(D));
+rules.put(SqlSetOption.class, R(D, D, D));
--- End diff --

can you add a comment explaining why this rule is needed ?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901005#comment-14901005
 ] 

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_r39997032
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
 ---
@@ -83,14 +84,30 @@ public OptionValue getOption(final String name) {
*/
   abstract boolean setLocalOption(OptionValue value);
 
+  /**
+   * Deletes the option with given name for this manager without falling 
back.
+   *
+   * @param type option type
+   * @return true iff the option was successfully deleted
+   */
+  abstract boolean deleteLocalOptions(OptionType type);
--- End diff --

can you rename this to `deleteAllLocalOptions` ?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901016#comment-14901016
 ] 

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_r39997731
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
 ---
@@ -241,6 +243,30 @@ public void setOption(final OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, OptionType type) {
+assert type == OptionType.SYSTEM;
--- End diff --

can you please add an error message to the assertion ?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901014#comment-14901014
 ] 

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_r39997680
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -20,14 +20,21 @@
 import com.google.common.base.Predicate;
 import com.google.common.collect.Collections2;
 import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.common.map.CaseInsensitiveMap;
 import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.server.options.OptionValue.OptionType;
 
 import java.util.Collection;
 import java.util.Map;
 
 /**
- * {@link OptionManager} that holds options within {@link 
org.apache.drill.exec.rpc.user.UserSession} context.
+ * {@link OptionManager} that holds options within {@link 
org.apache.drill.exec.rpc.user.UserSession} context. Options
+ * set at the session level only apply to queries that you run during the 
current Drill connection. Session level
+ * settings override system level settings.
+ *
+ * NOTE that currently, the effects of deleting a short lived option (see 
{@link OptionValidator#isShortLived}) are
+ * undefined.
--- End diff --

can you please elaborate on the _undefined_ effects of deleting short lived 
options ?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901027#comment-14901027
 ] 

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_r39998259
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java ---
@@ -46,19 +48,207 @@ public void testOptions() throws Exception{
   @Test
   public void checkValidationException() throws Exception {
 thrownException.expect(new UserExceptionMatcher(VALIDATION));
-test(String.format("ALTER session SET `%s` = '%s';", 
ExecConstants.SLICE_TARGET, "fail"));
+test("ALTER session SET `%s` = '%s';", SLICE_TARGET, "fail");
   }
 
   @Test // DRILL-3122
   public void checkChangedColumn() throws Exception {
-test(String.format("ALTER session SET `%s` = %d;", 
ExecConstants.SLICE_TARGET,
+test(String.format("ALTER session SET `%s` = %d;", SLICE_TARGET,
   ExecConstants.SLICE_TARGET_DEFAULT));
 testBuilder()
-.sqlQuery(String.format("SELECT status FROM sys.options WHERE name 
= '%s' AND type = 'SESSION'", ExecConstants.SLICE_TARGET))
+.sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
 .unOrdered()
 .baselineColumns("status")
 .baselineValues("DEFAULT")
 .build()
 .run();
   }
+
+  @Test
+  public void setAndResetSessionOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+
+// change option
+test("SET `%s` = %d;", SLICE_TARGET, 10);
+// check changed
+test("SELECT status, type, name FROM sys.options WHERE type = 
'SESSION';");
+testBuilder()
+  .sqlQuery("SELECT num_val FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .baselineColumns("num_val")
+  .baselineValues((long) 10)
+  .build()
+  .run();
+
+// reset option
+test("RESET `%s`;", SLICE_TARGET);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+  }
+
+  @Test
+  public void setAndResetSystemOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("status")
+  .baselineValues("DEFAULT")
+  .build()
+  .run();
+
+// change option
+test("ALTER system SET `%s` = %b;", ENABLE_VERBOSE_ERRORS_KEY, true);
+// check changed
+testBuilder()
+  .sqlQuery("SELECT bool_val FROM sys.options WHERE name = '%s' AND 
type = 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("bool_val")
+  .baselineValues(true)
+  .build()
+  .run();
+
+// reset option
+test("ALTER system RESET `%s`;", ENABLE_VERBOSE_ERRORS_KEY);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("status")
+  .baselineValues("DEFAULT")
+  .build()
+  .run();
+  }
+
+  @Test
+  public void resetAllSessionOptions() throws Exception {
--- End diff --

resetting all _SYSTEM_ options is not covered here


> 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  
> 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.



--

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901024#comment-14901024
 ] 

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_r39998097
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControls.java
 ---
@@ -80,7 +80,7 @@
  * @param ttl  the number of queries for which this option should be 
valid
  */
 public ControlsOptionValidator(final String name, final String def, 
final int ttl) {
-  super(name, OptionValue.Kind.STRING, 
OptionValue.createString(OptionType.SESSION, name, def));
+  super(name, OptionValue.Kind.STRING, 
OptionValue.createString(OptionType.SYSTEM, name, def));
--- End diff --

can you add a comment explaining why this needs to be a _SYSTEM_ rather 
than a _SESSION_ option ? someone else might revert this change inadvertently.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901025#comment-14901025
 ] 

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_r39998161
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java ---
@@ -46,19 +48,207 @@ public void testOptions() throws Exception{
   @Test
   public void checkValidationException() throws Exception {
 thrownException.expect(new UserExceptionMatcher(VALIDATION));
-test(String.format("ALTER session SET `%s` = '%s';", 
ExecConstants.SLICE_TARGET, "fail"));
+test("ALTER session SET `%s` = '%s';", SLICE_TARGET, "fail");
   }
 
   @Test // DRILL-3122
   public void checkChangedColumn() throws Exception {
-test(String.format("ALTER session SET `%s` = %d;", 
ExecConstants.SLICE_TARGET,
+test(String.format("ALTER session SET `%s` = %d;", SLICE_TARGET,
--- End diff --

you can remove `String.format` here too


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901366#comment-14901366
 ] 

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_r40023540
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -20,14 +20,21 @@
 import com.google.common.base.Predicate;
 import com.google.common.collect.Collections2;
 import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.common.map.CaseInsensitiveMap;
 import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.server.options.OptionValue.OptionType;
 
 import java.util.Collection;
 import java.util.Map;
 
 /**
- * {@link OptionManager} that holds options within {@link 
org.apache.drill.exec.rpc.user.UserSession} context.
+ * {@link OptionManager} that holds options within {@link 
org.apache.drill.exec.rpc.user.UserSession} context. Options
+ * set at the session level only apply to queries that you run during the 
current Drill connection. Session level
+ * settings override system level settings.
+ *
+ * NOTE that currently, the effects of deleting a short lived option (see 
{@link OptionValidator#isShortLived}) are
+ * undefined.
--- End diff --

No, I meant could you explain what you mean by _undefined_ effects. I think 
you mean if we set a short lived session, for example we inject an exception, 
then try to delete it, depending on where the exception was injected the reset 
query could either succeed or the exception could actually be thrown in the 
reset query itself.
Just improve the comment to explain what you mean


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901321#comment-14901321
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40020795
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java ---
@@ -46,19 +48,207 @@ public void testOptions() throws Exception{
   @Test
   public void checkValidationException() throws Exception {
 thrownException.expect(new UserExceptionMatcher(VALIDATION));
-test(String.format("ALTER session SET `%s` = '%s';", 
ExecConstants.SLICE_TARGET, "fail"));
+test("ALTER session SET `%s` = '%s';", SLICE_TARGET, "fail");
   }
 
   @Test // DRILL-3122
   public void checkChangedColumn() throws Exception {
-test(String.format("ALTER session SET `%s` = %d;", 
ExecConstants.SLICE_TARGET,
+test(String.format("ALTER session SET `%s` = %d;", SLICE_TARGET,
   ExecConstants.SLICE_TARGET_DEFAULT));
 testBuilder()
-.sqlQuery(String.format("SELECT status FROM sys.options WHERE name 
= '%s' AND type = 'SESSION'", ExecConstants.SLICE_TARGET))
+.sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
 .unOrdered()
 .baselineColumns("status")
 .baselineValues("DEFAULT")
 .build()
 .run();
   }
+
+  @Test
+  public void setAndResetSessionOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+
+// change option
+test("SET `%s` = %d;", SLICE_TARGET, 10);
+// check changed
+test("SELECT status, type, name FROM sys.options WHERE type = 
'SESSION';");
+testBuilder()
+  .sqlQuery("SELECT num_val FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .baselineColumns("num_val")
+  .baselineValues((long) 10)
+  .build()
+  .run();
+
+// reset option
+test("RESET `%s`;", SLICE_TARGET);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+  }
+
+  @Test
+  public void setAndResetSystemOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("status")
+  .baselineValues("DEFAULT")
+  .build()
+  .run();
+
+// change option
+test("ALTER system SET `%s` = %b;", ENABLE_VERBOSE_ERRORS_KEY, true);
+// check changed
+testBuilder()
+  .sqlQuery("SELECT bool_val FROM sys.options WHERE name = '%s' AND 
type = 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("bool_val")
+  .baselineValues(true)
+  .build()
+  .run();
+
+// reset option
+test("ALTER system RESET `%s`;", ENABLE_VERBOSE_ERRORS_KEY);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("status")
+  .baselineValues("DEFAULT")
+  .build()
+  .run();
+  }
+
+  @Test
+  public void resetAllSessionOptions() throws Exception {
--- End diff --

changeSessionAndNotSystem is testing this.

Also, we cannot test resetting all system options because the unit tests 
have a "before" method that changes a system option.


> 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  
> If we don't want to

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901325#comment-14901325
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40020971
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControls.java
 ---
@@ -80,7 +80,7 @@
  * @param ttl  the number of queries for which this option should be 
valid
  */
 public ControlsOptionValidator(final String name, final String def, 
final int ttl) {
-  super(name, OptionValue.Kind.STRING, 
OptionValue.createString(OptionType.SESSION, name, def));
+  super(name, OptionValue.Kind.STRING, 
OptionValue.createString(OptionType.SYSTEM, name, def));
--- End diff --

I don't think this requires a comment in code, since this applies for all 
option validators.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901339#comment-14901339
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40021824
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
 ---
@@ -241,6 +243,30 @@ public void setOption(final OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, OptionType type) {
+assert type == OptionType.SYSTEM;
+try { // ensure option exists
--- End diff --

I left this as is because the callers could add context and handle the 
exception differently.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901343#comment-14901343
 ] 

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_r40021955
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java ---
@@ -46,19 +48,207 @@ public void testOptions() throws Exception{
   @Test
   public void checkValidationException() throws Exception {
 thrownException.expect(new UserExceptionMatcher(VALIDATION));
-test(String.format("ALTER session SET `%s` = '%s';", 
ExecConstants.SLICE_TARGET, "fail"));
+test("ALTER session SET `%s` = '%s';", SLICE_TARGET, "fail");
   }
 
   @Test // DRILL-3122
   public void checkChangedColumn() throws Exception {
-test(String.format("ALTER session SET `%s` = %d;", 
ExecConstants.SLICE_TARGET,
+test(String.format("ALTER session SET `%s` = %d;", SLICE_TARGET,
   ExecConstants.SLICE_TARGET_DEFAULT));
 testBuilder()
-.sqlQuery(String.format("SELECT status FROM sys.options WHERE name 
= '%s' AND type = 'SESSION'", ExecConstants.SLICE_TARGET))
+.sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
 .unOrdered()
 .baselineColumns("status")
 .baselineValues("DEFAULT")
 .build()
 .run();
   }
+
+  @Test
+  public void setAndResetSessionOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+
+// change option
+test("SET `%s` = %d;", SLICE_TARGET, 10);
+// check changed
+test("SELECT status, type, name FROM sys.options WHERE type = 
'SESSION';");
+testBuilder()
+  .sqlQuery("SELECT num_val FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .baselineColumns("num_val")
+  .baselineValues((long) 10)
+  .build()
+  .run();
+
+// reset option
+test("RESET `%s`;", SLICE_TARGET);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+  }
+
+  @Test
+  public void setAndResetSystemOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("status")
+  .baselineValues("DEFAULT")
+  .build()
+  .run();
+
+// change option
+test("ALTER system SET `%s` = %b;", ENABLE_VERBOSE_ERRORS_KEY, true);
+// check changed
+testBuilder()
+  .sqlQuery("SELECT bool_val FROM sys.options WHERE name = '%s' AND 
type = 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("bool_val")
+  .baselineValues(true)
+  .build()
+  .run();
+
+// reset option
+test("ALTER system RESET `%s`;", ENABLE_VERBOSE_ERRORS_KEY);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SYSTEM'", ENABLE_VERBOSE_ERRORS_KEY)
+  .unOrdered()
+  .baselineColumns("status")
+  .baselineValues("DEFAULT")
+  .build()
+  .run();
+  }
+
+  @Test
+  public void resetAllSessionOptions() throws Exception {
--- End diff --

my bad, I meant: we change the same option at the _SESSION_ and _SYSTEM_ 
level then we reset it at the _SESSION_ level and we make sure it's still 
changed at the _SYSTEM_ level.
`changeSessionAndNotSystem` kind of does that but with a `RESET ALL` and we 
need to make sure `ALTER SESSION RESET A` also works fine


> 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

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901349#comment-14901349
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40022217
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -20,14 +20,21 @@
 import com.google.common.base.Predicate;
 import com.google.common.collect.Collections2;
 import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.common.map.CaseInsensitiveMap;
 import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.server.options.OptionValue.OptionType;
 
 import java.util.Collection;
 import java.util.Map;
 
 /**
- * {@link OptionManager} that holds options within {@link 
org.apache.drill.exec.rpc.user.UserSession} context.
+ * {@link OptionManager} that holds options within {@link 
org.apache.drill.exec.rpc.user.UserSession} context. Options
+ * set at the session level only apply to queries that you run during the 
current Drill connection. Session level
+ * settings override system level settings.
+ *
+ * NOTE that currently, the effects of deleting a short lived option (see 
{@link OptionValidator#isShortLived}) are
+ * undefined.
--- End diff --

Should I add a TODO and create a ticket?
(personally I don't think this is worth a JIRA yet)


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901354#comment-14901354
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40022907
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java
 ---
@@ -31,8 +33,33 @@
   void setOption(OptionValue value);
 
   /**
+   * Deletes the option. Unfortunately, the type is required given the 
fallback structure of option managers.
--- End diff --

*Unfortunately* because we assume *fallback structure of option managers*


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901356#comment-14901356
 ] 

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_r40022994
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControls.java
 ---
@@ -80,7 +80,7 @@
  * @param ttl  the number of queries for which this option should be 
valid
  */
 public ControlsOptionValidator(final String name, final String def, 
final int ttl) {
-  super(name, OptionValue.Kind.STRING, 
OptionValue.createString(OptionType.SESSION, name, def));
+  super(name, OptionValue.Kind.STRING, 
OptionValue.createString(OptionType.SYSTEM, name, def));
--- End diff --

Ok, I see, `TypeValidator` always expects _SYSTEM_ OptionValues
Should we add an assertion to TypeValidator to make sure this doesn't 
happen again ? 


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901249#comment-14901249
 ] 

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_r40014413
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/CompoundIdentifierConverter.java
 ---
@@ -161,6 +171,7 @@ public SqlNode visitChild(
 rules.put(SqlJoin.class, R(D, D, D, D, D, E));
 rules.put(SqlOrderBy.class, R(D, E, D, D));
 rules.put(SqlDropTable.class, R(D));
+rules.put(SqlSetOption.class, R(D, D, D));
--- End diff --

the comment doesn't explain why this specific rule has been added. It just 
explains how the rules work in general.
Why do we need this specific rule, what happens if I remove it ?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901255#comment-14901255
 ] 

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_r40014502
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java ---
@@ -46,19 +48,124 @@ public void testOptions() throws Exception{
   @Test
   public void checkValidationException() throws Exception {
 thrownException.expect(new UserExceptionMatcher(VALIDATION));
-test(String.format("ALTER session SET `%s` = '%s';", 
ExecConstants.SLICE_TARGET, "fail"));
+test("ALTER session SET `%s` = '%s';", SLICE_TARGET, "fail");
   }
 
   @Test // DRILL-3122
   public void checkChangedColumn() throws Exception {
-test(String.format("ALTER session SET `%s` = %d;", 
ExecConstants.SLICE_TARGET,
+test(String.format("ALTER session SET `%s` = %d;", SLICE_TARGET,
   ExecConstants.SLICE_TARGET_DEFAULT));
 testBuilder()
-.sqlQuery(String.format("SELECT status FROM sys.options WHERE name 
= '%s' AND type = 'SESSION'", ExecConstants.SLICE_TARGET))
+.sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
 .unOrdered()
 .baselineColumns("status")
 .baselineValues("DEFAULT")
 .build()
 .run();
   }
+
+  @Test
+  public void setAndResetSessionOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+
+// change option
+test("SET `%s` = %d;", SLICE_TARGET, 10);
+// check changed
+test("SELECT status, type, name FROM sys.options WHERE type = 
'SESSION';");
+testBuilder()
+  .sqlQuery("SELECT num_val FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .baselineColumns("num_val")
+  .baselineValues((long) 10)
+  .build()
+  .run();
+
+// reset option
+test("RESET `%s`;", SLICE_TARGET);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+  }
+
+  @Test
+  public void setAndResetSystemOption() throws Exception {
--- End diff --

thanks for adding those unit tests


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901185#comment-14901185
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40010220
  
--- 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 --

This is not a change I introduced (just difference in spacing), but I'll 
address this issue.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901214#comment-14901214
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40012463
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java ---
@@ -46,19 +48,124 @@ public void testOptions() throws Exception{
   @Test
   public void checkValidationException() throws Exception {
 thrownException.expect(new UserExceptionMatcher(VALIDATION));
-test(String.format("ALTER session SET `%s` = '%s';", 
ExecConstants.SLICE_TARGET, "fail"));
+test("ALTER session SET `%s` = '%s';", SLICE_TARGET, "fail");
   }
 
   @Test // DRILL-3122
   public void checkChangedColumn() throws Exception {
-test(String.format("ALTER session SET `%s` = %d;", 
ExecConstants.SLICE_TARGET,
+test(String.format("ALTER session SET `%s` = %d;", SLICE_TARGET,
   ExecConstants.SLICE_TARGET_DEFAULT));
 testBuilder()
-.sqlQuery(String.format("SELECT status FROM sys.options WHERE name 
= '%s' AND type = 'SESSION'", ExecConstants.SLICE_TARGET))
+.sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
 .unOrdered()
 .baselineColumns("status")
 .baselineValues("DEFAULT")
 .build()
 .run();
   }
+
+  @Test
+  public void setAndResetSessionOption() throws Exception {
+// check unchanged
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+
+// change option
+test("SET `%s` = %d;", SLICE_TARGET, 10);
+// check changed
+test("SELECT status, type, name FROM sys.options WHERE type = 
'SESSION';");
+testBuilder()
+  .sqlQuery("SELECT num_val FROM sys.options WHERE name = '%s' AND 
type = 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .baselineColumns("num_val")
+  .baselineValues((long) 10)
+  .build()
+  .run();
+
+// reset option
+test("RESET `%s`;", SLICE_TARGET);
+// check reverted
+testBuilder()
+  .sqlQuery("SELECT status FROM sys.options WHERE name = '%s' AND type 
= 'SESSION'", SLICE_TARGET)
+  .unOrdered()
+  .expectsEmptyResultSet()
+  .build()
+  .run();
+  }
+
+  @Test
+  public void setAndResetSystemOption() throws Exception {
--- End diff --

changeSessionAndNotSystem and changeSystemAndNotSession are testing this.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901189#comment-14901189
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40010391
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/CompoundIdentifierConverter.java
 ---
@@ -161,6 +171,7 @@ public SqlNode visitChild(
 rules.put(SqlJoin.class, R(D, D, D, D, D, E));
 rules.put(SqlOrderBy.class, R(D, E, D, D));
 rules.put(SqlDropTable.class, R(D));
+rules.put(SqlSetOption.class, R(D, D, D));
--- End diff --

There is already a comment explaining the rules, and why they are needed. 
What more?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901431#comment-14901431
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40027080
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
 ---
@@ -102,6 +119,29 @@ public void setOption(OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, final OptionType type) {
+try {
+  SystemOptionManager.getValidator(name); // ensure the option exists
+} catch (final IllegalArgumentException e) {
+  throw UserException.validationError(e)
+.build(logger);
+}
+
+// fallback if unable to delete locally
+if (!deleteLocalOption(name, type)) {
--- End diff --

Implementations of OptionManager are required to be case insensitive to 
option names (mentioned in the OptionManager class docs). Calling 
```name.toLowerCase()``` in this one place is insufficient to ensure 
*implementation will always be case insensitive.*


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901413#comment-14901413
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r40026355
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/CompoundIdentifierConverter.java
 ---
@@ -161,6 +171,7 @@ public SqlNode visitChild(
 rules.put(SqlJoin.class, R(D, D, D, D, D, E));
 rules.put(SqlOrderBy.class, R(D, E, D, D));
 rules.put(SqlDropTable.class, R(D));
+rules.put(SqlSetOption.class, R(D, D, D));
--- End diff --

I see the confusion. We now allow option names to be compound identifiers. 
Let me add this to the JIRA and the commit message.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14803366#comment-14803366
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-141176948
  
I worked around the fallback structure of option managers, which is wrong. 
I will update the pull request soon.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804471#comment-14804471
 ] 

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_r39796601
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/InMemoryOptionManager.java
 ---
@@ -35,6 +37,20 @@
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+throw UserException.unsupportedError()
+  .message("This manager does not support deleting an option.")
+  .build(logger);
+  }
+
+  @Override
+  public void deleteAllOptions(final OptionValue.OptionType type) {
+throw UserException.unsupportedError()
--- End diff --

same here


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804469#comment-14804469
 ] 

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_r39796579
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/InMemoryOptionManager.java
 ---
@@ -35,6 +37,20 @@
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+throw UserException.unsupportedError()
--- End diff --

Why throw a UserException here ? is the error message relevant to the user 
? do we even expect this method to be called ? 


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804481#comment-14804481
 ] 

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_r39797379
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -46,6 +47,33 @@ public SessionOptionManager(final OptionManager 
systemOptions, final UserSession
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+if (type == OptionValue.OptionType.SESSION) {
+  try { // ensure option exists
+SystemOptionManager.getValidator(name);
+  } catch (final IllegalArgumentException e) {
+throw UserException.validationError()
--- End diff --

you can just use:
```
throw UserException.validationError(e).build(logger);
```


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804513#comment-14804513
 ] 

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_r39799140
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
 ---
@@ -241,6 +243,31 @@ public void setOption(final OptionValue value) {
   }
 
   @Override
+  public void deleteOption(final String name, OptionType type) {
+assert type == OptionType.SYSTEM;
+try { // ensure option exists
+  getValidator(name);
+} catch (final IllegalArgumentException e) {
+  throw UserException.validationError()
--- End diff --

you can just use:
```
throw UserException.validationError(e).build(logger);
```


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804626#comment-14804626
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on the pull request:

https://github.com/apache/drill/pull/159#issuecomment-141251928
  
I have added a note to the SessionOptionManager that *the effects of 
deleting a short lived option are undefined* because that might require 
slightly extensive changes, which is out of scope for this JIRA.

@jacques-n please review the changes in *CompoundIdentifierConverter* class.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread Sudheesh Katkam (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804598#comment-14804598
 ] 

Sudheesh Katkam commented on DRILL-1065:


Also, adding:
SET `option name`
RESET `option name`
RESET ALL

as discussed 
[here|http://mail-archives.apache.org/mod_mbox/calcite-dev/201507.mbox/%3ccampyv7c9ynatuk-2ufh_suddmc6d5-e4jd1_taqmnuoifz1...@mail.gmail.com%3E]
 with an assumption that the scope is SESSION.

> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804505#comment-14804505
 ] 

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_r39798736
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -46,6 +47,33 @@ public SessionOptionManager(final OptionManager 
systemOptions, final UserSession
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+if (type == OptionValue.OptionType.SESSION) {
+  try { // ensure option exists
+SystemOptionManager.getValidator(name);
+  } catch (final IllegalArgumentException e) {
+throw UserException.validationError()
+  .message(e.getMessage())
+  .build(logger);
+  }
+  options.remove(name);
+  shortLivedOptions.remove(name);
+} else {
+  fallback.deleteOption(name, type);
--- End diff --

so if I change an option both at the _session_ and _system_ levels, then I 
_reset_ the same option at the _system_ level it will leave it changed at the 
_session_ level. Is this expected ?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804504#comment-14804504
 ] 

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_r39798636
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -46,6 +47,33 @@ public SessionOptionManager(final OptionManager 
systemOptions, final UserSession
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+if (type == OptionValue.OptionType.SESSION) {
+  try { // ensure option exists
--- End diff --

You only ensure the option exists when `type == SESSION`. How about 
_SYSTEM_, is it done by the underlying _fallback_ OptionManager ?


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804587#comment-14804587
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r39803498
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -46,6 +47,33 @@ public SessionOptionManager(final OptionManager 
systemOptions, final UserSession
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+if (type == OptionValue.OptionType.SESSION) {
+  try { // ensure option exists
+SystemOptionManager.getValidator(name);
+  } catch (final IllegalArgumentException e) {
+throw UserException.validationError()
+  .message(e.getMessage())
+  .build(logger);
+  }
+  options.remove(name);
+  shortLivedOptions.remove(name);
+} else {
+  fallback.deleteOption(name, type);
--- End diff --

I'd say yes. Session options always override system options (default or 
not).


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804583#comment-14804583
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r39803386
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -46,6 +47,33 @@ public SessionOptionManager(final OptionManager 
systemOptions, final UserSession
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+if (type == OptionValue.OptionType.SESSION) {
+  try { // ensure option exists
--- End diff --

Yes, the fallback for a SessionOptionManager is a SystemOptionManager.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804511#comment-14804511
 ] 

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_r39799014
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java
 ---
@@ -46,6 +47,33 @@ public SessionOptionManager(final OptionManager 
systemOptions, final UserSession
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+if (type == OptionValue.OptionType.SESSION) {
+  try { // ensure option exists
+SystemOptionManager.getValidator(name);
+  } catch (final IllegalArgumentException e) {
+throw UserException.validationError()
+  .message(e.getMessage())
+  .build(logger);
+  }
+  options.remove(name);
+  shortLivedOptions.remove(name);
+} else {
+  fallback.deleteOption(name, type);
+}
+  }
+
+  @Override
+  public void deleteAllOptions(final OptionValue.OptionType type) {
+if (type == OptionValue.OptionType.SESSION) {
+  options.clear();
+  shortLivedOptions.clear();
+} else {
+  fallback.deleteAllOptions(type);
--- End diff --

same here


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-17 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14804580#comment-14804580
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r39803317
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/InMemoryOptionManager.java
 ---
@@ -35,6 +37,20 @@
   }
 
   @Override
+  public void deleteOption(final String name, final OptionValue.OptionType 
type) {
+throw UserException.unsupportedError()
--- End diff --

Removing this in my new implementation.


> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-16 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14790548#comment-14790548
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/159#discussion_r39644014
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java
 ---
@@ -49,45 +47,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,
+
context.getOptions().getOption(ExecConstants.ADMIN_USER_GROUPS_KEY).string_val))
 {
+throw UserException.permissionError()
+.message("Not authorized to change SYSTEM options.")
+.build(logger);
   }
+}
+
+// Currently, we use one part identifiers.
+final SqlIdentifier nameIdentifier = option.getName();
+if (!nameIdentifier.isSimple()) {
+  throw UserException.validationError()
+.message("Drill does not support multi-part identifier for an 
option name (%s).",
+  nameIdentifier.toString())
+.build(logger);
+}
 
+final String name = nameIdentifier.getSimple();
+if (value != null) { // SET option
   final OptionValue optionValue = createOptionValue(name, type, 
(SqlLiteral) value);
   context.getOptions().setOption(optionValue);
-}else{
-  throw new ValidationException("Sql options can only be literals.");
+} else { // RESET option
+  if ("ALL".equals(name)) {
--- End diff --

.equalsIgnoreCase(...)


> Provide a reset command to reset an option to its default value
> ---
>
>

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-09-15 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14746489#comment-14746489
 ] 

ASF GitHub Bot commented on DRILL-1065:
---

GitHub user sudheeshkatkam opened a pull request:

https://github.com/apache/drill/pull/159

DRILL-1065: Support for ALTER ... RESET statement

+ Support for "SET option = value" statement (assumes scope as SESSION)
+ Better error messages in SetOptionHandler
+ Changes in CompoundIdentifierConverter
  - update when rewritten operand is not deeply equal to the original 
operand
  - Added Override annotations
+ Default ExecutionControls option value should be at SYSTEM level

@jacques-n @vkorukanti please review.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sudheeshkatkam/drill DRILL-1065

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/159.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #159


commit f785c3ba4d50bfe5a7ff77a8d37b3589bbe1c1cf
Author: Sudheesh Katkam 
Date:   2015-09-15T23:13:23Z

DRILL-1065: Support for ALTER ... RESET statement

+ Support for "SET option = value" statement (assumes scope as SESSION)
+ Better error messages in SetOptionHandler
+ Changes in CompoundIdentifierConverter
  - update when rewritten operand is not deeply equal to the original 
operand
  - Added Override annotations
+ Default ExecutionControls option value should be at SYSTEM level




> 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  
> 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)

[jira] [Commented] (DRILL-1065) Provide a reset command to reset an option to its default value

2015-05-04 Thread Jacques Nadeau (JIRA)


[ 
https://issues.apache.org/jira/browse/DRILL-1065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14526872#comment-14526872
 ] 

Jacques Nadeau commented on DRILL-1065:
---

Propose syntax: 

ALTER SESSION RESET `option name`
ALTER SYSTEM RESET `option name`

ALTER SESSION RESET ALL
ALTER SYSTEM RESET ALL

 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
Reporter: Aman Sinha
Assignee: Sudheesh Katkam
Priority: Minor
 Fix For: 1.0.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)

71 matches

Mail list logo