This is an automated email from the ASF dual-hosted git repository. volodymyr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit 6d98c127548ef13ab0c2ec61cc59299ee00a2232 Author: Volodymyr Vysotskyi <[email protected]> AuthorDate: Sat Mar 28 00:49:50 2020 +0200 DRILL-7673: View set query fails with NPE for non-existing option closes #2043 --- .../planner/sql/handlers/SetOptionHandler.java | 6 ++++- .../planner/sql/handlers/SetOptionHandlerTest.java | 27 +++++++++++++++++----- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java index 41a1b78..d041444 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java @@ -26,6 +26,7 @@ import org.apache.drill.exec.ops.QueryContext; import org.apache.drill.exec.physical.PhysicalPlan; import org.apache.drill.exec.planner.sql.DirectPlan; import org.apache.drill.exec.planner.sql.parser.DrillSqlSetOption; +import org.apache.drill.exec.server.options.OptionDefinition; import org.apache.drill.exec.server.options.OptionManager; import org.apache.drill.exec.server.options.OptionValue; import org.apache.drill.exec.server.options.OptionValue.OptionScope; @@ -63,9 +64,12 @@ public class SetOptionHandler extends AbstractSqlSetHandler { SqlNode optionValue = statement.getValue(); if (optionValue == null) { + // OptionManager.getOptionDefinition() call ensures that the specified option name is valid + OptionDefinition optionDefinition = optionManager.getOptionDefinition(optionName); String value = String.valueOf(optionManager.getOption(optionName).getValue()); - return DirectPlan.createDirectPlan(context, new SetOptionViewResult(optionName, value)); + // obtains option name from OptionDefinition to use the name as defined in the option, rather than what the user provided + return DirectPlan.createDirectPlan(context, new SetOptionViewResult(optionDefinition.getValidator().getOptionName(), value)); } else { if (optionScope == OptionValue.OptionScope.SYSTEM) { checkAdminPrivileges(context.getOptions()); diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandlerTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandlerTest.java index def1240..3f2854d 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandlerTest.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandlerTest.java @@ -18,6 +18,7 @@ package org.apache.drill.exec.planner.sql.handlers; import org.apache.drill.categories.SqlTest; +import org.apache.drill.common.exceptions.UserRemoteException; import org.apache.drill.exec.ExecConstants; import org.apache.drill.exec.compile.ClassCompilerSelector; import org.apache.drill.test.ClusterFixture; @@ -27,6 +28,10 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; +import static org.hamcrest.CoreMatchers.startsWith; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + @Category(SqlTest.class) public class SetOptionHandlerTest extends ClusterTest { @@ -42,11 +47,11 @@ public class SetOptionHandlerTest extends ClusterTest { ClassCompilerSelector.JAVA_COMPILER_DEBUG_OPTION) .singletonString(); - boolean newValue = !Boolean.valueOf(defaultValue); + boolean newValue = !Boolean.parseBoolean(defaultValue); try { client.alterSession(ClassCompilerSelector.JAVA_COMPILER_DEBUG_OPTION, newValue); - String changedValue = client.queryBuilder() + String changedValue = queryBuilder() .sql("SELECT val from sys.options where name = '%s' limit 1", ClassCompilerSelector.JAVA_COMPILER_DEBUG_OPTION) .singletonString(); @@ -59,32 +64,42 @@ public class SetOptionHandlerTest extends ClusterTest { @Test public void testViewSetQuery() throws Exception { - client.testBuilder() // BIT + testBuilder() // BIT .sqlQuery("SET `%s`", ExecConstants.ENABLE_ITERATOR_VALIDATION_OPTION) .unOrdered() .sqlBaselineQuery("SELECT name, val as value FROM sys.options where name = '%s' limit 1", ExecConstants.ENABLE_ITERATOR_VALIDATION_OPTION) .go(); - client.testBuilder() // BIGINT + testBuilder() // BIGINT .sqlQuery("SET `%s`", ExecConstants.OUTPUT_BATCH_SIZE) .unOrdered() .sqlBaselineQuery("SELECT name, val as value FROM sys.options where name = '%s' limit 1", ExecConstants.OUTPUT_BATCH_SIZE) .go(); - client.testBuilder() // FLOAT + testBuilder() // FLOAT .sqlQuery("SET `%s`", ExecConstants.OUTPUT_BATCH_SIZE_AVAIL_MEM_FACTOR) .unOrdered() .sqlBaselineQuery("SELECT name, val as value FROM sys.options where name = '%s' limit 1", ExecConstants.OUTPUT_BATCH_SIZE_AVAIL_MEM_FACTOR) .go(); - client.testBuilder() // VARCHAR + testBuilder() // VARCHAR .sqlQuery("SET `%s`", ExecConstants.FILESYSTEM_PARTITION_COLUMN_LABEL) .unOrdered() .sqlBaselineQuery("SELECT name, val as value FROM sys.options where name = '%s' limit 1", ExecConstants.FILESYSTEM_PARTITION_COLUMN_LABEL) .go(); } + + @Test + public void testViewSetWithIncorrectOption() throws Exception { + try { + run("set `non-existing option`"); + fail(); + } catch (UserRemoteException e) { + assertThat(e.getMessage(), startsWith("VALIDATION ERROR")); + } + } }
