[jira] [Created] (DRILL-6803) Remove server protobuf definitions from client protobufs
Sorabh Hamirwasia created DRILL-6803: Summary: Remove server protobuf definitions from client protobufs Key: DRILL-6803 URL: https://issues.apache.org/jira/browse/DRILL-6803 Project: Apache Drill Issue Type: Sub-task Reporter: Sorabh Hamirwasia Update client side protobuf to not have server side definitions like QueryProfile / CoreOperatorType etc. This is a backward compatibility breaking change and might be good for 2.0 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (DRILL-6802) Update jdbc-all pom.xml to dependent upon new java-exec client module only not entire java-exec module
Sorabh Hamirwasia created DRILL-6802: Summary: Update jdbc-all pom.xml to dependent upon new java-exec client module only not entire java-exec module Key: DRILL-6802 URL: https://issues.apache.org/jira/browse/DRILL-6802 Project: Apache Drill Issue Type: Sub-task Reporter: Sorabh Hamirwasia Update jdbc-all pom to include newly created client artifact and jdbc driver artifact. * Have multiple profiles to include and exclude any profile specific dependency. For example MapR profile will exclude hadoop dependency whereas apache profile will include it. * We can create 2 artifacts for jdbc-all: one with and other without Hadoop dependencies. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (DRILL-6801) Divide java-exec into separate client, server and common module with separate pom.xml files for each module
Sorabh Hamirwasia created DRILL-6801: Summary: Divide java-exec into separate client, server and common module with separate pom.xml files for each module Key: DRILL-6801 URL: https://issues.apache.org/jira/browse/DRILL-6801 Project: Apache Drill Issue Type: Sub-task Reporter: Sorabh Hamirwasia -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (DRILL-6800) Simplify packaging of Jdbc-all jar
Sorabh Hamirwasia created DRILL-6800: Summary: Simplify packaging of Jdbc-all jar Key: DRILL-6800 URL: https://issues.apache.org/jira/browse/DRILL-6800 Project: Apache Drill Issue Type: Improvement Components: Client - JDBC Reporter: Sorabh Hamirwasia Today Jdbc-all package is created using drill-java-exec as dependency and then excluding unnecessary dependency. Also there is size check for jdbc-all jar to avoid including any unwanted dependency. But configured size has increased over time and doesn't really provide a good mechanism to enforce small footprint of jdbc-all jar. Following are some recommendation to improve it: 1) Divide java-exec module into separate client/server and common module 2) Have size check for client artifact only. 3) Update jdbc-all pom to include newly created client artifact and jdbc driver artifact. * Have multiple profiles to include and exclude any profile specific dependency. For example MapR profile will exclude hadoop dependency whereas apache profile will include it. * We can create 2 artifacts for jdbc-all: one with and other without (for smaller jar size) Hadoop dependencies. 4) Update client side protobuf to not have server side definitions. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] lushuifeng commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225775240 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java ## @@ -1074,7 +1078,12 @@ private void addBatchHolder(int part, int batchRowCount) { // These methods are overridden in the generated class when created as plain Java code. protected BatchHolder newBatchHolder(int batchRowCount) { -return new BatchHolder(batchRowCount); +return this.injectMembers(new BatchHolder(batchRowCount)); + } + + protected BatchHolder injectMembers(BatchHolder batchHolder) { +CodeGenMemberInjector.injectMembers(cg, batchHolder, context); Review comment: The ClassGenerator and FragmentContext is needed in this method for general cases, but not for test classes, see example in `ExampleTemplateWithInner.java` newBatchHolder is override by generated code can be implemented as follows to remove above method `protected BatchHolder newBatchHolder(int batchRowCount) { return CodeGenMemberInjector.injectMembers(cg, new BatchHolder(batchRowCount), context); }` but the `cg` and `context` params are hard coded, not flexable. I think it is better to make a function call to hide the implementation which may differ in each template. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225770061 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/CodeGenMemberInjector.java ## @@ -0,0 +1,79 @@ +/* + * 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.physical.impl.common; + +import com.sun.codemodel.JMethod; +import io.netty.buffer.DrillBuf; +import org.apache.calcite.util.Pair; +import org.apache.drill.exec.expr.ClassGenerator; +import org.apache.drill.exec.expr.holders.ValueHolder; +import org.apache.drill.exec.ops.FragmentContext; +import org.apache.drill.shaded.guava.com.google.common.base.Function; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; + +public class CodeGenMemberInjector { + + /** + * Generated code for a class may have several class members, they + * are initialized by invoking this method when the instance created. + * + * @param cg the class generator + * @param instance the class instance created by the compiler + * @param context the fragment context + */ + public static void injectMembers(ClassGenerator cg, Object instance, FragmentContext context) { +Map cachedInstances = new HashMap<>(); +for (Map.Entry, Function> setter : cg.getSetters().entrySet()) { Review comment: Sorry I don't quite follow you since the instance is created somewhere else not in setter collection 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225764315 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java ## @@ -646,7 +687,7 @@ public void preparePlainJava() { Class p = params[i]; childNew.arg(shim.param(model._ref(p), "arg" + i)); } - shim.body()._return(childNew); + shim.body()._return(JExpr._this().invoke("injectMembers").arg(childNew)); Review comment: The innerClasses like BatchHolder may have the constants, BatchHolder instance is created via newBatchHolder dynamically when processing recordBatch, that is the difference between innerClasses and nested splitting classes. The innerClasses member fields is initialized by invoking injectMembers(innerClassInstance) which is override generated by this code as shown in `protected BatchHolder newBatchHolder(int batchRowCount) { return this.injectMembers(new BatchHolder(batchRowCount)); }` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225762550 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java ## @@ -531,11 +541,42 @@ public JVar declareClassField(String prefix, JType t) { public JVar declareClassField(String prefix, JType t, JExpression init) { if (innerClassGenerator != null && hasMaxIndexValue()) { - return innerClassGenerator.clazz.field(JMod.NONE, t, prefix + index++, init); + return innerClassGenerator.declareClassField(prefix, t, init); } return clazz.field(JMod.NONE, t, prefix + index++, init); } + /** + * declare a setter method for the argument {@code var}. + * argument {@code function} holds the constant value which + * returns a value holder must be invoked when the class instance created. + * the setter method of innerClassField will be invoked if innerClassGenerator exists. + * + * @param var the class member variable + * @param function the function holds the constant value + * @return the depth of nested class, setter method + */ + public Pair declareSetterMethod(JVar var, Function function) { +JMethod setter = clazz.method(JMod.PUBLIC, void.class, "set" + StringUtils.capitalize(var.name())); Review comment: Is It better by NOT declaring the setter function, these fields can be accessed by reflection directly, so the number of constants in constant pool can be reduced 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Created] (DRILL-6799) Enhance the Hash-Join Operator to perform Anti-Semi-Join
Boaz Ben-Zvi created DRILL-6799: --- Summary: Enhance the Hash-Join Operator to perform Anti-Semi-Join Key: DRILL-6799 URL: https://issues.apache.org/jira/browse/DRILL-6799 Project: Apache Drill Issue Type: Improvement Components: Execution - Relational Operators, Query Planning Optimization Affects Versions: 1.14.0 Reporter: Boaz Ben-Zvi Assignee: Boaz Ben-Zvi Fix For: 1.16.0 Similar to handling Semi-Join (see DRILL-6735), the Anti-Semi-Join can be enhanced by eliminating the extra DISTINCT (i.e. Hash-Aggr) operator. Example (note the NOT IN): select c.c_first_name, c.c_last_name from dfs.`/data/json/s1/customer` c where c.c_customer_sk NOT IN (select s.ss_customer_sk from dfs.`/data/json/s1/store_sales` s) limit 4; -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (DRILL-6798) Planner changes to support semi-join
Boaz Ben-Zvi created DRILL-6798: --- Summary: Planner changes to support semi-join Key: DRILL-6798 URL: https://issues.apache.org/jira/browse/DRILL-6798 Project: Apache Drill Issue Type: Sub-task Affects Versions: 1.14.0 Reporter: Boaz Ben-Zvi Assignee: Hanumath Rao Maduri Fix For: 1.15.0 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] kkhatua commented on issue #668: DRILL-5068: Add a new system table for completed profiles
kkhatua commented on issue #668: DRILL-5068: Add a new system table for completed profiles URL: https://github.com/apache/drill/pull/668#issuecomment-430452821 Addressed in #1077 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] kkhatua closed pull request #668: DRILL-5068: Add a new system table for completed profiles
kkhatua closed pull request #668: DRILL-5068: Add a new system table for completed profiles URL: https://github.com/apache/drill/pull/668 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/store/sys/ProfileIterator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/ProfileIterator.java new file mode 100644 index 000..7152f45b986 --- /dev/null +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/ProfileIterator.java @@ -0,0 +1,141 @@ +/** + * 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.sys; + +import com.google.common.base.Function; +import com.google.common.collect.Iterators; +import org.apache.drill.exec.ops.FragmentContext; +import org.apache.drill.exec.proto.UserBitShared; +import org.apache.drill.exec.work.foreman.QueryManager; + +import javax.annotation.Nullable; +import java.sql.Timestamp; +import java.util.Iterator; +import java.util.Map; + +/** + * DRILL-5068: Add a new system table for completed profiles + */ +public class ProfileIterator implements Iterator { + static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ProfileIterator.class); + + private final Iterator itr; + + public ProfileIterator(FragmentContext context) { +itr = iterateProfileInfo(context); + } + + private Iterator iterateProfileInfo(FragmentContext context) { +try { + PersistentStore profiles = context +.getDrillbitContext() +.getStoreProvider() +.getOrCreateStore(QueryManager.QUERY_PROFILE); + + return transform(profiles.getAll()); + +} catch (Exception e) { + logger.error(String.format("Unable to get persistence store: %s, ", QueryManager.QUERY_PROFILE.getName()), e); + return Iterators.singletonIterator(ProfileInfo.getDefault()); +} + } + + /** + * Iterating persistentStore as a iterator of {@link org.apache.drill.exec.store.sys.ProfileIterator.ProfileInfo}. + */ + private Iterator transform(Iterator> all) { +return Iterators.transform(all, new Function, ProfileInfo>() { + @Nullable + @Override + public ProfileInfo apply(@Nullable Map.Entry input) { +if (input == null || input.getValue() == null) { + return ProfileInfo.getDefault(); +} + +final String queryID = input.getKey(); + +return new ProfileInfo(queryID, + mkHref(queryID), + new Timestamp(input.getValue().getStart()), + input.getValue().getEnd() - input.getValue().getStart(), + input.getValue().getUser(), + input.getValue().getQuery(), + input.getValue().getState().name() +); + } + + /** + * Generate a link to detailed profile page using queryID. this makes user be able to jump to that page directly from query result. + * @param queryID query ID + * @return html href link of the input query ID + */ + private String mkHref(String queryID) { +return String.format("%s", queryID, queryID); + } +}); + } + + @Override + public boolean hasNext() { +return itr.hasNext(); + } + + @Override + public Object next() { +return itr.next(); + } + + @Override + public void remove() { +throw new UnsupportedOperationException(); + } + + public static class ProfileInfo { +private static final ProfileInfo DEFAULT = new ProfileInfo(); + +public final String query_id; +public final String link; +public final Timestamp time; +public final long latency; +public final String user; +public final String query; +public final String state; + +public ProfileInfo(String query_id, String link, Timestamp time, long latency, String user, String query, String state) { + this.query_id = query_id; + this.link = link; + this.time = time; + this.latency = latency; + this.user = user; + this.query = query; + this.state = state; +
[GitHub] kkhatua commented on issue #536: DRILL-4699: Add description column to sys.options table
kkhatua commented on issue #536: DRILL-4699: Add description column to sys.options table URL: https://github.com/apache/drill/pull/536#issuecomment-430450365 Addressed as part of #1279 . 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] kkhatua closed pull request #536: DRILL-4699: Add description column to sys.options table
kkhatua closed pull request #536: DRILL-4699: Add description column to sys.options table URL: https://github.com/apache/drill/pull/536 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java index 0d7e0d09bec..9ac59304c1f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java @@ -33,64 +33,65 @@ import org.apache.drill.exec.testing.ExecutionControls; import org.apache.drill.exec.util.ImpersonationUtil; -public interface ExecConstants { - String ZK_RETRY_TIMES = "drill.exec.zk.retry.count"; - String ZK_RETRY_DELAY = "drill.exec.zk.retry.delay"; - String ZK_CONNECTION = "drill.exec.zk.connect"; - String ZK_TIMEOUT = "drill.exec.zk.timeout"; - String ZK_ROOT = "drill.exec.zk.root"; - String ZK_REFRESH = "drill.exec.zk.refresh"; - String BIT_RETRY_TIMES = "drill.exec.rpc.bit.server.retry.count"; - String BIT_RETRY_DELAY = "drill.exec.rpc.bit.server.retry.delay"; - String BIT_TIMEOUT = "drill.exec.bit.timeout" ; - String SERVICE_NAME = "drill.exec.cluster-id"; - String INITIAL_BIT_PORT = "drill.exec.rpc.bit.server.port"; - String BIT_RPC_TIMEOUT = "drill.exec.rpc.bit.timeout"; - String INITIAL_USER_PORT = "drill.exec.rpc.user.server.port"; - String USER_RPC_TIMEOUT = "drill.exec.rpc.user.timeout"; - String METRICS_CONTEXT_NAME = "drill.exec.metrics.context"; - String USE_IP_ADDRESS = "drill.exec.rpc.use.ip"; - String CLIENT_RPC_THREADS = "drill.exec.rpc.user.client.threads"; - String BIT_SERVER_RPC_THREADS = "drill.exec.rpc.bit.server.threads"; - String USER_SERVER_RPC_THREADS = "drill.exec.rpc.user.server.threads"; - String TRACE_DUMP_DIRECTORY = "drill.exec.trace.directory"; - String TRACE_DUMP_FILESYSTEM = "drill.exec.trace.filesystem"; - String TEMP_DIRECTORIES = "drill.exec.tmp.directories"; - String TEMP_FILESYSTEM = "drill.exec.tmp.filesystem"; - String INCOMING_BUFFER_IMPL = "drill.exec.buffer.impl"; +public final class ExecConstants { + + public static final String ZK_RETRY_TIMES = "drill.exec.zk.retry.count"; + public static final String ZK_RETRY_DELAY = "drill.exec.zk.retry.delay"; + public static final String ZK_CONNECTION = "drill.exec.zk.connect"; + public static final String ZK_TIMEOUT = "drill.exec.zk.timeout"; + public static final String ZK_ROOT = "drill.exec.zk.root"; + public static final String ZK_REFRESH = "drill.exec.zk.refresh"; + public static final String BIT_RETRY_TIMES = "drill.exec.rpc.bit.server.retry.count"; + public static final String BIT_RETRY_DELAY = "drill.exec.rpc.bit.server.retry.delay"; + public static final String BIT_TIMEOUT = "drill.exec.bit.timeout"; + public static final String SERVICE_NAME = "drill.exec.cluster-id"; + public static final String INITIAL_BIT_PORT = "drill.exec.rpc.bit.server.port"; + public static final String BIT_RPC_TIMEOUT = "drill.exec.rpc.bit.timeout"; + public static final String INITIAL_USER_PORT = "drill.exec.rpc.user.server.port"; + public static final String USER_RPC_TIMEOUT = "drill.exec.rpc.user.timeout"; + public static final String METRICS_CONTEXT_NAME = "drill.exec.metrics.context"; + public static final String USE_IP_ADDRESS = "drill.exec.rpc.use.ip"; + public static final String CLIENT_RPC_THREADS = "drill.exec.rpc.user.client.threads"; + public static final String BIT_SERVER_RPC_THREADS = "drill.exec.rpc.bit.server.threads"; + public static final String USER_SERVER_RPC_THREADS = "drill.exec.rpc.user.server.threads"; + public static final String TRACE_DUMP_DIRECTORY = "drill.exec.trace.directory"; + public static final String TRACE_DUMP_FILESYSTEM = "drill.exec.trace.filesystem"; + public static final String TEMP_DIRECTORIES = "drill.exec.tmp.directories"; + public static final String TEMP_FILESYSTEM = "drill.exec.tmp.filesystem"; + public static final String INCOMING_BUFFER_IMPL = "drill.exec.buffer.impl"; /** incoming buffer size (number of batches) */ - String INCOMING_BUFFER_SIZE = "drill.exec.buffer.size"; - String SPOOLING_BUFFER_DELETE = "drill.exec.buffer.spooling.delete"; - String SPOOLING_BUFFER_MEMORY = "drill.exec.buffer.spooling.size"; - String BATCH_PURGE_THRESHOLD = "drill.exec.sort.purge.threshold"; - String EXTERNAL_SORT_TARGET_BATCH_SIZE = "drill.exec.sort.external.batch.size"; - String EXTERNAL_SORT_TARGET_SPILL_BATCH_SIZE = "drill.exec.sort.external.spill.batch.size"; - String EXTERNAL_SORT_SPILL_GROUP_SIZE = "drill.exec.sort.external.spill.group.size"; - String EXTERNAL_SORT_SPILL_THRESHOLD = "drill.exec.sort.external.spill.threshold"; - String
[GitHub] kkhatua edited a comment on issue #1491: DRILL-6084: Show Drill functions in WebUI for autocomplete
kkhatua edited a comment on issue #1491: DRILL-6084: Show Drill functions in WebUI for autocomplete URL: https://github.com/apache/drill/pull/1491#issuecomment-430449367 @arina-ielchiieva made some additional changes: 1. Introduced the boolean `internal` field in the `sys.functions` table. For the WebUI, these functions are not available. 2. Tagged 2 function templates as `internal` 3. Added tests to validate the `internal` field in the unit tests. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] kkhatua commented on issue #1491: DRILL-6084: Show Drill functions in WebUI for autocomplete
kkhatua commented on issue #1491: DRILL-6084: Show Drill functions in WebUI for autocomplete URL: https://github.com/apache/drill/pull/1491#issuecomment-430449367 @arina-ielchiieva made some additional changes: 1. Introduced the boolean `internal` field in the `sys.functions` table. For the WebUI, these functions are not available. 2. Added tests to validate the `internal` field. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] amansinha100 commented on a change in pull request #1466: DRILL-6381: Add support for index based planning and execution
amansinha100 commented on a change in pull request #1466: DRILL-6381: Add support for index based planning and execution URL: https://github.com/apache/drill/pull/1466#discussion_r225740876 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java ## @@ -166,4 +169,25 @@ public void onMatch(RelOptRuleCall call) { return list; } + public static Project replace(Project topProject, Project bottomProject) { Review comment: They can be merged into the top level Project that allows duplicates...however, the purpose of the `replace()` method here is to simply allow the caller to replace a project with another with the assumption that callers know exactly what they are doing. This is not applying the full fledged `DrillMergeProjectRule`. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] amansinha100 commented on a change in pull request #1466: DRILL-6381: Add support for index based planning and execution
amansinha100 commented on a change in pull request #1466: DRILL-6381: Add support for index based planning and execution URL: https://github.com/apache/drill/pull/1466#discussion_r225737406 ## File path: contrib/format-maprdb/src/test/java/com/mapr/drill/maprdb/tests/index/IndexPlanTest.java ## @@ -0,0 +1,1715 @@ +/* + * 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 com.mapr.drill.maprdb.tests.index; + +import com.mapr.db.Admin; +import com.mapr.drill.maprdb.tests.MaprDBTestsSuite; +import com.mapr.drill.maprdb.tests.json.BaseJsonTest; +import com.mapr.tests.annotations.ClusterTest; +import org.apache.drill.PlanTestBase; +import org.joda.time.DateTime; +import org.joda.time.format.DateTimeFormat; +import org.apache.drill.common.config.DrillConfig; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.FixMethodOrder; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runners.MethodSorters; +import java.util.Properties; + + +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +@Category(ClusterTest.class) +public class IndexPlanTest extends BaseJsonTest { + + final static String PRIMARY_TABLE_NAME = "/tmp/index_test_primary"; + + final static int PRIMARY_TABLE_SIZE = 1; + private static final String sliceTargetSmall = "alter session set `planner.slice_target` = 1"; + private static final String sliceTargetDefault = "alter session reset `planner.slice_target`"; + private static final String noIndexPlan = "alter session set `planner.enable_index_planning` = false"; + private static final String defaultHavingIndexPlan = "alter session reset `planner.enable_index_planning`"; + private static final String disableHashAgg = "alter session set `planner.enable_hashagg` = false"; + private static final String enableHashAgg = "alter session set `planner.enable_hashagg` = true"; + private static final String defaultnonCoveringSelectivityThreshold = "alter session set `planner.index.noncovering_selectivity_threshold` = 0.025"; + private static final String incrnonCoveringSelectivityThreshold = "alter session set `planner.index.noncovering_selectivity_threshold` = 0.25"; + private static final String disableFTS = "alter session set `planner.disable_full_table_scan` = true"; + private static final String enableFTS = "alter session reset `planner.disable_full_table_scan`"; + private static final String preferIntersectPlans = "alter session set `planner.index.prefer_intersect_plans` = true"; + private static final String defaultIntersectPlans = "alter session reset `planner.index.prefer_intersect_plans`"; + private static final String lowRowKeyJoinBackIOFactor + = "alter session set `planner.index.rowkeyjoin_cost_factor` = 0.01"; + private static final String defaultRowKeyJoinBackIOFactor + = "alter session reset `planner.index.rowkeyjoin_cost_factor`"; + + /** + * A sample row of this 10K table: + --+-++ + | 1012 | {"city":"pfrrs","state":"pc"} | {"email":"kffzkuz...@gmail.com","phone":"655471"} | + {"ssn":"17423"} | {"fname":"KfFzK","lname":"UZwNk"} | {"age":53.0,"income":45.0} | 1012 | + * + * This test suite generate random content to fill all the rows, since the random function always start from + * the same seed for different runs, when the row count is not changed, the data in table will always be the same, + * thus the query result could be predicted and verified. + */ + + @BeforeClass + public static void setupTableIndexes() throws Exception { + +Properties overrideProps = new Properties(); + overrideProps.setProperty("format-maprdb.json.useNumRegionsForDistribution", "true"); +updateTestCluster(1, DrillConfig.create(overrideProps)); + +MaprDBTestsSuite.setupTests(); +MaprDBTestsSuite.createPluginAndGetConf(getDrillbitContext()); + +test(incrnonCoveringSelectivityThreshold); + +System.out.print("setupTableIndexes begins"); +Admin admin = MaprDBTestsSuite.getAdmin(); +if (admin != null) { + if (admin.tableExists(PRIMARY_TABLE_NAME)) { +
Re: Topics for Drill Hackathon/Drill Developers Day - 2018!
@All, I don’t know if remote folks can host a session, but if so, I’d volunteer. — C > On Oct 16, 2018, at 17:13, Vitalii Diravka wrote: > > Yes, I can edit and post suggestions in the document. > Thank you! > > On Tue, Oct 16, 2018 at 11:50 PM Hanumath Rao Maduri > wrote: > >> Hello Vitalli, >> >> I have given permissions to edit the document. Please let me know if it is >> fine. >> >> Regards, >> -Hanu >> >> On Tue, Oct 16, 2018 at 11:10 AM Vitalii Diravka >> wrote: >> >>> Could you provide the possibility of commenting for the document? >>> It will allow to make suggestions for the topics. >>> >>> On Tue, Oct 16, 2018 at 6:22 AM Hanumath Rao Maduri >>> wrote: >>> Hello Drill Development Team, Thank you all for the interest in attending the Drill Developers Day. I have curated a list of topics that can be discussed at the up-coming Drill Developers Day. Please feel free to suggest any other topics >> which you are interested in. Here is the link for the topics. >>> >> https://docs.google.com/document/d/1x9v_3UdENotONSuLm93hQJ-pDu1GS5tAhbXOaJrelsw/edit?usp=sharing Volunteers to lead the discussions are welcome. Please pick any topic >> of your interest to volunteer the discussion. Agenda and format for the discussions will be shared as we get closer >> to the event. We all are quite excited to meet you at the event. Thanks, -Hanu >>> >>
Re: [HANGOUT] Topics for 10/16/2018
Hi All, Thanks for attending the hangout. Meeting Minutes: - There was a question of using WebEx or Zoom to allow more people, but currently with new link we don't see any issue for number of people joining remotely so for now we will keep using hangout link. - Vitalli explained about Circle CI advantages compared to Travis. There was a proposal to look for getting paid version of Travis instead of switching to Circle CI for solving existing limitations. Pritesh will follow up on this. - We discussed about the topics for Drill Developer day and potential volunteers to lead individual topics. There was a proposal to send out the survey to know interests in individual topics from community. Hanu will follow up on this. Topics can be found here: https://docs.google.com/document/d/1x9v_3UdENotONSuLm93hQJ-pDu1GS5tAhbXOaJrelsw/edit?usp=sharing - There were list of topics which were identified to present in future hangout sessions. With each hangout invitation we will announce the topics which are finalized for that week. Thanks, Sorabh On Mon, Oct 15, 2018 at 11:05 AM Sorabh Hamirwasia wrote: > Resending with correct date (10/16) in subject. > > On Mon, Oct 15, 2018 at 10:59 AM Sorabh Hamirwasia > wrote: > >> Hi All, >> >> We will have Apache Drill Hangout tomorrow (October 16, 2018) at 10 AM >> PST. Please suggest any topic you will be interested in discussing by >> replying to this thread. We will also accept additional topics at the >> beginning of the hangout. >> >> Please use the below link to join: >> https://meet.google.com/yki-iqdf-tai >> >> Thanks, >> Sorabh >> >
[GitHub] amansinha100 commented on a change in pull request #1466: DRILL-6381: Add support for index based planning and execution
amansinha100 commented on a change in pull request #1466: DRILL-6381: Add support for index based planning and execution URL: https://github.com/apache/drill/pull/1466#discussion_r225721627 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/JoinControl.java ## @@ -0,0 +1,53 @@ +/* + * 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.common; + +/** + * For the int type control, + * the meaning of each bit start from lowest: + * bit 0: intersect or not, 0 -- default(no intersect), 1 -- INTERSECT (DISTINCT as default) + * bit 1: intersect type, 0 -- default (DISTINCT), 1 -- INTERSECT_ALL + */ +public class JoinControl { Review comment: @Ben-Zvi I have incorporated the joinControl logic as part of the hash join probe phase. Please see changes in [1]. I haven't done it for build phase since it will be superseded by the semi-join changes. Can you pls review this change ? [1] https://github.com/apache/drill/pull/1466/commits/5330fd684a20587e9e860d7894e3d3602a6d0495 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: us...@infra.apache.org With regards, Apache Git Services
Re: Topics for Drill Hackathon/Drill Developers Day - 2018!
Yes, I can edit and post suggestions in the document. Thank you! On Tue, Oct 16, 2018 at 11:50 PM Hanumath Rao Maduri wrote: > Hello Vitalli, > > I have given permissions to edit the document. Please let me know if it is > fine. > > Regards, > -Hanu > > On Tue, Oct 16, 2018 at 11:10 AM Vitalii Diravka > wrote: > > > Could you provide the possibility of commenting for the document? > > It will allow to make suggestions for the topics. > > > > On Tue, Oct 16, 2018 at 6:22 AM Hanumath Rao Maduri > > wrote: > > > > > Hello Drill Development Team, > > > > > > Thank you all for the interest in attending the Drill Developers Day. > > > I have curated a list of topics that can be discussed at the up-coming > > > Drill Developers Day. Please feel free to suggest any other topics > which > > > you are interested in. Here is the link for the topics. > > > > > > > > > > > > https://docs.google.com/document/d/1x9v_3UdENotONSuLm93hQJ-pDu1GS5tAhbXOaJrelsw/edit?usp=sharing > > > > > > Volunteers to lead the discussions are welcome. Please pick any topic > of > > > your interest to volunteer the discussion. > > > > > > Agenda and format for the discussions will be shared as we get closer > to > > > the event. > > > > > > We all are quite excited to meet you at the event. > > > > > > Thanks, > > > -Hanu > > > > > >
Re: Topics for Drill Hackathon/Drill Developers Day - 2018!
Hello Vitalli, I have given permissions to edit the document. Please let me know if it is fine. Regards, -Hanu On Tue, Oct 16, 2018 at 11:10 AM Vitalii Diravka wrote: > Could you provide the possibility of commenting for the document? > It will allow to make suggestions for the topics. > > On Tue, Oct 16, 2018 at 6:22 AM Hanumath Rao Maduri > wrote: > > > Hello Drill Development Team, > > > > Thank you all for the interest in attending the Drill Developers Day. > > I have curated a list of topics that can be discussed at the up-coming > > Drill Developers Day. Please feel free to suggest any other topics which > > you are interested in. Here is the link for the topics. > > > > > > > https://docs.google.com/document/d/1x9v_3UdENotONSuLm93hQJ-pDu1GS5tAhbXOaJrelsw/edit?usp=sharing > > > > Volunteers to lead the discussions are welcome. Please pick any topic of > > your interest to volunteer the discussion. > > > > Agenda and format for the discussions will be shared as we get closer to > > the event. > > > > We all are quite excited to meet you at the event. > > > > Thanks, > > -Hanu > > >
[GitHub] gparai commented on a change in pull request #1466: DRILL-6381: Add support for index based planning and execution
gparai commented on a change in pull request #1466: DRILL-6381: Add support for index based planning and execution URL: https://github.com/apache/drill/pull/1466#discussion_r225675265 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java ## @@ -114,6 +114,28 @@ public static final String UNIONALL_DISTRIBUTE_KEY = "planner.enable_unionall_distribute"; public static final BooleanValidator UNIONALL_DISTRIBUTE = new BooleanValidator(UNIONALL_DISTRIBUTE_KEY, null); + // --- Index planning related options BEGIN -- + public static final String USE_SIMPLE_OPTIMIZER_KEY = "planner.use_simple_optimizer"; + public static final BooleanValidator USE_SIMPLE_OPTIMIZER = new BooleanValidator(USE_SIMPLE_OPTIMIZER_KEY, null); + public static final BooleanValidator INDEX_PLANNING = new BooleanValidator("planner.enable_index_planning", null); + public static final BooleanValidator ENABLE_STATS = new BooleanValidator("planner.enable_statistics", null); Review comment: Yes, that was the intent - allow statistics use for any source with one option. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] priteshm commented on issue #1505: DRILL-6797: Fix UntypedNull handling for complex types
priteshm commented on issue #1505: DRILL-6797: Fix UntypedNull handling for complex types URL: https://github.com/apache/drill/pull/1505#issuecomment-430356449 @arina-ielchiieva should the title of PR to match the JIRA? 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: us...@infra.apache.org With regards, Apache Git Services
Re: Topics for Drill Hackathon/Drill Developers Day - 2018!
Could you provide the possibility of commenting for the document? It will allow to make suggestions for the topics. On Tue, Oct 16, 2018 at 6:22 AM Hanumath Rao Maduri wrote: > Hello Drill Development Team, > > Thank you all for the interest in attending the Drill Developers Day. > I have curated a list of topics that can be discussed at the up-coming > Drill Developers Day. Please feel free to suggest any other topics which > you are interested in. Here is the link for the topics. > > > https://docs.google.com/document/d/1x9v_3UdENotONSuLm93hQJ-pDu1GS5tAhbXOaJrelsw/edit?usp=sharing > > Volunteers to lead the discussions are welcome. Please pick any topic of > your interest to volunteer the discussion. > > Agenda and format for the discussions will be shared as we get closer to > the event. > > We all are quite excited to meet you at the event. > > Thanks, > -Hanu >
[GitHub] vdiravka commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9
vdiravka commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9 URL: https://github.com/apache/drill/pull/1503#discussion_r225618389 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java ## @@ -0,0 +1,110 @@ +/* + * 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.expr.fn; + +import java.io.Writer; +import java.util.List; + +import org.codehaus.janino.Java; +import org.codehaus.janino.Unparser; +import org.codehaus.janino.util.AutoIndentWriter; + +/** + * This is a modified version of {@link Unparser} so that we can avoid + * rendering few things. + */ +public class ModifiedUnparser extends Unparser { + + private String returnLabel; + + public ModifiedUnparser(Writer writer) { +super(writer); + } + + @Override + public void unparseBlockStatement(Java.BlockStatement blockStatement) { +// Unparser uses anonymous classes for visiting statements, +// therefore added this check for customizing of handling ReturnStatement. +if (blockStatement instanceof Java.ReturnStatement) { + visitReturnStatement((Java.ReturnStatement) blockStatement); +} else { + super.unparseBlockStatement(blockStatement); +} + } + + /** + * Parses specified {@link Java.MethodDeclarator}, wraps its content + * with replaced {@code return} statements by {@code break} ones into the + * block with label and stores it into {@link java.io.PrintWriter}. + * + * @param methodDeclarator method to parse + */ + public void visitMethodDeclarator(Java.MethodDeclarator methodDeclarator) { +if (methodDeclarator.optionalStatements == null) { + pw.print(';'); +} else if (methodDeclarator.optionalStatements.isEmpty()) { + pw.print(" {}"); +} else { + pw.println(' '); + // Add labels to handle return statements within function templates + String[] fQCN = methodDeclarator.getDeclaringType().getClassName().split("\\."); + returnLabel = fQCN[fQCN.length - 1] + "_" + methodDeclarator.name; + pw.print(returnLabel); + pw.println(": {"); + pw.print(AutoIndentWriter.INDENT); + unparseStatements(methodDeclarator.optionalStatements); + pw.print(AutoIndentWriter.UNINDENT); + pw.println("}"); + pw.print(' '); +} + } + + private void visitReturnStatement(Java.ReturnStatement returnStatement) { Review comment: Thanks for creating the ticket. Agree, the current solution is acceptable. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] arina-ielchiieva commented on issue #1502: DRILL-6785: DataClient is using RootAllocator in the bootstrap instea…
arina-ielchiieva commented on issue #1502: DRILL-6785: DataClient is using RootAllocator in the bootstrap instea… URL: https://github.com/apache/drill/pull/1502#issuecomment-430300490 Sounds good, +1 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9
vvysotskyi commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9 URL: https://github.com/apache/drill/pull/1503#discussion_r225607534 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java ## @@ -0,0 +1,110 @@ +/* + * 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.expr.fn; + +import java.io.Writer; +import java.util.List; + +import org.codehaus.janino.Java; +import org.codehaus.janino.Unparser; +import org.codehaus.janino.util.AutoIndentWriter; + +/** + * This is a modified version of {@link Unparser} so that we can avoid + * rendering few things. + */ +public class ModifiedUnparser extends Unparser { + + private String returnLabel; + + public ModifiedUnparser(Writer writer) { +super(writer); + } + + @Override + public void unparseBlockStatement(Java.BlockStatement blockStatement) { +// Unparser uses anonymous classes for visiting statements, +// therefore added this check for customizing of handling ReturnStatement. +if (blockStatement instanceof Java.ReturnStatement) { + visitReturnStatement((Java.ReturnStatement) blockStatement); +} else { + super.unparseBlockStatement(blockStatement); +} + } + + /** + * Parses specified {@link Java.MethodDeclarator}, wraps its content + * with replaced {@code return} statements by {@code break} ones into the + * block with label and stores it into {@link java.io.PrintWriter}. + * + * @param methodDeclarator method to parse + */ + public void visitMethodDeclarator(Java.MethodDeclarator methodDeclarator) { +if (methodDeclarator.optionalStatements == null) { + pw.print(';'); +} else if (methodDeclarator.optionalStatements.isEmpty()) { + pw.print(" {}"); +} else { + pw.println(' '); + // Add labels to handle return statements within function templates + String[] fQCN = methodDeclarator.getDeclaringType().getClassName().split("\\."); + returnLabel = fQCN[fQCN.length - 1] + "_" + methodDeclarator.name; + pw.print(returnLabel); + pw.println(": {"); + pw.print(AutoIndentWriter.INDENT); + unparseStatements(methodDeclarator.optionalStatements); + pw.print(AutoIndentWriter.UNINDENT); + pw.println("}"); + pw.print(' '); +} + } + + private void visitReturnStatement(Java.ReturnStatement returnStatement) { Review comment: Agree, created https://github.com/janino-compiler/janino/issues/61 for this. But anyway, I think this is not a blocker for this PR. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9
vvysotskyi commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9 URL: https://github.com/apache/drill/pull/1503#discussion_r225586948 ## File path: exec/java-exec/pom.xml ## @@ -349,6 +349,14 @@ joda-time 2.9 + Review comment: Currently, Drill has only transitive dependencies to `janino` and `commons-compiler`, but their classes are used in Drill code, so specified direct dependencies to avoid relying on transitive ones. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9
vvysotskyi commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9 URL: https://github.com/apache/drill/pull/1503#discussion_r225587554 ## File path: pom.xml ## @@ -1366,6 +1366,11 @@ janino ${janino.version} + +org.codehaus.janino +commons-compiler Review comment: Yes, you are correct, it was a transitive dependency earlier. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on issue #1502: DRILL-6785: DataClient is using RootAllocator in the bootstrap instea…
sohami commented on issue #1502: DRILL-6785: DataClient is using RootAllocator in the bootstrap instea… URL: https://github.com/apache/drill/pull/1502#issuecomment-430293134 @arina-ielchiieva - There was no issues which was caused/found because of this. I happen to notice it while browsing code so made the change for correctness. 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: us...@infra.apache.org With regards, Apache Git Services
Re: [DISCUSSION] CI for Drill
Hi all, DRILL-6777 is merged to Apache Drill master branch. To set-up CircleCI builds for your Drill fork do the following steps: 1. SignUp into https://circleci.com/dashboard with your GitHub account. 2. Give permissions 3. Choose the organization (your GitHub account) 4. Choose the project (your Drill fork) 5. Start building (make sure that your master branch is updated) INFRA revises the possibility of providing CircleCI for Apache and Drill itself [1]. [1] https://issues.apache.org/jira/browse/INFRA-17133?focusedCommentId=16648674=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16648674 On Fri, Oct 12, 2018 at 1:42 PM Vitalii Diravka wrote: > The CircleCI + GitHub integration as simple as it's shown in [1]. > Just need to click "Start Building Now" and to proceed with "Sign Up with > GitHub". > You can find more complicated tuning in [2]. > > Once there will be an update from INFRA, I will inform this thread. > > > [1] https://circleci.com/integrations/github/ > [2] https://circleci.com/docs/2.0/gh-bb-integration/ > > On Fri, Oct 12, 2018 at 1:15 PM Arina Yelchiyeva < > arina.yelchiy...@gmail.com> wrote: > >> Vitalii, >> >> in this case I think it's ok to merge CircleCI configs. Could you please >> share how to setup CircleCI for custom builds? >> Also could you follow up with INFRA ticket there would be any response? >> >> Kind regards, >> Arina >> >> On Thu, Oct 11, 2018 at 2:20 PM Vitalii Diravka >> wrote: >> >> > Hi all! >> > >> > I have opened PR with adding CircleCI configs for Drill build [1]. >> > And the ticket [2] for INFRA to setup CircleCI for ApacheDrill. >> > But then I've noticed that INFRA can't allow write access for 3d party >> > (Apache Arrow + CircleCI [3]). >> > So here are two ways: >> > * to merge it, then CircleCI builds will work for Drill forks only. >> > * try to help INFRA to enable CircleCI for Apache Drill main repo via >> > configuring CircleCI webhooks [4] >> > >> > I think we can proceed with both of them, since even just to merge >> > .circleci to the Drill will be useful for the forks >> > of committers and contributors (like in the Apache Cassandra [5]). >> > Thoughts? >> > >> > [1] https://github.com/apache/drill/pull/1493 >> > [2] https://issues.apache.org/jira/browse/INFRA-17133 >> > [3] >> > >> > >> https://issues.apache.org/jira/browse/INFRA-15964?focusedCommentId=16351422=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16351422 >> > [4] >> > >> > >> https://issues.apache.org/jira/browse/INFRA-12197?focusedCommentId=15652850=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15652850 >> > [5] https://github.com/apache/cassandra/tree/trunk/.circleci >> > >> > On Wed, Sep 12, 2018 at 3:41 PM Vitalii Diravka < >> vitalii.dira...@gmail.com >> > > >> > wrote: >> > >> > > The current issue with CircleCI is default RAM limit of medium >> (default) >> > > instance for Docker images - 4Gb [1]. >> > > It can be expanded by using VM instead of Docker image or possibly >> > > CircleCI team can provide us bigger instance for it [2] >> > > >> > > I have created the Jira ticket for it [3]. Further discussion can be >> > > continued there. >> > > >> > > [1] >> > https://circleci.com/docs/2.0/configuration-reference/#resource_class >> > > [2] https://circleci.com/pricing/ >> > > [3] https://issues.apache.org/jira/browse/DRILL-6741 >> > > >> > > Kind regards >> > > Vitalii >> > > >> > > >> > > On Wed, Sep 12, 2018 at 12:27 PM Arina Yelchiyeva < >> > > arina.yelchiy...@gmail.com> wrote: >> > > >> > >> +1, especially if other Apache project uses it, there should not be >> any >> > >> issues with Apache. >> > >> >> > >> Kind regards, >> > >> Arina >> > >> >> > >> On Wed, Sep 12, 2018 at 12:36 AM Timothy Farkas >> > wrote: >> > >> >> > >> > +1 For trying out Circle CI. I've used it in the past, and I think >> the >> > >> UI >> > >> > is much better than Travis. >> > >> > >> > >> > Tim >> > >> > >> > >> > On Tue, Sep 11, 2018 at 8:21 AM Vitalii Diravka < >> > >> vitalii.dira...@gmail.com >> > >> > > >> > >> > wrote: >> > >> > >> > >> > > Recently we discussed Travis build failures and there were >> excluded >> > >> more >> > >> > > tests to make Travis happy [1]. But looks like the issue returned >> > back >> > >> > and >> > >> > > Travis build fails intermittently. >> > >> > > >> > >> > > I tried to find other solution instead of exclusion Drill unit >> tests >> > >> and >> > >> > > found other good CI - CircleCI [2]. Looks like this CI will >> allow to >> > >> run >> > >> > > all unit tests successfully. >> > >> > > And it offers good conditions for open-source projects [3] (even >> OS >> > X >> > >> > > environment is available). >> > >> > > The example of Apache project, which uses this CI is Apache >> > Cassandra >> > >> [4] >> > >> > > >> > >> > > My quick set-up of CircleCI for Drill still fails, but it should >> be >> > >> just >> > >> > > configured properly [5]. >> > >> > >
[GitHub] arina-ielchiieva opened a new pull request #1505: DRILL-6797: Fix UntypedNull handling for complex types
arina-ielchiieva opened a new pull request #1505: DRILL-6797: Fix UntypedNull handling for complex types URL: https://github.com/apache/drill/pull/1505 Details in [DRILL-6797](https://issues.apache.org/jira/browse/DRILL-6797). 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9
vdiravka commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9 URL: https://github.com/apache/drill/pull/1503#discussion_r225515750 ## File path: pom.xml ## @@ -1366,6 +1366,11 @@ janino ${janino.version} + +org.codehaus.janino +commons-compiler Review comment: Just for clarification, it was a Transitive Dependency earlier, wasn't it? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9
vdiravka commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9 URL: https://github.com/apache/drill/pull/1503#discussion_r225005654 ## File path: exec/java-exec/pom.xml ## @@ -349,6 +349,14 @@ joda-time 2.9 + Review comment: Why it is necessary in `java-exec` for now? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9
vdiravka commented on a change in pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.9 URL: https://github.com/apache/drill/pull/1503#discussion_r225579834 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java ## @@ -0,0 +1,110 @@ +/* + * 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.expr.fn; + +import java.io.Writer; +import java.util.List; + +import org.codehaus.janino.Java; +import org.codehaus.janino.Unparser; +import org.codehaus.janino.util.AutoIndentWriter; + +/** + * This is a modified version of {@link Unparser} so that we can avoid + * rendering few things. + */ +public class ModifiedUnparser extends Unparser { + + private String returnLabel; + + public ModifiedUnparser(Writer writer) { +super(writer); + } + + @Override + public void unparseBlockStatement(Java.BlockStatement blockStatement) { +// Unparser uses anonymous classes for visiting statements, +// therefore added this check for customizing of handling ReturnStatement. +if (blockStatement instanceof Java.ReturnStatement) { + visitReturnStatement((Java.ReturnStatement) blockStatement); +} else { + super.unparseBlockStatement(blockStatement); +} + } + + /** + * Parses specified {@link Java.MethodDeclarator}, wraps its content + * with replaced {@code return} statements by {@code break} ones into the + * block with label and stores it into {@link java.io.PrintWriter}. + * + * @param methodDeclarator method to parse + */ + public void visitMethodDeclarator(Java.MethodDeclarator methodDeclarator) { +if (methodDeclarator.optionalStatements == null) { + pw.print(';'); +} else if (methodDeclarator.optionalStatements.isEmpty()) { + pw.print(" {}"); +} else { + pw.println(' '); + // Add labels to handle return statements within function templates + String[] fQCN = methodDeclarator.getDeclaringType().getClassName().split("\\."); + returnLabel = fQCN[fQCN.length - 1] + "_" + methodDeclarator.name; + pw.print(returnLabel); + pw.println(": {"); + pw.print(AutoIndentWriter.INDENT); + unparseStatements(methodDeclarator.optionalStatements); + pw.print(AutoIndentWriter.UNINDENT); + pw.println("}"); + pw.print(' '); +} + } + + private void visitReturnStatement(Java.ReturnStatement returnStatement) { Review comment: It is a potential ticket for improvement in Janino, to allow usage of default implementations of visitor's methods and possibility to override it, if it is necessary. Once it will be done in Janino, the overridden `unparseBlockStatement()` method could be removed. Do you agree? 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Created] (DRILL-6797) Split function index extraction fails when result has no rows
Arina Ielchiieva created DRILL-6797: --- Summary: Split function index extraction fails when result has no rows Key: DRILL-6797 URL: https://issues.apache.org/jira/browse/DRILL-6797 Project: Apache Drill Issue Type: Bug Affects Versions: 1.12.0 Reporter: Arina Ielchiieva Assignee: Arina Ielchiieva Fix For: 1.15.0 *Query:* select split(n_name, ' ') [1] from cp.`tpch/nation.parquet` where n_nationkey = -1 group by n_name order by n_name limit 10 *Error:* {noformat} Error: SYSTEM ERROR: CompileException: Line 23, Column 35: No applicable constructor/method found for actual parameters "int, org.apache.drill.exec.vector.UntypedNullHolder"; candidates are: "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(int, org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(int, org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.TinyIntReader.read(org.apache.drill.exec.expr.holders.TinyIntHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.TinyIntReader.read(org.apache.drill.exec.expr.holders.NullableTinyIntHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(int, org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.UInt1Reader.read(org.apache.drill.exec.expr.holders.UInt1Holder)", "public abstract void org.apache.drill.exec.vector.complex.reader.UInt1Reader.read(org.apache.drill.exec.expr.holders.NullableUInt1Holder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(int, org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.UInt2Reader.read(org.apache.drill.exec.expr.holders.UInt2Holder)", "public abstract void org.apache.drill.exec.vector.complex.reader.UInt2Reader.read(org.apache.drill.exec.expr.holders.NullableUInt2Holder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(int, org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.SmallIntReader.read(org.apache.drill.exec.expr.holders.SmallIntHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.SmallIntReader.read(org.apache.drill.exec.expr.holders.NullableSmallIntHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(int, org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.IntReader.read(org.apache.drill.exec.expr.holders.IntHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.IntReader.read(org.apache.drill.exec.expr.holders.NullableIntHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(int, org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.UInt4Reader.read(org.apache.drill.exec.expr.holders.UInt4Holder)", "public abstract void org.apache.drill.exec.vector.complex.reader.UInt4Reader.read(org.apache.drill.exec.expr.holders.NullableUInt4Holder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(int, org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.BaseReader.read(org.apache.drill.exec.expr.holders.UnionHolder)", "public abstract void org.apache.drill.exec.vector.complex.reader.Float4Reader.read(org.apache.drill.exec.expr.holders.Float4Holder)", "public abstract void org.apache.drill.exec.vector.complex.reade . ... ... exec.expr.holders.UnionHolder)", "public abstract void
[GitHub] ihuzenko edited a comment on issue #1488: DRILL-786: Allow CROSS JOIN syntax
ihuzenko edited a comment on issue #1488: DRILL-786: Allow CROSS JOIN syntax URL: https://github.com/apache/drill/pull/1488#issuecomment-430249450 > @ihuzenko I have a minor comment about the error message (sorry I just noticed it when I was about to merge your PR). If you can address it, I will do the merge soon after. Hi @amansinha100 , no problems. I've fixed the issue, could you please take a look ? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ihuzenko commented on issue #1488: DRILL-786: Allow CROSS JOIN syntax
ihuzenko commented on issue #1488: DRILL-786: Allow CROSS JOIN syntax URL: https://github.com/apache/drill/pull/1488#issuecomment-430249450 > @ihuzenko I have a minor comment about the error message (sorry I just noticed it when I was about to merge your PR). If you can address it, I will do the merge soon after. Hi, no problems. I've fixed the issue, could you please take a look ? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ihuzenko commented on a change in pull request #1488: DRILL-786: Allow CROSS JOIN syntax
ihuzenko commented on a change in pull request #1488: DRILL-786: Allow CROSS JOIN syntax URL: https://github.com/apache/drill/pull/1488#discussion_r225487564 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java ## @@ -65,6 +68,11 @@ } private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(JoinUtils.class); + public static final String FAILED_TO_PLAN_CARTESIAN_JOIN = String.format( + "This query cannot be planned possibly due to either a cartesian join or an inequality join. " + Review comment: done 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225504704 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java ## @@ -87,6 +95,8 @@ private LinkedList[] blocks; private LinkedList[] oldBlocks; + private JVar innerClassField; + /** * Assumed that field has 3 indexes within the constant pull: index of the CONSTANT_Fieldref_info + Review comment: Please note that with introducing new methods in the class, this formula should be revised to take them into account. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225494095 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java ## @@ -531,11 +541,42 @@ public JVar declareClassField(String prefix, JType t) { public JVar declareClassField(String prefix, JType t, JExpression init) { if (innerClassGenerator != null && hasMaxIndexValue()) { - return innerClassGenerator.clazz.field(JMod.NONE, t, prefix + index++, init); + return innerClassGenerator.declareClassField(prefix, t, init); } return clazz.field(JMod.NONE, t, prefix + index++, init); } + /** + * declare a setter method for the argument {@code var}. + * argument {@code function} holds the constant value which + * returns a value holder must be invoked when the class instance created. + * the setter method of innerClassField will be invoked if innerClassGenerator exists. + * + * @param var the class member variable + * @param function the function holds the constant value + * @return the depth of nested class, setter method + */ + public Pair declareSetterMethod(JVar var, Function function) { +JMethod setter = clazz.method(JMod.PUBLIC, void.class, "set" + StringUtils.capitalize(var.name())); Review comment: Please move the initialization to the else block below and put code which uses this var after the `if...else` block. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225500775 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/CodeGenMemberInjector.java ## @@ -0,0 +1,79 @@ +/* + * 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.physical.impl.common; + +import com.sun.codemodel.JMethod; +import io.netty.buffer.DrillBuf; +import org.apache.calcite.util.Pair; +import org.apache.drill.exec.expr.ClassGenerator; +import org.apache.drill.exec.expr.holders.ValueHolder; +import org.apache.drill.exec.ops.FragmentContext; +import org.apache.drill.shaded.guava.com.google.common.base.Function; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; + +public class CodeGenMemberInjector { + + /** + * Generated code for a class may have several class members, they + * are initialized by invoking this method when the instance created. + * + * @param cg the class generator + * @param instance the class instance created by the compiler + * @param context the fragment context + */ + public static void injectMembers(ClassGenerator cg, Object instance, FragmentContext context) { +Map cachedInstances = new HashMap<>(); +for (Map.Entry, Function> setter : cg.getSetters().entrySet()) { Review comment: We may change the type of collection which used for storing setter methods. Can be used Guava's Table, so we will be able to omit usage of `cachedInstances` map. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225498265 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java ## @@ -646,7 +687,7 @@ public void preparePlainJava() { Class p = params[i]; childNew.arg(shim.param(model._ref(p), "arg" + i)); } - shim.body()._return(childNew); + shim.body()._return(JExpr._this().invoke("injectMembers").arg(childNew)); Review comment: Could you please explain what is the reason for this change? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
vvysotskyi commented on a change in pull request #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#discussion_r225499955 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java ## @@ -1074,7 +1078,12 @@ private void addBatchHolder(int part, int batchRowCount) { // These methods are overridden in the generated class when created as plain Java code. protected BatchHolder newBatchHolder(int batchRowCount) { -return new BatchHolder(batchRowCount); +return this.injectMembers(new BatchHolder(batchRowCount)); + } + + protected BatchHolder injectMembers(BatchHolder batchHolder) { +CodeGenMemberInjector.injectMembers(cg, batchHolder, context); Review comment: Is it possible to move this method call to the place, general for other similar classes? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] arina-ielchiieva commented on issue #1502: DRILL-6785: DataClient is using RootAllocator in the bootstrap instea…
arina-ielchiieva commented on issue #1502: DRILL-6785: DataClient is using RootAllocator in the bootstrap instea… URL: https://github.com/apache/drill/pull/1502#issuecomment-430195019 @sohami the change seems obvious but could you please highlight what incorrect usage causes and how issue was found? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-430141004 @vvysotskyi All tests have been passed, could you please take a look? thanks! 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: us...@infra.apache.org With regards, Apache Git Services