Skip to content

Commit

Permalink
DRILL-6922: Do not set return result set option on query level if it …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
arina-ielchiieva authored and vdiravka committed Dec 24, 2018
1 parent 4a771d0 commit 44f17ea
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 42 deletions.
Expand Up @@ -854,7 +854,7 @@ public static String bootDefaultFor(String name) {
* the shutdown request is triggered. The primary use of grace period is to * 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 * avoid the race conditions caused by zookeeper delay in updating the state
* information of the drillbit that is shutting down. So, it is advisable * 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. * refresh time.
*/ */
public static final String GRACE_PERIOD = "drill.exec.grace_period_ms"; public static final String GRACE_PERIOD = "drill.exec.grace_period_ms";
Expand All @@ -880,10 +880,11 @@ public static String bootDefaultFor(String name) {


public static final String LIST_FILES_RECURSIVELY = "storage.list_files_recursively"; public static final String LIST_FILES_RECURSIVELY = "storage.list_files_recursively";
public static final BooleanValidator LIST_FILES_RECURSIVELY_VALIDATOR = new BooleanValidator(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, 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+)")); "If set to false affected rows count will be returned instead and result set will be null. Default is true. (Drill 1.15+)"));
} }
Expand Up @@ -173,11 +173,13 @@ private static PhysicalPlan getQueryPlan(QueryContext context, String sql, Point
handler = new DefaultSqlHandler(config, textPlan); handler = new DefaultSqlHandler(config, textPlan);
} }


boolean returnResultSet = context.getOptions().getBoolean(ExecConstants.RETURN_RESULT_SET_FOR_DDL); // Determines whether result set should be returned for the query based on return result set option and sql node kind.
// Determine whether result set should be returned for the query based on `exec.return_result_set_for_ddl` // Overrides the option on a query level if it differs from the current value.
// and sql node kind. Overrides the option on a query level. boolean currentReturnResultValue = context.getOptions().getBoolean(ExecConstants.RETURN_RESULT_SET_FOR_DDL);
context.getOptions().setLocalOption(ExecConstants.RETURN_RESULT_SET_FOR_DDL, boolean newReturnResultSetValue = currentReturnResultValue || !SqlKind.DDL.contains(sqlNode.getKind());
returnResultSet || !SqlKind.DDL.contains(sqlNode.getKind())); if (newReturnResultSetValue != currentReturnResultValue) {
context.getOptions().setLocalOption(ExecConstants.RETURN_RESULT_SET_FOR_DDL, true);
}


return handler.getPlan(sqlNode); return handler.getPlan(sqlNode);
} }
Expand Down
Expand Up @@ -84,7 +84,7 @@ public enum Kind {
* This defines where an option was actually configured. * This defines where an option was actually configured.
*/ */
public enum OptionScope { public enum OptionScope {
BOOT, SYSTEM, SESSION, QUERY; BOOT, SYSTEM, SESSION, QUERY
} }


public final String name; public final String name;
Expand Down Expand Up @@ -186,6 +186,11 @@ public Object getValue() {
} }
} }


@JsonIgnore
public OptionScope getScope() {
return scope;
}

public PersistedOptionValue toPersisted() { public PersistedOptionValue toPersisted() {
return new PersistedOptionValue(kind, name, num_val, string_val, bool_val, float_val); return new PersistedOptionValue(kind, name, num_val, string_val, bool_val, float_val);
} }
Expand Down
Expand Up @@ -24,6 +24,9 @@
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; 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.commons.lang3.tuple.ImmutablePair;
import org.apache.drill.common.config.DrillConfig; import org.apache.drill.common.config.DrillConfig;
Expand All @@ -40,7 +43,6 @@


import com.fasterxml.jackson.databind.ObjectMapper; 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.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. * Wrapper class for a {@link #profile query profile}, so it to be presented through web UI.
Expand Down Expand Up @@ -303,22 +305,39 @@ public String getOperatorsJSON() {
return sb.append("}").toString(); 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, * Generates sorted map with properties used to display on Web UI,
* where key is property name and value is property string value. * 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', * When property value is null, it would be replaced with 'null',
* this is achieved using {@link String#valueOf(Object)} method. * this is achieved using {@link String#valueOf(Object)} method.
* Options will be stored in ascending key order, sorted according * Options will be stored in ascending key order, sorted according
* to the natural order for the option name represented by {@link String}. * 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 * @return map with properties names and string values
*/ */
public Map<String, String> getOptions() { private Map<String, String> getOptions(Predicate<OptionValue> filter) {
final Map<String, String> map = Maps.newTreeMap(); return options.stream()
for (OptionValue option : options) { .filter(filter)
map.put(option.getName(), String.valueOf(option.getValue())); .collect(Collectors.toMap(
} OptionValue::getName,
return map; o -> String.valueOf(o.getValue()),
(o, n) -> n,
TreeMap::new));
} }


/** /**
Expand Down
2 changes: 1 addition & 1 deletion exec/java-exec/src/main/resources/drill-module.conf
Expand Up @@ -646,5 +646,5 @@ drill.exec.options: {
planner.index.prefer_intersect_plans: false, planner.index.prefer_intersect_plans: false,
planner.index.max_indexes_to_intersect: 5, planner.index.max_indexes_to_intersect: 5,
exec.query.rowkeyjoin_batchsize: 128, exec.query.rowkeyjoin_batchsize: 128,
exec.return_result_set_for_ddl: true, exec.query.return_result_set_for_ddl: true
} }
57 changes: 33 additions & 24 deletions exec/java-exec/src/main/resources/rest/profile/profile.ftl
Expand Up @@ -288,38 +288,23 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
</div> </div>
</div> </div>
<#assign options = model.getOptions()> <#assign sessionOptions = model.getSessionOptions()>
<#if (options?keys?size > 0)> <#assign queryOptions = model.getQueryOptions()>
<#if (sessionOptions?keys?size > 0 || queryOptions?keys?size > 0) >
<div class="page-header"></div> <div class="page-header"></div>
<h3>Session Options</h3> <h3>Options</h3>
<div class="panel-group" id="session-options-accordion"> <div class="panel-group" id="options-accordion">
<div class="panel panel-default"> <div class="panel panel-default">
<div class="panel-heading"> <div class="panel-heading">
<h4 class="panel-title"> <h4 class="panel-title">
<a data-toggle="collapse" href="#session-options-overview"> <a data-toggle="collapse" href="#options-overview">
Overview Overview
</a> </a>
</h4> </h4>
</div> </div>
<div id="session-options-overview" class="panel-collapse collapse in"> <div id="options-overview" class="panel-collapse collapse in">
<div class="panel-body"> <@list_options options=sessionOptions scope="Session" />
<table class="table table-bordered"> <@list_options options=queryOptions scope="Query" />
<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> </div>
</div> </div>
</div> </div>
Expand Down Expand Up @@ -527,4 +512,28 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
</#macro> </#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/> <@page_html/>

0 comments on commit 44f17ea

Please sign in to comment.