Re: Agenda for Hangout?
Hello Charles, About the first topic, we finally agreed to compare available options[1], so it's not planned to make any moves in this way until POC will be ready. I'll be happy to hear about all three other topics that you've mentioned. Thanks for arranging the Drill Hangout. [1] https://issues.apache.org/jira/browse/DRILL-7541 Kind regards, Igor On Thu, Jan 23, 2020 at 5:24 AM Charles Givre wrote: > Hello all, > I wanted to propose an agenda for Friday's hangout. I know we've been > talking about Drill/Arrow, so I think that merits some discussion > > 1. Drill/Arrow Discussion > 2. Code cleanup > 3. New Features > 4. Future of Drill > > Are there other topics or things we should discuss? > --C
Re: Slack Channel
Charles, I don’t think Slack channel is that popular among Drill devs. I guess best recommendation is to ask to send email to user mailing list. Maybe some automatic reply can be configured. Kind regards, Arina > On Jan 22, 2020, at 9:33 PM, Charles Givre wrote: > > Hey Drill Devs > There are two pending questions on the Drill slack channel, one relating to > Hive and the other relating to complex data in Drill. Could you guys take a > look? > Thx, > -- C
[GitHub] [drill] cgivre commented on issue #1959: DRILL-7548: Updating Environment.md to Skip Tests
cgivre commented on issue #1959: DRILL-7548: Updating Environment.md to Skip Tests URL: https://github.com/apache/drill/pull/1959#issuecomment-577496050 +1 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Created] (DRILL-7548) Updating Environment.md to Skip Tests
Charles Givre created DRILL-7548: Summary: Updating Environment.md to Skip Tests Key: DRILL-7548 URL: https://issues.apache.org/jira/browse/DRILL-7548 Project: Apache Drill Issue Type: Bug Reporter: Charles Givre Building Drill from source with all unit tests sometimes fails and takes > 40 minutes. Developers should be aware that skipping the tests is the most efficient way to initially build Drill. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Agenda for Hangout?
Hello all, I wanted to propose an agenda for Friday's hangout. I know we've been talking about Drill/Arrow, so I think that merits some discussion 1. Drill/Arrow Discussion 2. Code cleanup 3. New Features 4. Future of Drill Are there other topics or things we should discuss? --C
[GitHub] [drill] drewctate opened a new pull request #1959: Updating dev environment setup doc to skip tests on install
drewctate opened a new pull request #1959: Updating dev environment setup doc to skip tests on install URL: https://github.com/apache/drill/pull/1959 Updating the instructions for setting up the development environment. Developers are now instructed to skip running tests as part of initial setup. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Created] (DRILL-7547) More secure storage for mongodb credentials
Dobes Vandermeer created DRILL-7547: --- Summary: More secure storage for mongodb credentials Key: DRILL-7547 URL: https://issues.apache.org/jira/browse/DRILL-7547 Project: Apache Drill Issue Type: Improvement Components: Storage - MongoDB Affects Versions: 1.17.0 Reporter: Dobes Vandermeer Currently you can sort of "hide" S3 AWS credentials in core-site.xml, but for the mongodb connection the username and password are accessible from the Web UI and API because it is placed in the configuration for the storage plugin. I wonder if it would be possible to store the username and password used for mongodb connection in a more secure manner, maybe it could be encrypted when you first save it, then even if you look at the configuration for the mongodb storage plugin you cannot extract the username and password. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Slack Channel
Hey Drill Devs There are two pending questions on the Drill slack channel, one relating to Hive and the other relating to complex data in Drill. Could you guys take a look? Thx, -- C
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369729458
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/FilterPushDownStrategy.java
##
@@ -0,0 +1,416 @@
+/*
+ * 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.store.base.filter;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptRuleOperand;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.util.Pair;
+import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.ops.OptimizerRulesContext;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.planner.common.DrillRelOptUtil;
+import org.apache.drill.exec.planner.logical.DrillFilterRel;
+import org.apache.drill.exec.planner.logical.DrillOptiq;
+import org.apache.drill.exec.planner.logical.DrillParseContext;
+import org.apache.drill.exec.planner.logical.DrillProjectRel;
+import org.apache.drill.exec.planner.logical.DrillScanRel;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+import org.apache.drill.exec.planner.physical.FilterPrel;
+import org.apache.drill.exec.planner.physical.PrelUtil;
+import org.apache.drill.exec.store.StoragePluginOptimizerRule;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+
+/**
+ * Generalized filter push-down strategy which performs all the tree-walking
+ * and tree restructuring work, allowing a "listener" to do the work needed
+ * for a particular scan.
+ *
+ * General usage in a storage plugin:
+ * public Set getPhysicalOptimizerRules(
+ *OptimizerRulesContext optimizerRulesContext) {
+ * return FilterPushDownStrategy.rulesFor(optimizerRulesContext,
+ * new MyPushDownListener(...));
+ * }
+ *
+ */
+
+public class FilterPushDownStrategy {
+
+ private static final Collection BANNED_OPERATORS =
+ Lists.newArrayList("flatten");
+
+ /**
+ * Base rule that passes target information to the push-down strategy
+ */
+
+ private static abstract class AbstractFilterPushDownRule extends
StoragePluginOptimizerRule {
+
+protected final FilterPushDownStrategy strategy;
+
+public AbstractFilterPushDownRule(RelOptRuleOperand operand, String
description,
+FilterPushDownStrategy strategy) {
+ super(operand, description);
+ this.strategy = strategy;
+}
+ }
+
+ /**
+ * Calcite rule for FILTER --> PROJECT --> SCAN
+ */
+
+ private static class ProjectAndFilterRule extends AbstractFilterPushDownRule
{
+
+private ProjectAndFilterRule(FilterPushDownStrategy strategy) {
+ super(RelOptHelper.some(FilterPrel.class,
RelOptHelper.some(DrillProjectRel.class, RelOptHelper.any(DrillScanRel.class))),
+strategy.namePrefix() + "PushDownFilter:Filter_On_Project",
+strategy);
+}
+
+@Override
+public boolean matches(RelOptRuleCall call) {
+ if (!super.matches(call)) {
+return false;
+ }
+ DrillScanRel scan = call.rel(2);
+ return strategy.isTargetScan(scan);
+}
+
+@Override
+public void onMatch(RelOptRuleCall call) {
+ DrillFilterRel filterRel = call.rel(0);
+ DrillProjectRel projectRel = call.rel(1);
+ DrillScanRel scanRel = call.rel(2);
+ strategy.onMatch(call, filterRel, projectRel, scanRel);
+}
+ }
+
+ /**
+ * Calcite rule for FILTER --> SCAN
+ */
+
+ private static class FilterWithoutProjectRule extends
AbstractFilterPushDownRule {
+
+private FilterWithoutProjectRule(FilterPushDownStrategy strategy) {
+ super(RelOptHelper.some(DrillFilterRel.class,
RelOptHelper.any(DrillScanRel.class)),
+strategy.namePrefix() +
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369664401
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/FilterPushDownStrategy.java
##
@@ -0,0 +1,404 @@
+/*
+ * 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.store.base.filter;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptRuleOperand;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.util.Pair;
+import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.ops.OptimizerRulesContext;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.planner.common.DrillRelOptUtil;
+import org.apache.drill.exec.planner.logical.DrillFilterRel;
+import org.apache.drill.exec.planner.logical.DrillOptiq;
+import org.apache.drill.exec.planner.logical.DrillParseContext;
+import org.apache.drill.exec.planner.logical.DrillProjectRel;
+import org.apache.drill.exec.planner.logical.DrillScanRel;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+import org.apache.drill.exec.planner.physical.FilterPrel;
+import org.apache.drill.exec.planner.physical.PrelUtil;
+import org.apache.drill.exec.store.StoragePluginOptimizerRule;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+
+/**
+ * Generalized filter push-down strategy which performs all the tree-walking
+ * and tree restructuring work, allowing a "listener" to do the work needed
+ * for a particular scan.
+ *
+ * General usage in a storage plugin:
+ * public Set getPhysicalOptimizerRules(
+ *OptimizerRulesContext optimizerRulesContext) {
+ * return FilterPushDownStrategy.rulesFor(optimizerRulesContext,
+ * new MyPushDownListener(...));
+ * }
+ *
+ */
+public class FilterPushDownStrategy {
+
+ private static final Collection BANNED_OPERATORS =
+ Collections.singletonList("flatten");
+
+ /**
+ * Base rule that passes target information to the push-down strategy
+ */
+ private static abstract class AbstractFilterPushDownRule extends
StoragePluginOptimizerRule {
+
+protected final FilterPushDownStrategy strategy;
+
+public AbstractFilterPushDownRule(RelOptRuleOperand operand, String
description,
+FilterPushDownStrategy strategy) {
+ super(operand, description);
+ this.strategy = strategy;
+}
+ }
+
+ /**
+ * Calcite rule for FILTER --> PROJECT --> SCAN
+ */
+ private static class ProjectAndFilterRule extends AbstractFilterPushDownRule
{
+
+private ProjectAndFilterRule(FilterPushDownStrategy strategy) {
+ super(RelOptHelper.some(FilterPrel.class,
RelOptHelper.some(DrillProjectRel.class, RelOptHelper.any(DrillScanRel.class))),
+strategy.namePrefix() + "PushDownFilter:Filter_On_Project",
+strategy);
+}
+
+@Override
+public boolean matches(RelOptRuleCall call) {
+ if (!super.matches(call)) {
+return false;
+ }
+ DrillScanRel scan = call.rel(2);
+ return strategy.isTargetScan(scan);
+}
+
+@Override
+public void onMatch(RelOptRuleCall call) {
+ DrillFilterRel filterRel = call.rel(0);
+ DrillProjectRel projectRel = call.rel(1);
+ DrillScanRel scanRel = call.rel(2);
+ strategy.onMatch(call, filterRel, projectRel, scanRel);
+}
+ }
+
+ /**
+ * Calcite rule for FILTER --> SCAN
+ */
+ private static class FilterWithoutProjectRule extends
AbstractFilterPushDownRule {
+
+private FilterWithoutProjectRule(FilterPushDownStrategy strategy) {
+ super(RelOptHelper.some(DrillFilterRel.class,
RelOptHelper.any(DrillScanRel.class)),
+strategy.namePrefix() + "PushDownFilter:Filter_On_Scan",
+strategy);
+}
+
+
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins URL: https://github.com/apache/drill/pull/1914#discussion_r369673001 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/package-info.java ## @@ -0,0 +1,48 @@ +/* + * 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. + */ +/** + * Provides a standard, reusable framework for implementing filter push-down + * for storage plugins. Handles the work of parsing a Drill physical plan + * (using Calcite rules) to extract candidate predicates, converting those + * to a normalized form, and calling a listener to check if the predicates + * are eligible for push-down, then to implement the push-down. + * + * Some plugins may which to remove the pushed conditions. That way, Drill Review comment: Looks like a mistake: may which This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369663654
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/FilterPushDownStrategy.java
##
@@ -0,0 +1,404 @@
+/*
+ * 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.store.base.filter;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptRuleOperand;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.util.Pair;
+import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.ops.OptimizerRulesContext;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.planner.common.DrillRelOptUtil;
+import org.apache.drill.exec.planner.logical.DrillFilterRel;
+import org.apache.drill.exec.planner.logical.DrillOptiq;
+import org.apache.drill.exec.planner.logical.DrillParseContext;
+import org.apache.drill.exec.planner.logical.DrillProjectRel;
+import org.apache.drill.exec.planner.logical.DrillScanRel;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+import org.apache.drill.exec.planner.physical.FilterPrel;
+import org.apache.drill.exec.planner.physical.PrelUtil;
+import org.apache.drill.exec.store.StoragePluginOptimizerRule;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+
+/**
+ * Generalized filter push-down strategy which performs all the tree-walking
+ * and tree restructuring work, allowing a "listener" to do the work needed
+ * for a particular scan.
+ *
+ * General usage in a storage plugin:
+ * public Set getPhysicalOptimizerRules(
+ *OptimizerRulesContext optimizerRulesContext) {
+ * return FilterPushDownStrategy.rulesFor(optimizerRulesContext,
+ * new MyPushDownListener(...));
+ * }
+ *
+ */
+public class FilterPushDownStrategy {
+
+ private static final Collection BANNED_OPERATORS =
+ Collections.singletonList("flatten");
+
+ /**
+ * Base rule that passes target information to the push-down strategy
+ */
+ private static abstract class AbstractFilterPushDownRule extends
StoragePluginOptimizerRule {
+
+protected final FilterPushDownStrategy strategy;
+
+public AbstractFilterPushDownRule(RelOptRuleOperand operand, String
description,
+FilterPushDownStrategy strategy) {
+ super(operand, description);
+ this.strategy = strategy;
+}
+ }
+
+ /**
+ * Calcite rule for FILTER --> PROJECT --> SCAN
Review comment:
Please remove Calcite here, replace it with a planner or something like that
to avoid confusion. We use Calcite rules, but these rules are provided by
Calcite.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369720302
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/base/TestFilterPushDown.java
##
@@ -0,0 +1,264 @@
+/*
+ * 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.store.base;
+
+import org.apache.drill.test.ClusterFixtureBuilder;
+import org.apache.drill.test.ClusterTest;
+import org.junit.BeforeClass;
+import org.junit.Ignore;
+import org.junit.Test;
+
+/**
+ * Tests for the Filter push-down helper classes as part of the
+ * "Base" storage plugin to be used for add-on plugins. Uses a
+ * "test mule" ("Dummy") plug-in which goes through the paces of
+ * planning push down, but then glosses over the details at run time.
+ * The focus here are plans: the tests plan a query then compare the
+ * actual plan against and expected ("golden") plan.
+ */
+public class TestFilterPushDown extends ClusterTest {
+
+ private static final String BASE_SQL = "SELECT a, b FROM dummy.myTable";
+ private static final String BASE_WHERE = BASE_SQL + " WHERE ";
+ private static final PlanVerifier verifier = new
PlanVerifier("/store/base/");
+
+ // Uncomment the next line to save failing plans to /tmp
+ // static { verifier.saveResults = true; }
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ClusterFixtureBuilder builder = new ClusterFixtureBuilder(dirTestWatcher);
+startCluster(builder);
+
+DummyStoragePluginConfig config =
+new DummyStoragePluginConfig(true, true, false);
+cluster.defineStoragePlugin("dummy", config);
+ }
+
+ //-
+ // Unsupported filter push-down cases
+
+ // No predicates
+
+ @Test
+ public void testNoPushDown() throws Exception
+ {
Review comment:
Please move brace to the same line
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins URL: https://github.com/apache/drill/pull/1914#discussion_r369689965 ## File path: exec/java-exec/src/test/java/org/apache/drill/exec/store/base/DummyBatchReader.java ## @@ -0,0 +1,83 @@ +package org.apache.drill.exec.store.base; Review comment: Please move it below license. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369665119
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/FilterPushDownStrategy.java
##
@@ -0,0 +1,404 @@
+/*
+ * 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.store.base.filter;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptRuleOperand;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.util.Pair;
+import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.ops.OptimizerRulesContext;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.planner.common.DrillRelOptUtil;
+import org.apache.drill.exec.planner.logical.DrillFilterRel;
+import org.apache.drill.exec.planner.logical.DrillOptiq;
+import org.apache.drill.exec.planner.logical.DrillParseContext;
+import org.apache.drill.exec.planner.logical.DrillProjectRel;
+import org.apache.drill.exec.planner.logical.DrillScanRel;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+import org.apache.drill.exec.planner.physical.FilterPrel;
+import org.apache.drill.exec.planner.physical.PrelUtil;
+import org.apache.drill.exec.store.StoragePluginOptimizerRule;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+
+/**
+ * Generalized filter push-down strategy which performs all the tree-walking
+ * and tree restructuring work, allowing a "listener" to do the work needed
+ * for a particular scan.
+ *
+ * General usage in a storage plugin:
+ * public Set getPhysicalOptimizerRules(
+ *OptimizerRulesContext optimizerRulesContext) {
+ * return FilterPushDownStrategy.rulesFor(optimizerRulesContext,
+ * new MyPushDownListener(...));
+ * }
+ *
+ */
+public class FilterPushDownStrategy {
+
+ private static final Collection BANNED_OPERATORS =
+ Collections.singletonList("flatten");
+
+ /**
+ * Base rule that passes target information to the push-down strategy
+ */
+ private static abstract class AbstractFilterPushDownRule extends
StoragePluginOptimizerRule {
+
+protected final FilterPushDownStrategy strategy;
+
+public AbstractFilterPushDownRule(RelOptRuleOperand operand, String
description,
+FilterPushDownStrategy strategy) {
+ super(operand, description);
+ this.strategy = strategy;
+}
+ }
+
+ /**
+ * Calcite rule for FILTER --> PROJECT --> SCAN
+ */
+ private static class ProjectAndFilterRule extends AbstractFilterPushDownRule
{
+
+private ProjectAndFilterRule(FilterPushDownStrategy strategy) {
+ super(RelOptHelper.some(FilterPrel.class,
RelOptHelper.some(DrillProjectRel.class, RelOptHelper.any(DrillScanRel.class))),
+strategy.namePrefix() + "PushDownFilter:Filter_On_Project",
+strategy);
+}
+
+@Override
+public boolean matches(RelOptRuleCall call) {
+ if (!super.matches(call)) {
+return false;
+ }
+ DrillScanRel scan = call.rel(2);
+ return strategy.isTargetScan(scan);
+}
+
+@Override
+public void onMatch(RelOptRuleCall call) {
+ DrillFilterRel filterRel = call.rel(0);
+ DrillProjectRel projectRel = call.rel(1);
+ DrillScanRel scanRel = call.rel(2);
+ strategy.onMatch(call, filterRel, projectRel, scanRel);
+}
+ }
+
+ /**
+ * Calcite rule for FILTER --> SCAN
+ */
+ private static class FilterWithoutProjectRule extends
AbstractFilterPushDownRule {
+
+private FilterWithoutProjectRule(FilterPushDownStrategy strategy) {
+ super(RelOptHelper.some(DrillFilterRel.class,
RelOptHelper.any(DrillScanRel.class)),
+strategy.namePrefix() + "PushDownFilter:Filter_On_Scan",
+strategy);
+}
+
+
[jira] [Created] (DRILL-7546) DrillbitContext.getCurrentEndpoint state is allways "startup"
Oleg Zinoviev created DRILL-7546: Summary: DrillbitContext.getCurrentEndpoint state is allways "startup" Key: DRILL-7546 URL: https://issues.apache.org/jira/browse/DRILL-7546 Project: Apache Drill Issue Type: Bug Affects Versions: 1.16.0 Reporter: Oleg Zinoviev For now DrillbitContext:: getEndpoint::getState always returns STARTUP. Since CoordinationProtos.DrillbitEndpoint is actively used in comparisons on equals or as a key in hash map (e.g. in AssignmentCreator). this can lead to situations where the comparison for the current node returns false if the node information obtained from the coordinator and from DrillbitContext::getEndpoint is compared. Мне кажется, что я наблюдаю такую ситуацию в AssignmentCreator. Не могли бы вы проверить это поведение? К сожалению я не понимаю, как написать тест, демонстрирующий это. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369654393
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/FilterPushDownListener.java
##
@@ -0,0 +1,140 @@
+/*
+ * 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.store.base.filter;
+
+import java.util.List;
+
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.Pair;
+import org.apache.drill.exec.physical.base.GroupScan;
+
+/**
+ * Call-back (listener) implementation for a push-down filter.
+ * Abstracts away the common work; plugins implement this class
+ * to do work specific to the plugin.
+ *
+ * Supports two kinds of filter push down:
+ *
+ * Conjunctive Normal Form (CNF)
+ * A series of expressions joined by an AND: the scan should
+ * produce only rows that satisfy all the conditions.
+ * Disjunctive Normal Form (DNF)
+ * A series of alternative values for a single column, essentially
+ * a set of expressions joined by OR. The scan spits into multiple
+ * scans, each scanning one of the partitions (or regions or
+ * queries) identified by the case. This is an implementation of the
+ * SQL {@code IN} clause.
+ *
+ *
+ * In both cases, the conditions are in the form of a
+ * {@link RelOp} in which one side refers to a column in the scan
+ * and the other is a constant expression. The "driver" will ensure
+ * the rel op is of the correct form; this class ensures that the
+ * column is valid for the scan and the type of the value matches the
+ * column type (or can be converted.)
+ *
+ * The DNF form further ensures that all rel ops refer to the same
+ * column, and that only the equality operator appears in the
+ * terms.
+ */
+public interface FilterPushDownListener {
+
+ /**
+ * @return a prefix to display in filter rules
+ */
+ String prefix();
+
+ /**
+ * Broad check to see if the scan is of the correct type for this
+ * listener. Generally implemented as:
+ * public boolean isTargetScan(ScanPrel scan) {
+ * return scan.getGroupScan() instanceof MyGroupScan;
+ * }
+ *
+ * @return true if the given group scan is one this listener can
+ * handle, false otherwise
+ */
+ boolean isTargetScan(GroupScan groupScan);
+
+ /**
+ * Check if the filter rule should be applied to the target group scan.
+ * Calcite will run this rule multiple times during planning, but the
+ * transform only needs to occur once.
+ * Allows the group scan to mark in its own way whether the rule has
+ * been applied.
+ *
+ * @param groupScan the scan node
+ * @return true if filter push-down should be applied
+ */
+ boolean needsApplication(GroupScan groupScan);
+
+ /**
+ * Determine if the given relational operator (which is already in the form
+ * {@code }, qualifies for push down for
+ * this scan.
+ *
+ * If so, return an equivalent RelOp with the value normalized to what
+ * the plugin needs. The returned value may be the same as the original
+ * one if the value is already normalized.
+ *
+ * @param groupScan the scan element. Use {@code scan.getGroupScan()}
+ * to get the group scan
+ * @param relOp the description of the relational operator expression
+ * @return a normalized RelOp if this relop can be transformed into a filter
+ * push-down, @{code null} if not and thus the relop should remain in
+ * the Drill plan
+ * @see {@link
ConstantHolder#normalize(org.apache.drill.common.types.TypeProtos.MinorType)}
+ */
+ RelOp accept(GroupScan groupScan, RelOp relOp);
+
+ /**
+ * Transform a normalized DNF term into a new scan. Normalized form is:
+ *
+ * (a AND b AND (x OR y))
+ * In which each {@code OR} term represents a scan partition. It
+ * is up to the code here to determine if the scan partition can be handled,
+ * corresponds to a storage partition, or can be done as a separate
+ * scan (as for a JDBC or REST plugin, say.)
+ *
+ * Each term is accompanied by the Calcite expression from which it was
+ * derived. The caller is responsible for determining which expressions,
+ * if any, to
[GitHub] [drill] KazydubB commented on a change in pull request #1954: DRILL-7509: Incorrect TupleSchema is created for DICT column when querying Parquet files
KazydubB commented on a change in pull request #1954: DRILL-7509: Incorrect TupleSchema is created for DICT column when querying Parquet files URL: https://github.com/apache/drill/pull/1954#discussion_r369654055 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/FilterEvaluatorUtils.java ## @@ -113,7 +113,9 @@ public static RowsMatch matches(LogicalExpression expr, Map rangeExprEvaluator = new StatisticsProvider(columnsStatistics, rowCount); rowsMatch = parquetPredicate.matches(rangeExprEvaluator); } -return rowsMatch == RowsMatch.ALL && isRepeated(schemaPathsInExpr, fileMetadata) ? RowsMatch.SOME : rowsMatch; +return rowsMatch == RowsMatch.ALL +&& (isRepeated(schemaPathsInExpr, fileMetadata) || isDictOrRepeatedMapChild(schemaPathsInExpr, fileMetadata)) Review comment: I'll better leave this check after the `if (parquetPredicate != null)` to allow `RowsMatch.NONE` to be returned. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] KazydubB commented on issue #1954: DRILL-7509: Incorrect TupleSchema is created for DICT column when querying Parquet files
KazydubB commented on issue #1954: DRILL-7509: Incorrect TupleSchema is created for DICT column when querying Parquet files URL: https://github.com/apache/drill/pull/1954#issuecomment-577257787 @paul-rogers, what is the best place to leave the comment: somewhere in (affected) code or git message? Thanks, I'll add something when I'll find the best place to do so. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369617363
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/FilterPushDownListener.java
##
@@ -0,0 +1,140 @@
+/*
+ * 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.store.base.filter;
+
+import java.util.List;
+
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.Pair;
+import org.apache.drill.exec.physical.base.GroupScan;
+
+/**
+ * Call-back (listener) implementation for a push-down filter.
+ * Abstracts away the common work; plugins implement this class
+ * to do work specific to the plugin.
+ *
+ * Supports two kinds of filter push down:
+ *
+ * Conjunctive Normal Form (CNF)
+ * A series of expressions joined by an AND: the scan should
+ * produce only rows that satisfy all the conditions.
+ * Disjunctive Normal Form (DNF)
+ * A series of alternative values for a single column, essentially
+ * a set of expressions joined by OR. The scan spits into multiple
+ * scans, each scanning one of the partitions (or regions or
+ * queries) identified by the case. This is an implementation of the
+ * SQL {@code IN} clause.
+ *
+ *
+ * In both cases, the conditions are in the form of a
+ * {@link RelOp} in which one side refers to a column in the scan
+ * and the other is a constant expression. The "driver" will ensure
+ * the rel op is of the correct form; this class ensures that the
+ * column is valid for the scan and the type of the value matches the
+ * column type (or can be converted.)
+ *
+ * The DNF form further ensures that all rel ops refer to the same
+ * column, and that only the equality operator appears in the
+ * terms.
+ */
+public interface FilterPushDownListener {
+
+ /**
+ * @return a prefix to display in filter rules
+ */
+ String prefix();
+
+ /**
+ * Broad check to see if the scan is of the correct type for this
+ * listener. Generally implemented as:
+ * public boolean isTargetScan(ScanPrel scan) {
+ * return scan.getGroupScan() instanceof MyGroupScan;
+ * }
+ *
+ * @return true if the given group scan is one this listener can
+ * handle, false otherwise
+ */
+ boolean isTargetScan(GroupScan groupScan);
+
+ /**
+ * Check if the filter rule should be applied to the target group scan.
+ * Calcite will run this rule multiple times during planning, but the
+ * transform only needs to occur once.
+ * Allows the group scan to mark in its own way whether the rule has
+ * been applied.
+ *
+ * @param groupScan the scan node
+ * @return true if filter push-down should be applied
+ */
+ boolean needsApplication(GroupScan groupScan);
+
+ /**
+ * Determine if the given relational operator (which is already in the form
+ * {@code }, qualifies for push down for
+ * this scan.
+ *
+ * If so, return an equivalent RelOp with the value normalized to what
+ * the plugin needs. The returned value may be the same as the original
+ * one if the value is already normalized.
+ *
+ * @param groupScan the scan element. Use {@code scan.getGroupScan()}
+ * to get the group scan
+ * @param relOp the description of the relational operator expression
+ * @return a normalized RelOp if this relop can be transformed into a filter
+ * push-down, @{code null} if not and thus the relop should remain in
+ * the Drill plan
+ * @see {@link
ConstantHolder#normalize(org.apache.drill.common.types.TypeProtos.MinorType)}
+ */
+ RelOp accept(GroupScan groupScan, RelOp relOp);
+
+ /**
+ * Transform a normalized DNF term into a new scan. Normalized form is:
+ *
+ * (a AND b AND (x OR y))
+ * In which each {@code OR} term represents a scan partition. It
+ * is up to the code here to determine if the scan partition can be handled,
+ * corresponds to a storage partition, or can be done as a separate
+ * scan (as for a JDBC or REST plugin, say.)
+ *
+ * Each term is accompanied by the Calcite expression from which it was
+ * derived. The caller is responsible for determining which expressions,
+ * if any, to
Re: DICT keys in projection
Hi Paul,
Regarding your point "We can also handle a map projection: `a.b` which
matches:
* A (possibly repeated) map
* A (possibly repeated) DICT with VARCHAR keys
* A UNION (because a union might contain a possibly-repeated map)
* A LIST (because the list can contain a union which might contain a
possibly-repeated map)":
I am not sure why `a.b` is possible for REPEATED MAP - this looks as a
shortcut of some sort. I mean, it looks wrong with respect to data types,
isn't it? Consider an example in Java: `Map[] a = ...;
Object result = a.get("b");` does not yield array of Integer; let's pretend
the 'Map' represents a Drill's MAP. But this notation
could have been an alias to some 'function', like `Integer[] array =
collect((Map) a, "b")`. This does not work for REPEATED
MAP in Drill currently, though such behaviour is present in Hive. (I am not
saying this is wrong to support it for a REPEATED MAP, it may be useful.)
In the case of REPEATED DICT we _may_ choose not to support such
"shortcut", but provide UDFs with needed functionality.
Regarding using keys in filter: I think, it is a good idea to provide UDFs
for such needs. Hive, for example, has following functions for (Hive's) MAP
[1] (see "Collection Functions"):
array map_keys(Map)
array map_values(Map)
But yes, we must treat projections as general as possible until the real
schema is known and this is a hard task.
[1]
https://cwiki.apache.org/confluence/display/Hive/LanguageManual+UDF#LanguageManualUDF-OperatorsonComplexTypes
[GitHub] [drill] vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
vvysotskyi commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369492043
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/BaseGroupScan.java
##
@@ -0,0 +1,481 @@
+/*
+ * 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.store.base;
+
+import java.util.List;
+
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.physical.base.AbstractGroupScan;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.base.ScanStats;
+import org.apache.drill.exec.physical.base.SubScan;
+import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.server.options.OptionManager;
+import org.apache.drill.exec.store.SchemaConfig;
+import org.apache.drill.exec.store.SchemaFactory;
+import org.apache.drill.exec.store.StoragePluginRegistry;
+import org.apache.drill.exec.store.base.BaseStoragePlugin.StoragePluginOptions;
+import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/**
+ * Base group scan for storage plugins. A group scan is a "logical" scan: it is
+ * the representation of the scan used during the logical and physical planning
+ * phases. The group scan is converted to a "sub scan" (an executable scan
+ * specification) for inclusion in the physical plan sent to Drillbits for
+ * execution. The group scan represents the entire scan of a table or data
+ * source. The sub scan divides that scan into files, storage blocks or other
+ * forms of parallelism.
+ *
+ * The group scan participates in both logical and physical planning. As
+ * noted below, logical plan information is JSON serialized, but physical
+ * plan information is not. The transition from logical
+ * to physical planning is not clear. The first call to
+ * {@link #getMinParallelizationWidth()} or
+ * {@link #getMaxParallelizationWidth()} is a good signal. The group scan
+ * is copied multiple times (each with more information) during logical
+ * planning, but is not copied during physical planning. This means that
+ * physical planning state can be thought of as transient: it should not
+ * be serialized or copied.
+ *
+ * Because the group scan is part of the Calcite planning process, it is
+ * very helpful to understand the basics of query planning and how
+ * Calcite implements that process.
+ *
+ * Serialization
+ *
+ * Drill provides the ability to serialize the logical plan. This is most
+ * easily seen by issuing the EXPLAIN PLAN FOR command for a
+ * query. The Jackson-serialized representation of the group scan appears
+ * in the JSON partition of the EXPLAIN output, while the
+ * toString() output appears in the text output of that
+ * command.
+ *
+ * Care must be taken when serializing: include only the logical
+ * plan information, omit the physical plan information.
+ *
+ * Any fields that are part of the logical plan must be Jackson
+ * serializable. The following information should be serialized in the
+ * logical plan:
+ *
+ * Information from the scan spec (from the table lookup in the
+ * schema) if relevant to the plugin.
+ * The list of columns projected in the scan.
+ * Any filters pushed into the query (in whatever form makes sense
+ * for the plugin.
+ * Any other plugin-specific logical plan information.
+ *
+ * This base class (and its superclasses) serialize the plugin config,
+ * user name, column list and scan stats. The derived class should handle
+ * other fields.
+ *
+ * On the other hand, the kinds of information should not be
+ * serialized:
+ *
+ * The set of drillbits on which queries will run.
+ * The actual number of minor fragments that run scans.
+ * Other physical plan information.
+ * Cached data (such as the cached scan stats, etc.
+ *
+ *
+ * Jackson will use the constructor marked with @JsonCreator to
+ * deserialize
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369434855
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/base/DummyGroupScan.java
##
@@ -0,0 +1,214 @@
+/*
+ * 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.store.base;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.base.ScanStats;
+import org.apache.drill.exec.physical.base.ScanStats.GroupScanProperty;
+import org.apache.drill.exec.physical.base.SubScan;
+import org.apache.drill.exec.store.StoragePluginRegistry;
+import org.apache.drill.exec.store.base.filter.FilterSpec;
+import org.apache.drill.exec.store.base.filter.RelOp;
+import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+import com.fasterxml.jackson.annotation.JacksonInject;
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonInclude.Include;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+@JsonTypeName("dummy-scan")
+@JsonPropertyOrder({"userName", "scanSpec", "columns",
+"filters", "cost", "config"})
+@JsonInclude(value=Include.NON_EMPTY, content=Include.NON_NULL)
+public class DummyGroupScan extends BaseGroupScan {
+
+ private final DummyScanSpec scanSpec;
+ private final FilterSpec filters;
+
+ public DummyGroupScan(DummyStoragePlugin storagePlugin, String userName,
+ DummyScanSpec scanSpec) {
+super(storagePlugin, userName, null);
+this.scanSpec = scanSpec;
+filters = null;
+ }
+
+ public DummyGroupScan(DummyGroupScan from, List columns) {
+super(from.storagePlugin, from.getUserName(), columns);
+this.scanSpec = from.scanSpec;
+this.filters = from.filters;
+ }
+
+ @JsonCreator
+ public DummyGroupScan(
+ @JsonProperty("config") DummyStoragePluginConfig config,
+ @JsonProperty("userName") String userName,
+ @JsonProperty("scanSpec") DummyScanSpec scanSpec,
+ @JsonProperty("columns") List columns,
+ @JsonProperty("filters") FilterSpec filters,
+ @JacksonInject StoragePluginRegistry engineRegistry) {
+super(config, userName, columns, engineRegistry);
+this.scanSpec = scanSpec;
+this.filters = filters;
+ }
+
+ public DummyGroupScan(DummyGroupScan from, FilterSpec filters) {
+super(from);
+this.scanSpec = from.scanSpec;
+this.filters = filters;
+ }
+
+ @JsonProperty("scanSpec")
+ public DummyScanSpec scanSpec() { return scanSpec; }
+
+ @JsonProperty("filters")
+ public FilterSpec andFilters() { return filters; }
+
+ public boolean hasFilters() {
+return filters != null && ! filters.isEmpty();
+ }
+
+ private static final List FILTER_COLS = ImmutableList.of("a", "b",
"id");
Review comment:
Use `Arrays.asList`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369426397
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/ConstantHolder.java
##
@@ -0,0 +1,172 @@
+/*
+ * 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.store.base.filter;
+
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.store.base.PlanStringBuilder;
+import org.joda.time.DateTimeZone;
+import org.joda.time.format.DateTimeFormatter;
+import org.joda.time.format.ISODateTimeFormat;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
+
+/**
+ * Description of a constant argument of an expression.
+ */
+
+@JsonPropertyOrder({"type", "value"})
+public class ConstantHolder {
+ @JsonProperty("type")
+ public final MinorType type;
+ @JsonProperty("value")
+ public final Object value;
+
+ @JsonCreator
+ public ConstantHolder(
+ @JsonProperty("type") MinorType type,
+ @JsonProperty("value") Object value) {
+this.type = type;
+this.value = value;
+ }
+
+ public ConstantHolder convertTo(MinorType toType) {
+if (type == toType) {
+ return this;
+}
+switch (toType) {
+case INT:
+ return toInt();
+case BIGINT:
+ return toBigInt();
+case TIMESTAMP:
+ return toTimestamp(null);
+case VARCHAR:
+ return toVarChar();
+default:
+ throw conversionError(toType);
+}
+ }
+
+ public ConstantHolder normalize(MinorType toType) {
+try {
+ return convertTo(toType);
+} catch (Throwable e) {
+ return null;
Review comment:
What is others need to push down more types? What they should do?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369433958
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/BaseScanFactory.java
##
@@ -0,0 +1,82 @@
+/*
+ * 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.store.base;
+
+import java.util.List;
+
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.exec.metastore.MetadataProviderManager;
+import
org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework.ScanFrameworkBuilder;
+import org.apache.drill.exec.server.options.OptionManager;
+import org.apache.drill.exec.server.options.SessionOptionManager;
+
+/**
+ * Creates the multiple classes needed to plan and execute a scan. Centralizes
+ * most implementation-specific object creation in one location to avoid
needing
+ * to implement many methods just for type-specific creation.
+ *
+ * @param the storage plugin class
+ * @param the scan specification class
+ * @param the group scan class
+ * @param the sub scan class
+ */
+
+public abstract class BaseScanFactory<
+ PLUGIN extends BaseStoragePlugin,
+ SPEC,
+ GROUP extends BaseGroupScan,
+ SUB extends BaseSubScan> {
+
+ @SuppressWarnings("unchecked")
Review comment:
I see, I guess we don't have netter solution than casting...
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369433503
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/base/TestFilterPushDown.java
##
@@ -0,0 +1,339 @@
+/*
+ * 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.store.base;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.Reader;
+import java.io.StringReader;
+import java.net.URL;
+
+import org.apache.drill.exec.store.StoragePluginRegistry;
+import org.apache.drill.test.ClusterFixtureBuilder;
+import org.apache.drill.test.ClusterTest;
+import org.junit.BeforeClass;
+import org.junit.Ignore;
+import org.junit.Test;
+
+/**
+ * Tests for the Filter push-down helper classes as part of the
+ * "Base" storage plugin to be used for add-on plugins. Uses a
+ * "test mule" ("Dummy") plug-in which goes through the paces of
+ * planning push down, but then glosses over the details at run time.
+ * The focus here are plans: the tests plan a query then compare the
+ * actual plan against and expected ("golden") plan.
+ *
+ * If comparison fails, the tests print the actual plan to the console.
+ * Use this, when adding new tests, to create the initial "golden" file.
+ * Also, on failures, actual output is written to
+ * /tmp/drill/store/base. You can use your IDE to compare
+ * the actual and golden files to understand changes. If the changes
+ * are expected, use that same IDE to copy changes from the actual
+ * to the golden file.
+ *
+ * The JSON properties of the serialized classes are all controlled
+ * to have a fixed order to ensure that files compare across test
+ * runs.
+ */
+public class TestFilterPushDown extends ClusterTest {
+
+ private static final String BASE_SQL = "SELECT a, b FROM dummy.myTable";
+ private static final String BASE_WHERE = BASE_SQL + " WHERE ";
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ClusterFixtureBuilder builder = new ClusterFixtureBuilder(dirTestWatcher);
+startCluster(builder);
+
+StoragePluginRegistry pluginRegistry =
cluster.drillbit().getContext().getStorage();
+DummyStoragePluginConfig config =
+new DummyStoragePluginConfig(true, true, false);
+pluginRegistry.createOrUpdate("dummy", config, true);
Review comment:
It is used in JDBC plugin tests, please leave only one verions than yours or
previous one, I don't think we don't need two.
https://github.com/apache/drill/blob/465fae6b9dcea9e02a4bfb71d9a3b9d4b99e169d/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcPluginWithH2IT.java#L68
https://github.com/apache/drill/blob/465fae6b9dcea9e02a4bfb71d9a3b9d4b99e169d/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcPluginWithMySQLIT.java#L85
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369429278
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/base/PlanVerifier.java
##
@@ -0,0 +1,146 @@
+/*
+ * 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.store.base;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.Reader;
+import java.io.StringReader;
+import java.net.URL;
+
+import org.apache.drill.test.ClientFixture;
+
+/**
+ * Verifier for execution plans. A handy tool to ensure that the
+ * planner produces the expected plan given some set of conditions.
+ *
+ * The test works by comparing the actual {@code EXPLAIN} output
+ * to a "golden" file with the expected ("golden") plan.
+ *
+ * To create a test, just write it and let it fail due to a missing file.
+ * The output will go to the console. Inspect it. If it looks good,
+ * copy the plan to the golden file and run again.
+ *
+ * If comparison fails, you can optionally ask the verifier to write the
+ * output to {@code /tmp} so you can compare the golden and actual
+ * outputs using your favorite diff tool to understand changes. If the changes
+ * are expected, use that same IDE to copy changes from the actual
+ * to the golden file.
+ *
+ * The JSON properties of the serialized classes are all controlled
+ * to have a fixed order to ensure that files compare across test
+ * runs. If you see spurious failures do to changed JSON order, consider
+ * adding a {@code @JsonPropertyOrder} tag to enforce a consistent order.
+ *
+ * A fancier version of this class would use a regex or other mechanism
+ * to say "ignore differences in this bit of the plan." For example, when
+ * using systems with metadata, the exact values might bounce around
+ * some.
+ */
+public class PlanVerifier {
+
+ private final String basePath;
+
+ /**
+ * Persist test results. Turn this on if you have failing tests.
+ ( Then, you can diff the actual and golden results in your favorite
+ * tool or IDE.
+ */
+ public boolean saveResults;
Review comment:
Maybe add setter and make it private.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369428809
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/base/PlanVerifier.java
##
@@ -0,0 +1,146 @@
+/*
+ * 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.store.base;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.Reader;
+import java.io.StringReader;
+import java.net.URL;
+
+import org.apache.drill.test.ClientFixture;
+
+/**
+ * Verifier for execution plans. A handy tool to ensure that the
+ * planner produces the expected plan given some set of conditions.
+ *
+ * The test works by comparing the actual {@code EXPLAIN} output
+ * to a "golden" file with the expected ("golden") plan.
+ *
+ * To create a test, just write it and let it fail due to a missing file.
+ * The output will go to the console. Inspect it. If it looks good,
+ * copy the plan to the golden file and run again.
+ *
+ * If comparison fails, you can optionally ask the verifier to write the
+ * output to {@code /tmp} so you can compare the golden and actual
Review comment:
```suggestion
* output to {@code /tmp/drill/test} so you can compare the golden and actual
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369428346
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/base/DummyGroupScan.java
##
@@ -97,36 +100,33 @@ public RelOp acceptFilter(RelOp relOp) {
// Pretend that "id" is a special integer column. Can handle
// equality only.
-
if (relOp.colName.contentEquals("id")) {
// To allow easier testing, require exact type match: no
// attempt at type conversion here.
-
if (relOp.op != RelOp.Op.EQ || relOp.value.type != MinorType.INT) {
return null;
}
return relOp;
}
-// All other columns apply only if projected
-
-if (!FILTER_COLS.contains(relOp.colName)) {
+// "allTypes" table filters everything. All other tables
+// only project a fixed set of columns. Simulates a plugin
+// which can project only some columns.
+if (!scanSpec.tableName.equals("allTypes") &&
!FILTER_COLS.contains(relOp.colName)) {
Review comment:
Nit: usually constant goes first to avoid NPE `"allTypes".equals(...`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369426743
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/base/DummyStoragePlugin.java
##
@@ -0,0 +1,151 @@
+/*
+ * 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.store.base;
+
+import java.util.List;
+import java.util.Set;
+
+import org.apache.drill.common.exceptions.ChildErrorContext;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.common.types.Types;
+import org.apache.drill.exec.metastore.MetadataProviderManager;
+import org.apache.drill.exec.ops.OptimizerRulesContext;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedReader;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework;
+import
org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework.ReaderFactory;
+import
org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework.ScanFrameworkBuilder;
+import org.apache.drill.exec.physical.impl.scan.framework.SchemaNegotiator;
+import org.apache.drill.exec.planner.PlannerPhase;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.options.OptionManager;
+import org.apache.drill.exec.server.options.SessionOptionManager;
+import org.apache.drill.exec.store.StoragePluginOptimizerRule;
+import org.apache.drill.exec.store.base.filter.RelOp;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+
+/**
+ * "Test mule" for the base storage plugin and the filter push down
+ * framework.
+ */
+
+public class DummyStoragePlugin
+ extends BaseStoragePlugin {
+
+ private static class DummyScanFactory extends
+BaseScanFactory {
+
+@Override
+public DummyGroupScan newGroupScan(DummyStoragePlugin storagePlugin,
+String userName, DummyScanSpec scanSpec,
+SessionOptionManager sessionOptions,
+MetadataProviderManager metadataProviderManager) {
+
+ // Force user name to "dummy" so golden and actual test files are stable
+ return new DummyGroupScan(storagePlugin, "dummy", scanSpec);
+}
+
+@Override
+public DummyGroupScan groupWithColumns(DummyGroupScan group,
+List columns) {
+ return new DummyGroupScan(group, columns);
+}
+
+@Override
+public ScanFrameworkBuilder scanBuilder(DummyStoragePlugin storagePlugin,
+OptionManager options, DummySubScan subScan) {
+ ScanFrameworkBuilder builder = new ScanFrameworkBuilder();
Review comment:
Maybe add comment in Java doc somewhere? I could not have guessed this
without your comment...
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369426397
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/filter/ConstantHolder.java
##
@@ -0,0 +1,172 @@
+/*
+ * 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.store.base.filter;
+
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.store.base.PlanStringBuilder;
+import org.joda.time.DateTimeZone;
+import org.joda.time.format.DateTimeFormatter;
+import org.joda.time.format.ISODateTimeFormat;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
+
+/**
+ * Description of a constant argument of an expression.
+ */
+
+@JsonPropertyOrder({"type", "value"})
+public class ConstantHolder {
+ @JsonProperty("type")
+ public final MinorType type;
+ @JsonProperty("value")
+ public final Object value;
+
+ @JsonCreator
+ public ConstantHolder(
+ @JsonProperty("type") MinorType type,
+ @JsonProperty("value") Object value) {
+this.type = type;
+this.value = value;
+ }
+
+ public ConstantHolder convertTo(MinorType toType) {
+if (type == toType) {
+ return this;
+}
+switch (toType) {
+case INT:
+ return toInt();
+case BIGINT:
+ return toBigInt();
+case TIMESTAMP:
+ return toTimestamp(null);
+case VARCHAR:
+ return toVarChar();
+default:
+ throw conversionError(toType);
+}
+ }
+
+ public ConstantHolder normalize(MinorType toType) {
+try {
+ return convertTo(toType);
+} catch (Throwable e) {
+ return null;
Review comment:
What is others need to push down more? What they should do?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369426136
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/BaseStoragePlugin.java
##
@@ -0,0 +1,229 @@
+/*
+ * 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.store.base;
+
+import java.io.IOException;
+
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.drill.common.JSONOptions;
+import org.apache.drill.common.exceptions.ChildErrorContext;
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.metastore.MetadataProviderManager;
+import org.apache.drill.exec.ops.ExecutorFragmentContext;
+import org.apache.drill.exec.physical.base.AbstractGroupScan;
+import
org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework.ScanFrameworkBuilder;
+import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.options.SessionOptionManager;
+import org.apache.drill.exec.store.AbstractStoragePlugin;
+import org.apache.drill.exec.store.SchemaConfig;
+import org.apache.drill.exec.store.SchemaFactory;
+import org.apache.drill.exec.store.StoragePlugin;
+import org.apache.drill.exec.store.StoragePluginRegistry;
+import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+/**
+ * Base class for non-DFS storage plugins. Provides a number of convinces to
+ * abstract away some of the complexity around such plugins. The
+ * {@link StoragePluginOptions} class captures the many options that would
+ * otherwise be specified by overriding methods. The
+ * {@link BaseScanFactory} class provides methods to create various
+ * objects during planning and execution.
+ *
+ * @param the storage plugin configuration class
+ */
+
+public abstract class BaseStoragePlugin
+extends AbstractStoragePlugin {
+
+ private static final Logger logger =
LoggerFactory.getLogger(BaseStoragePlugin.class);
+ protected static final ObjectMapper DEFAULT_MAPPER = new ObjectMapper();
Review comment:
Well, unless it is static would would have to inject it using constructor or
though some context (for example, DrillbitContext).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369425418
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/BaseStoragePlugin.java
##
@@ -0,0 +1,229 @@
+/*
+ * 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.store.base;
+
+import java.io.IOException;
+
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.drill.common.JSONOptions;
+import org.apache.drill.common.exceptions.ChildErrorContext;
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.metastore.MetadataProviderManager;
+import org.apache.drill.exec.ops.ExecutorFragmentContext;
+import org.apache.drill.exec.physical.base.AbstractGroupScan;
+import
org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework.ScanFrameworkBuilder;
+import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.options.SessionOptionManager;
+import org.apache.drill.exec.store.AbstractStoragePlugin;
+import org.apache.drill.exec.store.SchemaConfig;
+import org.apache.drill.exec.store.SchemaFactory;
+import org.apache.drill.exec.store.StoragePlugin;
+import org.apache.drill.exec.store.StoragePluginRegistry;
+import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+/**
+ * Base class for non-DFS storage plugins. Provides a number of convinces to
+ * abstract away some of the complexity around such plugins. The
+ * {@link StoragePluginOptions} class captures the many options that would
+ * otherwise be specified by overriding methods. The
+ * {@link BaseScanFactory} class provides methods to create various
+ * objects during planning and execution.
+ *
+ * @param the storage plugin configuration class
+ */
+
+public abstract class BaseStoragePlugin
+extends AbstractStoragePlugin {
+
+ private static final Logger logger =
LoggerFactory.getLogger(BaseStoragePlugin.class);
+ protected static final ObjectMapper DEFAULT_MAPPER = new ObjectMapper();
+ public static final String DEFAULT_SCHEMA_NAME = "default";
+
+ public static class StoragePluginOptions {
+
+/**
+ * Identifies if the plugin supports read.
+ * true by default.
+ */
+public boolean supportsRead = true;
+
+/**
+ * Identifies if the plugin supports writes (as in
+ * CREATE TABLE AS. false
[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base framework for storage plugins
arina-ielchiieva commented on a change in pull request #1914: DRILL-7458: Base
framework for storage plugins
URL: https://github.com/apache/drill/pull/1914#discussion_r369425418
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/base/BaseStoragePlugin.java
##
@@ -0,0 +1,229 @@
+/*
+ * 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.store.base;
+
+import java.io.IOException;
+
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.drill.common.JSONOptions;
+import org.apache.drill.common.exceptions.ChildErrorContext;
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.metastore.MetadataProviderManager;
+import org.apache.drill.exec.ops.ExecutorFragmentContext;
+import org.apache.drill.exec.physical.base.AbstractGroupScan;
+import
org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework.ScanFrameworkBuilder;
+import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.options.SessionOptionManager;
+import org.apache.drill.exec.store.AbstractStoragePlugin;
+import org.apache.drill.exec.store.SchemaConfig;
+import org.apache.drill.exec.store.SchemaFactory;
+import org.apache.drill.exec.store.StoragePlugin;
+import org.apache.drill.exec.store.StoragePluginRegistry;
+import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+/**
+ * Base class for non-DFS storage plugins. Provides a number of convinces to
+ * abstract away some of the complexity around such plugins. The
+ * {@link StoragePluginOptions} class captures the many options that would
+ * otherwise be specified by overriding methods. The
+ * {@link BaseScanFactory} class provides methods to create various
+ * objects during planning and execution.
+ *
+ * @param the storage plugin configuration class
+ */
+
+public abstract class BaseStoragePlugin
+extends AbstractStoragePlugin {
+
+ private static final Logger logger =
LoggerFactory.getLogger(BaseStoragePlugin.class);
+ protected static final ObjectMapper DEFAULT_MAPPER = new ObjectMapper();
+ public static final String DEFAULT_SCHEMA_NAME = "default";
+
+ public static class StoragePluginOptions {
+
+/**
+ * Identifies if the plugin supports read.
+ * true by default.
+ */
+public boolean supportsRead = true;
+
+/**
+ * Identifies if the plugin supports writes (as in
+ * CREATE TABLE AS. false
[GitHub] [drill] arina-ielchiieva edited a comment on issue #1957: DRILL-7530: Fix class names in loggers
arina-ielchiieva edited a comment on issue #1957: DRILL-7530: Fix class names in loggers URL: https://github.com/apache/drill/pull/1957#issuecomment-577065856 @paul-rogers > Any plans to fix the remaining fully-qualified log class names? It gets tedious to do those by hand each time I touch a file. No, from my side. I also get tedious of the fixes but don't have time to fix all of them. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on issue #1957: DRILL-7530: Fix class names in loggers
arina-ielchiieva commented on issue #1957: DRILL-7530: Fix class names in
loggers
URL: https://github.com/apache/drill/pull/1957#issuecomment-577065856
@paul-rogers
{quote}
Any plans to fix the remaining fully-qualified log class names? It gets
tedious to do those by hand each time I touch a file.
{quote}
No, from my side. I also get tedious of the fixes but don't have time to fix
all of them.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
