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


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

Reply via email to