This is an automated email from the ASF dual-hosted git repository. vitalii pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit 44f17eaf1b9489fc16fadee0178e975f6aff5185 Author: Arina Ielchiieva <[email protected]> AuthorDate: Sun Dec 23 17:10:39 2018 +0200 DRILL-6922: Do not set return result set option on query level if it is the same as current value 1. Rename return result option name to `exec.query.return_result_set_for_ddl`. 2. Add check if option value is the same as current value in DrillSqlWorker before setting result set option on query level. 3. Separate Session and Query options on Web UI. closes #1584 --- .../java/org/apache/drill/exec/ExecConstants.java | 9 ++-- .../drill/exec/planner/sql/DrillSqlWorker.java | 12 +++-- .../drill/exec/server/options/OptionValue.java | 7 ++- .../exec/server/rest/profile/ProfileWrapper.java | 33 ++++++++++--- .../java-exec/src/main/resources/drill-module.conf | 2 +- .../src/main/resources/rest/profile/profile.ftl | 57 +++++++++++++--------- 6 files changed, 78 insertions(+), 42 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java index c4d7652..afa29d9 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java @@ -854,7 +854,7 @@ public final class ExecConstants { * the shutdown request is triggered. The primary use of grace period is to * avoid the race conditions caused by zookeeper delay in updating the state * information of the drillbit that is shutting down. So, it is advisable - * to have a grace period that is atleast twice the amount of zookeeper + * to have a grace period that is at least twice the amount of zookeeper * refresh time. */ public static final String GRACE_PERIOD = "drill.exec.grace_period_ms"; @@ -880,10 +880,11 @@ public final class ExecConstants { public static final String LIST_FILES_RECURSIVELY = "storage.list_files_recursively"; public static final BooleanValidator LIST_FILES_RECURSIVELY_VALIDATOR = new BooleanValidator(LIST_FILES_RECURSIVELY, - new OptionDescription("Enables recursive files listing when querying the `INFORMATION_SCHEMA.FILES` table or executing the SHOW FILES command. Default is false. (Drill 1.15+)")); + new OptionDescription("Enables recursive files listing when querying the `INFORMATION_SCHEMA.FILES` table or executing the SHOW FILES command. " + + "Default is false. (Drill 1.15+)")); - public static final String RETURN_RESULT_SET_FOR_DDL = "exec.return_result_set_for_ddl"; + public static final String RETURN_RESULT_SET_FOR_DDL = "exec.query.return_result_set_for_ddl"; public static final BooleanValidator RETURN_RESULT_SET_FOR_DDL_VALIDATOR = new BooleanValidator(RETURN_RESULT_SET_FOR_DDL, - new OptionDescription("Controls whether to return result set for CREATE TABLE/VIEW, DROP TABLE/VIEW, SET, USE etc. queries. " + + new OptionDescription("Controls whether to return result set for CREATE TABLE / VIEW, DROP TABLE / VIEW, SET, USE etc. queries. " + "If set to false affected rows count will be returned instead and result set will be null. Default is true. (Drill 1.15+)")); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java index c7963a5..c92f5f8 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java @@ -173,11 +173,13 @@ public class DrillSqlWorker { handler = new DefaultSqlHandler(config, textPlan); } - boolean returnResultSet = context.getOptions().getBoolean(ExecConstants.RETURN_RESULT_SET_FOR_DDL); - // Determine whether result set should be returned for the query based on `exec.return_result_set_for_ddl` - // and sql node kind. Overrides the option on a query level. - context.getOptions().setLocalOption(ExecConstants.RETURN_RESULT_SET_FOR_DDL, - returnResultSet || !SqlKind.DDL.contains(sqlNode.getKind())); + // Determines whether result set should be returned for the query based on return result set option and sql node kind. + // Overrides the option on a query level if it differs from the current value. + boolean currentReturnResultValue = context.getOptions().getBoolean(ExecConstants.RETURN_RESULT_SET_FOR_DDL); + boolean newReturnResultSetValue = currentReturnResultValue || !SqlKind.DDL.contains(sqlNode.getKind()); + if (newReturnResultSetValue != currentReturnResultValue) { + context.getOptions().setLocalOption(ExecConstants.RETURN_RESULT_SET_FOR_DDL, true); + } return handler.getPlan(sqlNode); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java index 3a7dcff..61d430d 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java @@ -84,7 +84,7 @@ public class OptionValue implements Comparable<OptionValue> { * This defines where an option was actually configured. */ public enum OptionScope { - BOOT, SYSTEM, SESSION, QUERY; + BOOT, SYSTEM, SESSION, QUERY } public final String name; @@ -186,6 +186,11 @@ public class OptionValue implements Comparable<OptionValue> { } } + @JsonIgnore + public OptionScope getScope() { + return scope; + } + public PersistedOptionValue toPersisted() { return new PersistedOptionValue(kind, name, num_val, string_val, bool_val, float_val); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java index 1faf6e4..35a3706 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java @@ -24,6 +24,9 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeMap; +import java.util.function.Predicate; +import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.drill.common.config.DrillConfig; @@ -40,7 +43,6 @@ import org.apache.drill.exec.server.rest.WebServer; import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.drill.shaded.guava.com.google.common.base.CaseFormat; -import org.apache.drill.shaded.guava.com.google.common.collect.Maps; /** * Wrapper class for a {@link #profile query profile}, so it to be presented through web UI. @@ -303,22 +305,39 @@ public class ProfileWrapper { return sb.append("}").toString(); } + public Map<String, String> getOptions() { + return getOptions(o -> true); + } + + public Map<String, String> getSessionOptions() { + return getOptions(o -> OptionValue.OptionScope.SESSION == o.getScope()); + } + + public Map<String, String> getQueryOptions() { + return getOptions(o -> OptionValue.OptionScope.QUERY == o.getScope()); + } + /** * Generates sorted map with properties used to display on Web UI, * where key is property name and value is property string value. + * Options are filtered based on {@link OptionValue.OptionScope}. + * <p/> * When property value is null, it would be replaced with 'null', * this is achieved using {@link String#valueOf(Object)} method. * Options will be stored in ascending key order, sorted according * to the natural order for the option name represented by {@link String}. * + * @param filter filter based on {@link OptionValue.OptionScope} * @return map with properties names and string values */ - public Map<String, String> getOptions() { - final Map<String, String> map = Maps.newTreeMap(); - for (OptionValue option : options) { - map.put(option.getName(), String.valueOf(option.getValue())); - } - return map; + private Map<String, String> getOptions(Predicate<OptionValue> filter) { + return options.stream() + .filter(filter) + .collect(Collectors.toMap( + OptionValue::getName, + o -> String.valueOf(o.getValue()), + (o, n) -> n, + TreeMap::new)); } /** diff --git a/exec/java-exec/src/main/resources/drill-module.conf b/exec/java-exec/src/main/resources/drill-module.conf index a2d3cdc..3682a85 100644 --- a/exec/java-exec/src/main/resources/drill-module.conf +++ b/exec/java-exec/src/main/resources/drill-module.conf @@ -646,5 +646,5 @@ drill.exec.options: { planner.index.prefer_intersect_plans: false, planner.index.max_indexes_to_intersect: 5, exec.query.rowkeyjoin_batchsize: 128, - exec.return_result_set_for_ddl: true, + exec.query.return_result_set_for_ddl: true } diff --git a/exec/java-exec/src/main/resources/rest/profile/profile.ftl b/exec/java-exec/src/main/resources/rest/profile/profile.ftl index 59f8485..ee126ab 100644 --- a/exec/java-exec/src/main/resources/rest/profile/profile.ftl +++ b/exec/java-exec/src/main/resources/rest/profile/profile.ftl @@ -288,38 +288,23 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de </div> </div> - <#assign options = model.getOptions()> - <#if (options?keys?size > 0)> + <#assign sessionOptions = model.getSessionOptions()> + <#assign queryOptions = model.getQueryOptions()> + <#if (sessionOptions?keys?size > 0 || queryOptions?keys?size > 0) > <div class="page-header"></div> - <h3>Session Options</h3> - <div class="panel-group" id="session-options-accordion"> + <h3>Options</h3> + <div class="panel-group" id="options-accordion"> <div class="panel panel-default"> <div class="panel-heading"> <h4 class="panel-title"> - <a data-toggle="collapse" href="#session-options-overview"> + <a data-toggle="collapse" href="#options-overview"> Overview </a> </h4> </div> - <div id="session-options-overview" class="panel-collapse collapse in"> - <div class="panel-body"> - <table class="table table-bordered"> - <thead> - <tr> - <th>Name</th> - <th>Value</th> - </tr> - </thead> - <tbody> - <#list options?keys as name> - <tr> - <td>${name}</td> - <td>${options[name]}</td> - </tr> - </#list> - </tbody> - </table> - </div> + <div id="options-overview" class="panel-collapse collapse in"> + <@list_options options=sessionOptions scope="Session" /> + <@list_options options=queryOptions scope="Query" /> </div> </div> </div> @@ -527,4 +512,28 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de </#macro> +<#macro list_options options scope> + <#if (options?keys?size > 0) > + <div class="panel-body"> + <h4>${scope} Options</h4> + <table id="${scope}_options_table" class="table table-bordered"> + <thead> + <tr> + <th>Name</th> + <th>Value</th> + </tr> + </thead> + <tbody> + <#list options?keys as name> + <tr> + <td>${name}</td> + <td>${options[name]}</td> + </tr> + </#list> + </tbody> + </table> + </div> + </#if> +</#macro> + <@page_html/>
