[
https://issues.apache.org/jira/browse/DRILL-4576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16497373#comment-16497373
]
ASF GitHub Bot commented on DRILL-4576:
---------------------------------------
ilooner closed pull request #466: DRILL-4576: Add PlannerCallback interface for
additional planner initialization.
URL: https://github.com/apache/drill/pull/466
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/planner/PlannerCallback.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerCallback.java
new file mode 100644
index 0000000000..96061e8a9b
--- /dev/null
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerCallback.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.planner;
+
+import java.util.Collection;
+
+import org.apache.calcite.plan.RelOptPlanner;
+
+/**
+ * A callback that StoragePlugins can initialize to allow further configuration
+ * of the Planner at initialization time. Examples could be to allow adding
lattices,
+ * materializations or additional traits to the planner that will be used in
+ * planning.
+ */
+public abstract class PlannerCallback {
+
+ /**
+ * Method that will be called before a planner is used to further configure
the planner.
+ * @param planner The planner to be configured.
+ */
+ public abstract void initializePlanner(RelOptPlanner planner);
+
+
+ public static PlannerCallback merge(Collection<PlannerCallback> callbacks){
+ return new PlannerCallbackCollection(callbacks);
+ }
+
+ private static class PlannerCallbackCollection extends PlannerCallback{
+ private Collection<PlannerCallback> callbacks;
+
+ private PlannerCallbackCollection(Collection<PlannerCallback> callbacks){
+ this.callbacks = callbacks;
+ }
+
+ @Override
+ public void initializePlanner(RelOptPlanner planner) {
+ for(PlannerCallback p : callbacks){
+ p.initializePlanner(planner);
+ }
+ }
+
+
+ }
+}
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
index 341bae271e..bbd014df2c 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
@@ -64,6 +64,7 @@
import org.apache.drill.exec.physical.base.AbstractPhysicalVisitor;
import org.apache.drill.exec.physical.base.PhysicalOperator;
import org.apache.drill.exec.physical.impl.join.JoinUtils;
+import org.apache.drill.exec.planner.PlannerCallback;
import org.apache.drill.exec.planner.PlannerPhase;
import org.apache.drill.exec.planner.PlannerType;
import org.apache.drill.exec.planner.common.DrillRelOptUtil;
@@ -358,6 +359,7 @@ protected RelNode transform(PlannerType plannerType,
PlannerPhase phase, RelNode
boolean log) {
final Stopwatch watch = Stopwatch.createStarted();
final RuleSet rules = config.getRules(phase);
+ final PlannerCallback callback = config.getPlannerCallback(phase);
final RelTraitSet toTraits = targetTraits.simplify();
final RelNode output;
@@ -373,6 +375,7 @@ protected RelNode transform(PlannerType plannerType,
PlannerPhase phase, RelNode
}
final HepPlanner planner = new HepPlanner(hepPgmBldr.build(),
context.getPlannerSettings());
+ callback.initializePlanner(planner);
final List<RelMetadataProvider> list = Lists.newArrayList();
list.add(DrillDefaultRelMetadataProvider.INSTANCE);
@@ -394,6 +397,8 @@ protected RelNode transform(PlannerType plannerType,
PlannerPhase phase, RelNode
// as weird as it seems, the cluster's only planner is the volcano
planner.
final RelOptPlanner planner = input.getCluster().getPlanner();
final Program program = Programs.of(rules);
+ callback.initializePlanner(planner);
+
Preconditions.checkArgument(planner instanceof VolcanoPlanner,
"Cluster is expected to be constructed using VolcanoPlanner. Was
actually of type %s.", planner.getClass()
.getName());
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerConfig.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerConfig.java
index 493b097091..d29174d8a5 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerConfig.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerConfig.java
@@ -19,16 +19,20 @@
package org.apache.drill.exec.planner.sql.handlers;
import java.util.Collection;
+import java.util.List;
import java.util.Map.Entry;
import org.apache.calcite.tools.RuleSet;
import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.planner.PlannerCallback;
import org.apache.drill.exec.planner.PlannerPhase;
import org.apache.drill.exec.planner.sql.SqlConverter;
+import org.apache.drill.exec.store.AbstractStoragePlugin;
import org.apache.drill.exec.store.StoragePlugin;
import com.google.common.collect.Lists;
+
public class SqlHandlerConfig {
private final QueryContext context;
@@ -45,11 +49,28 @@ public QueryContext getContext() {
}
public RuleSet getRules(PlannerPhase phase) {
+ return phase.getRules(context, getPlugins());
+ }
+
+ public PlannerCallback getPlannerCallback(PlannerPhase phase){
+ List<PlannerCallback> callbacks = Lists.newArrayList();
+ for(StoragePlugin plugin: getPlugins()){
+ if(plugin instanceof AbstractStoragePlugin){
+ PlannerCallback callback =
((AbstractStoragePlugin)plugin).getPlannerCallback(context, phase);
+ if(callback != null){
+ callbacks.add(callback);
+ }
+ }
+ }
+ return PlannerCallback.merge(callbacks);
+ }
+
+ private Collection<StoragePlugin> getPlugins(){
Collection<StoragePlugin> plugins = Lists.newArrayList();
for (Entry<String, StoragePlugin> k : context.getStorage()) {
plugins.add(k.getValue());
}
- return phase.getRules(context, plugins);
+ return plugins;
}
public SqlConverter getConverter() {
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
index fa2c450b7e..49529b0ba1 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
@@ -25,7 +25,9 @@
import org.apache.drill.common.JSONOptions;
import org.apache.drill.common.expression.SchemaPath;
import org.apache.drill.exec.ops.OptimizerRulesContext;
+import org.apache.drill.exec.ops.QueryContext;
import org.apache.drill.exec.physical.base.AbstractGroupScan;
+import org.apache.drill.exec.planner.PlannerCallback;
import org.apache.drill.exec.planner.PlannerPhase;
import com.google.common.collect.ImmutableSet;
@@ -98,6 +100,18 @@ public boolean supportsWrite() {
}
+ /**
+ * Return a planner callback for this storage plugin (or null if one doesn't
exist).
+ * @param queryContext The query context associated with the current
planning process.
+ * @param phase The phase of query planning that this callback will be
registered in.
+ * @return A callback or null if the plugin doesn't have any callbacks to
include.
+ *
+ * Note: Move this method to {@link StoragePlugin} interface in next major
version release.
+ */
+ public PlannerCallback getPlannerCallback(QueryContext queryContext,
PlannerPhase phase) {
+ return null;
+ }
+
@Override
public AbstractGroupScan getPhysicalScan(String userName, JSONOptions
selection) throws IOException {
return getPhysicalScan(userName, selection, AbstractGroupScan.ALL_COLUMNS);
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
index ad3858671d..a04d85b82c 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
@@ -31,6 +31,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
@@ -80,6 +81,8 @@
private final PersistentStore<StoragePluginConfig> pluginSystemTable;
private final LogicalPlanPersistence lpPersistence;
private final ScanResult classpathScan;
+ private final ConcurrentHashMap<String, Integer> manuallyAddedPlugins = new
ConcurrentHashMap<>();
+
private final LoadingCache<StoragePluginConfig, StoragePlugin>
ephemeralPlugins;
public StoragePluginRegistryImpl(DrillbitContext context) {
@@ -174,7 +177,9 @@ public void init() throws DrillbitStartupException {
activePlugins.put(INFORMATION_SCHEMA_PLUGIN, new
InfoSchemaStoragePlugin(new InfoSchemaConfig(), context,
INFORMATION_SCHEMA_PLUGIN));
+ manuallyAddedPlugins.put(INFORMATION_SCHEMA_PLUGIN, 0);
activePlugins.put(SYS_PLUGIN, new
SystemTablePlugin(SystemTablePluginConfig.INSTANCE, context, SYS_PLUGIN));
+ manuallyAddedPlugins.put(SYS_PLUGIN, 0);
return activePlugins;
} catch (IOException e) {
@@ -186,12 +191,15 @@ public void init() throws DrillbitStartupException {
@Override
public void addPlugin(String name, StoragePlugin plugin) {
plugins.put(name, plugin);
+ manuallyAddedPlugins.put(name, 0);
}
public void deletePlugin(String name) {
StoragePlugin plugin = plugins.remove(name);
closePlugin(plugin);
- pluginSystemTable.delete(name);
+ if(manuallyAddedPlugins.remove(name) == null){
+ pluginSystemTable.delete(name);
+ }
}
private void closePlugin(StoragePlugin plugin) {
@@ -245,7 +253,7 @@ public StoragePlugin createOrUpdate(String name,
StoragePluginConfig config, boo
public StoragePlugin getPlugin(String name) throws ExecutionSetupException {
StoragePlugin plugin = plugins.get(name);
- if (name.equals(SYS_PLUGIN) || name.equals(INFORMATION_SCHEMA_PLUGIN)) {
+ if (manuallyAddedPlugins.containsKey(name)) {
return plugin;
}
@@ -354,7 +362,7 @@ public void registerSchemas(SchemaConfig schemaConfig,
SchemaPlus parent) throws
}
// remove those which are no longer in the registry
for (String pluginName : currentPluginNames) {
- if (pluginName.equals(SYS_PLUGIN) ||
pluginName.equals(INFORMATION_SCHEMA_PLUGIN)) {
+ if (manuallyAddedPlugins.containsKey(pluginName)) {
continue;
}
plugins.remove(pluginName);
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/exec/planner/TestPlannerCallback.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/TestPlannerCallback.java
new file mode 100644
index 0000000000..1d4124e76d
--- /dev/null
+++
b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/TestPlannerCallback.java
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.planner;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.util.CancelFlag;
+import org.apache.drill.BaseTestQuery;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.store.AbstractStoragePlugin;
+import org.apache.drill.exec.store.SchemaConfig;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class TestPlannerCallback extends BaseTestQuery {
+
+ @BeforeClass
+ public static void setupDefaultTestCluster() throws Exception {
+ BaseTestQuery.setupDefaultTestCluster();
+ bits[0].getContext().getStorage().addPlugin("fake", new
FakeStoragePlugin());
+ }
+
+ @Test
+ public void ensureCallbackIsRegistered() throws Exception {
+ try{
+ test("select * from cp.`employee.json`");
+ }catch(Exception e){
+ assertTrue(e.getMessage().contains("Statement preparation aborted"));
+ return;
+ }
+
+ fail("Should have seen exception in planning due to planner
initialization.");
+ }
+
+ public static class FakeStoragePlugin extends AbstractStoragePlugin {
+
+ @Override
+ public StoragePluginConfig getConfig() {
+ return null;
+ }
+
+ @Override
+ public void registerSchemas(SchemaConfig schemaConfig, SchemaPlus parent)
throws IOException {
+ // TODO Auto-generated method stub
+ }
+
+ @Override
+ public PlannerCallback getPlannerCallback(QueryContext optimizerContext,
PlannerPhase phase) {
+ return new PlannerCallback(){
+ @Override
+ public void initializePlanner(RelOptPlanner planner) {
+ CancelFlag f = new CancelFlag();
+ f.requestCancel();
+ planner.setCancelFlag(f);
+ }
+
+ };
+ }
+
+
+ }
+}
----------------------------------------------------------------
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]
> Add StoragePlugin API to register materialization into planner
> --------------------------------------------------------------
>
> Key: DRILL-4576
> URL: https://issues.apache.org/jira/browse/DRILL-4576
> Project: Apache Drill
> Issue Type: Bug
> Reporter: Laurent Goujon
> Priority: Major
>
> There's no currently a good way to register materializations into Drill
> planner. Calcite's MaterializationService.instance() would be the way to go,
> but the registration happens in
> {{org.apache.calcite.prepare.Prepare.PreparedResult#prepareSql()}}, which is
> not called by Drill.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
