asfgit closed pull request #1584: DRILL-6922: Do not set return result set
option on query level if it is the same as current value
URL: https://github.com/apache/drill/pull/1584
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 c4d7652f3f0..afa29d902e3 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 static String bootDefaultFor(String name) {
* 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 static String bootDefaultFor(String name) {
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 c7963a50abe..c92f5f83027 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 @@ private static PhysicalPlan getQueryPlan(QueryContext
context, String sql, Point
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 3a7dcffaaa5..61d430ddf84 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 boolean inScopeOf(OptionScope scope) {
* 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 Object getValue() {
}
}
+ @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 1faf6e4ae35..35a37065597 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.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 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 String getOperatorsJSON() {
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 a2d3cdc7a50..3682a855fa0 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 59f84853a9b..ee126abd3b9 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/>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
