[GitHub] drill pull request: DRILL-4510: Revert DRILL-4476 since UnionAllRe...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/498 DRILL-4510: Revert DRILL-4476 since UnionAllRecordBatch cannot determ⦠â¦ine if a given record batch is from an empty file or not, just according to the number of rows You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4510 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/498.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #498 commit acb00f3a545e2db578aeb2dc14b0180858dd9a5f Author: hsuanyi Date: 2016-05-04T22:56:40Z DRILL-4510: Revert DRILL-4476 since UnionAllRecordBatch cannot determine if a given record batch is from an empty file or not, just according to the number of rows --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Drill 3764 preview
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/497 Drill 3764 preview The feature for skip records. This PR is just for the record. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-3764_Preview Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/497.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #497 commit 6ce1895444d7b5cedef2768a96f82fa85e75577c Author: Hsuan-Yi Chu Date: 2015-10-07T04:15:27Z DRILL-3764: CSV, JSON, PARQUET commit cce3aa04138a3d1dbda5af3241f2f9caa70751aa Author: Hsuan-Yi Chu Date: 2015-10-07T04:15:27Z DRILL-3764: Project, Filter commit 3e7230a2c04a48543f2550259d4741f3327aa60a Author: Hsuan-Yi Chu Date: 2016-02-16T01:32:36Z Debug commit a8e3ee647f0192a5153e73d38ea9e379bd5aae0b Author: Hsuan-Yi Chu Date: 2016-02-18T02:50:24Z DRILL-3764: Hbase commit f374a7b1e9019870853e7edc16c5b528338e6d27 Author: Hsuan-Yi Chu Date: 2016-02-18T03:09:41Z DRILL-3764: Hive commit 968118bc036c34a9339c879edb6d0032c7ea8770 Author: hsuanyi Date: 2016-05-02T22:49:17Z [Runtime Code] Improvement: The representation of error codes is simplified as ArrayList of integers where each element is an integer error code corresponding to different types of function failures. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4642: Remove customized RexBuilder.ensur...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/489#discussion_r62118608
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java
---
@@ -734,6 +772,41 @@ public static FunctionCall
convertSqlOperatorBindingToFunctionCall(final SqlOper
}
/**
+ * Based on whether decimal type is supported or not, this method
creates an ExplicitOperatorBinding which interprets
+ * the type of decimal literals accordingly.
+ */
+ public static SqlOperatorBinding convertDecimalLiteralToDouble(final
SqlOperatorBinding sqlOperatorBinding, final boolean isDecimalSupported) {
+final List types = Lists.newArrayList();
+for(int i = 0; i < sqlOperatorBinding.getOperandCount(); ++i) {
+ final RelDataType relDataType;
+ if(isDecimalLiteral(sqlOperatorBinding, i) && !isDecimalSupported) {
+relDataType = createCalciteTypeWithNullability(
+sqlOperatorBinding.getTypeFactory(),
+SqlTypeName.DOUBLE,
+sqlOperatorBinding.getOperandType(i).isNullable());
+ } else {
+relDataType = sqlOperatorBinding.getOperandType(i);
+ }
+ types.add(relDataType);
+}
+return new
ExplicitOperatorBinding(sqlOperatorBinding.getTypeFactory(),
sqlOperatorBinding.getOperator(), types);
--- End diff --
addressed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4642: Remove customized RexBuilder.ensur...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/489#discussion_r62117749
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
---
@@ -719,4 +727,73 @@ public void testWindowSumConstant() throws Exception {
final String[] excludedPlan = {};
PlanTestBase.testPlanMatchingPatterns(query, expectedPlan,
excludedPlan);
}
+
+ @Test // DRILL-4552
+ public void testDecimalPlusWhenDecimalEnabled() throws Exception {
+final String query = "select cast('99' as decimal(9,0)) + cast('99' as
decimal(9,0)) as col \n" +
+"from cp.`tpch/region.parquet` \n" +
+"limit 0";
+
+try {
+ final TypeProtos.MajorType majorTypeDouble =
TypeProtos.MajorType.newBuilder()
+ .setMinorType(TypeProtos.MinorType.FLOAT8)
+ .setMode(TypeProtos.DataMode.REQUIRED)
+ .build();
+
+ final List>
expectedSchemaDouble = Lists.newArrayList();
+ expectedSchemaDouble.add(Pair.of(SchemaPath.getSimplePath("col"),
majorTypeDouble));
+
+ testBuilder()
--- End diff --
Addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4642: Remove customized RexBuilder.ensur...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/489#discussion_r62117505
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
---
@@ -213,7 +221,7 @@ public void tesIsNull() throws Exception {
List> expectedSchema =
Lists.newArrayList();
TypeProtos.MajorType majorType = TypeProtos.MajorType.newBuilder()
.setMinorType(TypeProtos.MinorType.BIT)
-.setMode(TypeProtos.DataMode.REQUIRED)
+.setMode(TypeProtos.DataMode.OPTIONAL)
--- End diff --
In fact, I meant to let the tests in this test-suite run with Limit-0
optimization. However, that option is off by default. So I added
https://github.com/apache/drill/pull/489/files#diff-0e0d4b8f1e54d2a7e8b6bc04d8606bf7R35
However, that exposed an issue here:
https://issues.apache.org/jira/browse/DRILL-4656
So I think we should firstly turn this test off until we fix DRILL-4656
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/439#issuecomment-216920059 merge this PR with https://github.com/apache/drill/pull/489. So close this one. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
Github user hsuanyi closed the pull request at: https://github.com/apache/drill/pull/439 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3823, DRILL-4507: Add unit tests.
Github user hsuanyi closed the pull request at: https://github.com/apache/drill/pull/490 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3823, DRILL-4507: Add unit tests.
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/490#issuecomment-216919724 merge this PR with https://github.com/apache/drill/pull/489. So close this one. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4573: Zero copy LIKE, REGEXP_MATCHES, SU...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/458#issuecomment-215801663 Hi @jcmcote, I see. But can you do us a favor. Regex_replace()'s behavior changes after your patch https://issues.apache.org/jira/browse/DRILL-4645 Can you help fix this? And would you mind merging the missing piece with the new patch? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3823, DRILL-4507: Add unit tests.
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/490 DRILL-3823, DRILL-4507: Add unit tests. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-3823 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/490.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #490 commit 0ba6056baad9981088195c7809fd0abb93a29c94 Author: hsuanyi Date: 2016-04-29T03:58:25Z DRILL-3823, DRILL-4507: Add unit tests. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4642: Remove customized RexBuilder.ensur...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/489#issuecomment-215490663 @jinfengni Can you help review? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4642: Remove customized RexBuilder.ensur...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/489#issuecomment-215490190 DrillCalcite can be found at here: https://github.com/hsuanyi/incubator-calcite/tree/DRILL-4642 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4642: Remove customized RexBuilder.ensur...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/489 DRILL-4642: Remove customized RexBuilder.ensureType() Bump calcite version to 1.4.0-drill-r12 You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4642 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/489.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #489 commit 163e7b713fbb1a21603f4c1de77acd0481088158 Author: Hsuan-Yi Chu Date: 2016-04-25T22:09:01Z DRILL-4642: Remove customized RexBuilder.ensureType() Bump calcite version to 1.4.0-drill-r12 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4573: Zero copy LIKE, REGEXP_MATCHES, SU...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/458#issuecomment-214929161 Just pushed to master; Thanks for the contributions. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4573: Zero copy LIKE, REGEXP_MATCHES, SU...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/458#issuecomment-214905327 Yes, it passed the tests. I will push it shortly. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4573: Zero copy LIKE, REGEXP_MATCHES, SU...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/458#issuecomment-213650898 @jcmcote Sorry for the delay. Let me run some tests and commit it if it passes the tests. Your contribution is highly appreciated. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4577: Construct a specific path for quer...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/461#issuecomment-210144139 In the background, Chun, Rahul and Jinfeng have done some experiments: 1. Functional-wise: If Hive uses storage-based impersonation, when a user does "Show Tables" on Hive (without going through DRILL), this user will see the tables which might not be under his/her permission. So from the aspect, the behavior of Drill matches Hive with respect to visibility of tables displayed. 2. Performance-wise: For INFORMATION_SCHEMA query against a hive instance with 1000 tables (impersonation was off), we got 38 seconds when the patch is not applied (non bulk load), and 8 seconds is when the patch is applied (bulk load). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4589: Reduce planning time for file syst...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/468#issuecomment-207904379 LGTM +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4573: Zero copy LIKE, REGEXP_MATCHES, SU...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/458#issuecomment-207904016 And can you share the details regarding the performance improvement? For example, how the setting and data look like? Observed % of improvements? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4573: Zero copy LIKE, REGEXP_MATCHES, SU...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/458#issuecomment-207903918 Except for the comments, LGTM. Do you run the unit test ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4573: Zero copy LIKE, REGEXP_MATCHES, SU...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/458#discussion_r59125828
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSequenceWrapper.java
---
@@ -0,0 +1,49 @@
+/**
+ * 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.impl;
+
+import io.netty.buffer.DrillBuf;
+
+public class CharSequenceWrapper implements CharSequence {
--- End diff --
Can you add java doc for this class and the method setBuffer()?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4573: Zero copy LIKE, REGEXP_MATCHES, SU...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/458#discussion_r59125807
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSequenceWrapper.java
---
@@ -0,0 +1,49 @@
+/**
+ * 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.impl;
+
+import io.netty.buffer.DrillBuf;
+
+public class CharSequenceWrapper implements CharSequence {
+
+private int start;
+private int end;
+private DrillBuf buffer;
+
+@Override
+public int length() {
+return end - start;
+}
+
+@Override
+public char charAt(int index) {
+return (char) buffer.getByte(start + index);
--- End diff --
We might need to check if the index falls into the legal bounds.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4577: Construct a specific path for quer...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/461#discussion_r58986459
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
---
@@ -72,4 +80,76 @@ public String getTypeName() {
return HiveStoragePluginConfig.NAME;
}
+ @Override
+ public List> getTablesByNames(final
List tableNames) {
+final String schemaName = getName();
+final List> tableNameToTable =
Lists.newArrayList();
+List tables;
+// Retries once if the first call to fetch the metadata fails
+synchronized(mClient) {
+ final List tableNamesWithAuth = Lists.newArrayList();
+ for(String tableName : tableNames) {
+try {
+ if(mClient.tableExists(schemaName, tableName)) {
--- End diff --
We have some discussions in the background. So let me summarize them below:
Here are some findings:
There are two ways for authorization, "SQL Standard Based Authorization"
and "Storage Based Authorization" [1]. When "Storage Based Authorization" is
being used, and we use "getTableObjectsByName", the returned results will
display some tables, which are not authorized to this user.
The reason is when "getTableObjectsByName" is being used, hive meta storage
will return the tables which are not authorized to the users. Since the issue
is in Hive, currently I do not think there is a simple fix.
On the other hand, "SQL Standard Based Authorization" would not have this
issue.
[1]
https://drill.apache.org/docs/configuring-user-impersonation-with-hive-authorization/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4589: Reduce planning time for file syst...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/468#discussion_r58824977
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/DFSDirPartitionLocation.java
---
@@ -0,0 +1,70 @@
+/**
+ * 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.
+ */
+
+/**
+ * Class defines a single partition corresponding to a directory in a DFS
table.
+ */
+package org.apache.drill.exec.planner;
+
+
+import com.google.common.collect.Lists;
+
+import java.util.Collection;
+import java.util.List;
+
+public class DFSDirPartitionLocation implements PartitionLocation {
+ private final Collection subPartitions;
+ private final String[] dirs;
+
+ public DFSDirPartitionLocation(String[] dirs,
Collection subPartitions) {
+this.subPartitions = subPartitions;
+this.dirs = dirs;
+ }
+
+ @Override
+ public String getPartitionValue(int index) {
+assert index < dirs.length;
--- End diff --
I think the next line will throw IOOB if this line is not satisfied.
(But this is minor thing).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4589: Reduce planning time for file syst...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/468#discussion_r58824883
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/FileSystemPartitionDescriptor.java
---
@@ -148,13 +139,41 @@ public String getName(int index) {
return partitionLabel + index;
}
- private String getBaseTableLocation() {
+ protected String getBaseTableLocation() {
--- End diff --
I do not find this method being used outside this class. Is it intentional?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4577: Construct a specific path for quer...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/461#discussion_r58805242
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
---
@@ -72,4 +80,76 @@ public String getTypeName() {
return HiveStoragePluginConfig.NAME;
}
+ @Override
+ public List> getTablesByNames(final
List tableNames) {
+final String schemaName = getName();
+final List> tableNameToTable =
Lists.newArrayList();
+List tables;
+// Retries once if the first call to fetch the metadata fails
+synchronized(mClient) {
+ final List tableNamesWithAuth = Lists.newArrayList();
+ for(String tableName : tableNames) {
+try {
+ if(mClient.tableExists(schemaName, tableName)) {
--- End diff --
I did some tests here. When there are many tables, the improvement by
optimizing for the second objective is not significant enough. However, the
objective of this issue would make sense only when there are many tables. I
think I still need to figure out a solution.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4577: Construct a specific path for quer...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/461#discussion_r58758201
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
---
@@ -72,4 +80,76 @@ public String getTypeName() {
return HiveStoragePluginConfig.NAME;
}
+ @Override
+ public List> getTablesByNames(final
List tableNames) {
+final String schemaName = getName();
+final List> tableNameToTable =
Lists.newArrayList();
+List tables;
+// Retries once if the first call to fetch the metadata fails
+synchronized(mClient) {
+ final List tableNamesWithAuth = Lists.newArrayList();
+ for(String tableName : tableNames) {
+try {
+ if(mClient.tableExists(schemaName, tableName)) {
--- End diff --
Of course, eliminating the first one is important too. I am still
investigating that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4577: Construct a specific path for quer...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/461#discussion_r58758011
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
---
@@ -72,4 +80,76 @@ public String getTypeName() {
return HiveStoragePluginConfig.NAME;
}
+ @Override
+ public List> getTablesByNames(final
List tableNames) {
+final String schemaName = getName();
+final List> tableNameToTable =
Lists.newArrayList();
+List tables;
+// Retries once if the first call to fetch the metadata fails
+synchronized(mClient) {
+ final List tableNamesWithAuth = Lists.newArrayList();
+ for(String tableName : tableNames) {
+try {
+ if(mClient.tableExists(schemaName, tableName)) {
--- End diff --
There are two parts which makes the query slow.
One follows from your point; The other is fetching partitions which turned
out not used. [1]
[1]
https://github.com/apache/drill/blob/245da9790813569c5da9404e0fc5e45cc88e22bb/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java#L236
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4577: Construct a specific path for quer...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/461#discussion_r58751518
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
---
@@ -72,4 +80,56 @@ public String getTypeName() {
return HiveStoragePluginConfig.NAME;
}
+ @Override
+ public void visitTables(final RecordGenerator recordGenerator, final
String schemaPath) {
+final List tableNames = Lists.newArrayList(getTableNames());
+List tables;
+// Retries once if the first call to fetch the metadata fails
+synchronized(mClient) {
+ try {
+tables = mClient.getTableObjectsByName(getName(), tableNames);
--- End diff --
@vkorukanti Thanks for pointing this out. Regardless of the permission,
getTableObjectsByName will return the requested tables. Thus, as in [1], I used
mClient.tableExists() to check the permission.
[1]https://github.com/apache/drill/pull/461/files#diff-bb5d8a385888df1dacc85fc011acd94bR93
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4577: Construct a specific path for quer...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/461#discussion_r58750954
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractSchema.java ---
@@ -194,4 +195,26 @@ public void dropTable(String tableName) {
.message("Dropping tables is not supported in schema [%s]",
getSchemaPath())
.build(logger);
}
-}
+
+ /**
+ * Visit the tables in this schema and write to recordGenerator
+ * @param recordGenerator recordGenerator for the output
+ * @param schemaPath the path to the given schema
+ */
--- End diff --
I restructured the code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4577: Construct a specific path for quer...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/461 DRILL-4577: Construct a specific path for querying all the tables fro⦠â¦m a hive database @vkorukanti Can you help review? Thanks. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4577 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/461.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #461 commit 135464f53fe1a3070df4495e350be3e540183bfe Author: Hsuan-Yi Chu Date: 2016-04-04T23:05:04Z DRILL-4577: Construct a specific path for querying all the tables from a hive database --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4529: Force $SUM0 to be used when Window...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/447 DRILL-4529: Force $SUM0 to be used when Window Sum is supposed to ret⦠â¦urned non-nullable type You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4529 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/447.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #447 commit aea449236c30948a88d46eb27a8b886f1d6a0c6f Author: Hsuan-Yi Chu Date: 2016-03-27T23:18:21Z DRILL-4529: Force $SUM0 to be used when Window Sum is supposed to returned non-nullable type --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/439#issuecomment-200919168 In addition to @jinfengni's point: @sudheeshkatkam did a proposal to add a new SqlAvgAggFunction (the inference mechanism of the original is not ideal for Drill). It was declined by Calcite's community, and the argument was adding unnecessary duplicates to the code. I think this argument is fair. Otherwise, there might be tens of variants for a single operator. Regarding adding a new converlet: Initially, I though having three wrappers (operator, function, agg function) are sufficient for all scenarios. However, at one method during conversion, the type SqlBetweenOperator is being expected. Thus, I decided to add a new wrapper which extends SqlBetweenOperator, so we do not need to do anything in conversion. Certainly, we might be able to attain by not adding a new wrapper but adding a new converlet. I would not prefer this approach since 1. I tried not to spread out the code complexity (introduced by inference and operand type check) to too many places. 2. Adding our own converlet also introduces code complexity. And it probably is more complicated than a wrapper. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/438#discussion_r57251287
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -157,9 +159,23 @@ private void populateWrappedCalciteOperators() {
} else if(calciteOperator instanceof SqlFunction) {
wrapper = new DrillCalciteSqlFunctionWrapper((SqlFunction)
calciteOperator,
getFunctionListWithInference(calciteOperator.getName()));
+ } else if(calciteOperator instanceof SqlBetweenOperator) {
+// During the procedure of converting to RexNode,
+// StandardConvertletTable.convertBetween expects the SqlOperator
to be a subclass of SqlBetweenOperator
+final SqlBetweenOperator sqlBetweenOperator = (SqlBetweenOperator)
calciteOperator;
+wrapper = new SqlBetweenOperator(sqlBetweenOperator.flag,
sqlBetweenOperator.isNegated()) {
+ @Override
+ public boolean checkOperandTypes(
--- End diff --
@jacques-n, Calcite has different (more strict) definition of
type-compatibilty while Drill is more flexible.
What we have done is to have our own type-compatibilty in drill. Please see
the new pull request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/438#discussion_r57250944
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -157,9 +159,23 @@ private void populateWrappedCalciteOperators() {
} else if(calciteOperator instanceof SqlFunction) {
wrapper = new DrillCalciteSqlFunctionWrapper((SqlFunction)
calciteOperator,
getFunctionListWithInference(calciteOperator.getName()));
+ } else if(calciteOperator instanceof SqlBetweenOperator) {
+// During the procedure of converting to RexNode,
+// StandardConvertletTable.convertBetween expects the SqlOperator
to be a subclass of SqlBetweenOperator
+final SqlBetweenOperator sqlBetweenOperator = (SqlBetweenOperator)
calciteOperator;
+wrapper = new SqlBetweenOperator(sqlBetweenOperator.flag,
sqlBetweenOperator.isNegated()) {
+ @Override
+ public boolean checkOperandTypes(
--- End diff --
Correct. Otherwise, the regression reported in this DRILL-4525 will be
failed by Calcite because Calcite does not allow between to be operated on
timestamp and date.
By the way, this pull request is closed. The correct pull request is this
one:
https://github.com/apache/drill/pull/439
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/439#issuecomment-199936054 @jinfengni can you help review? Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/439 DRILL-4525: Allow SqlBetweenOperator to accept LOWER_OPERAND and UPPE⦠â¦R_OPERAND with different types You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4525 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/439.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #439 commit 0f6bd0a398c017bdf40c01d2baab4050f8f00a2a Author: Hsuan-Yi Chu Date: 2016-03-21T21:43:54Z DRILL-4525: Allow SqlBetweenOperator to accept LOWER_OPERAND and UPPER_OPERAND with different types --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
Github user hsuanyi closed the pull request at: https://github.com/apache/drill/pull/438 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4525: Allow SqlBetweenOperator to accept...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/438 DRILL-4525: Allow SqlBetweenOperator to accept LOWER_OPERAND and UPPE⦠â¦R_OPERAND with different types You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4525 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/438.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #438 commit 5884e9b923057f1e9ed77ecf0ba8531e4d2f03b7 Author: Hsuan-Yi Chu Date: 2016-03-21T21:43:54Z DRILL-4525: Allow SqlBetweenOperator to accept LOWER_OPERAND and UPPER_OPERAND with different types --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3623: For limit 0 queries, use a shorter...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/405#issuecomment-199650953 Except for my comments (which are for future improvements and can be filed as follow-up jira issues) LGTM +1 (non-binding) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3623: For limit 0 queries, use a shorter...
Github user hsuanyi commented on a diff in the pull request: https://github.com/apache/drill/pull/405#discussion_r56783075 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/DirectScanPrule.java --- @@ -0,0 +1,40 @@ +/** + * 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.physical; + +import org.apache.calcite.plan.RelOptRule; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelTraitSet; +import org.apache.drill.exec.planner.logical.DrillDirectScanRel; +import org.apache.drill.exec.planner.logical.RelOptHelper; + --- End diff -- java doc? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4006: Ensure the position of MapWriters ...
Github user hsuanyi closed the pull request at: https://github.com/apache/drill/pull/242 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4323: When converting HiveParquetScan To...
Github user hsuanyi closed the pull request at: https://github.com/apache/drill/pull/349 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4490: Ensure the count generated by Conv...
Github user hsuanyi closed the pull request at: https://github.com/apache/drill/pull/423 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Drill 4372 for review
Github user hsuanyi closed the pull request at: https://github.com/apache/drill/pull/377 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4510: Force Union-All to happen in a sin...
Github user hsuanyi closed the pull request at: https://github.com/apache/drill/pull/433 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56456027
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java
---
@@ -0,0 +1,740 @@
+/**
+ * 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.sql;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+
+import org.apache.calcite.avatica.util.TimeUnit;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlCharStringLiteral;
+import org.apache.calcite.sql.SqlDynamicParam;
+import org.apache.calcite.sql.SqlIntervalQualifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.SqlRankFunction;
+import org.apache.calcite.sql.fun.SqlAvgAggFunction;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.type.SqlReturnTypeInference;
+import org.apache.calcite.sql.type.SqlTypeName;
+
+import org.apache.drill.common.expression.ExpressionPosition;
+import org.apache.drill.common.expression.FunctionCall;
+import org.apache.drill.common.expression.FunctionCallFactory;
+import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.expression.MajorTypeInLogicalExpression;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.common.types.Types;
+import org.apache.drill.exec.expr.TypeHelper;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
+import org.apache.drill.exec.resolver.FunctionResolver;
+import org.apache.drill.exec.resolver.FunctionResolverFactory;
+import org.apache.drill.exec.resolver.TypeCastRules;
+
+import java.util.List;
+
+public class TypeInferenceUtils {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(TypeInferenceUtils.class);
+
+ public static final TypeProtos.MajorType UNKNOWN_TYPE =
TypeProtos.MajorType.getDefaultInstance();
+ private static final ImmutableMap
DRILL_TO_CALCITE_TYPE_MAPPING = ImmutableMap. builder()
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56455656
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
---
@@ -92,23 +118,106 @@ public DrillFunctionRegistry(ScanResult
classpathScan) {
}
public int size(){
-return methods.size();
+return registeredFunctions.size();
}
/** Returns functions with given name. Function name is case
insensitive. */
public List getMethods(String name) {
-return this.methods.get(name.toLowerCase());
+return this.registeredFunctions.get(name.toLowerCase());
}
public void register(DrillOperatorTable operatorTable) {
--- End diff --
It is for safely falling back. The more we diverge from the original
mechanism, the more risky we cannot fall back to the original behavior.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56455794
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -26,34 +33,88 @@
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.server.options.SystemOptionManager;
import java.util.List;
+import java.util.Map;
+/**
+ * Implementation of {@link SqlOperatorTable} that contains standard
operators and functions provided through
+ * {@link #inner SqlStdOperatorTable}, and Drill User Defined Functions.
+ */
public class DrillOperatorTable extends SqlStdOperatorTable {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
-
+// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
private static final SqlOperatorTable inner =
SqlStdOperatorTable.instance();
- private List operators;
- private ArrayListMultimap opMap =
ArrayListMultimap.create();
+ private final List operatorsCalcite = Lists.newArrayList();
--- End diff --
operatorsCalcite is native Calcite operators. I change it to
operatorsNativeCalcite. Hopefully, it gives better information.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56455331
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HiveUDFOperatorNotInfer.java
---
@@ -0,0 +1,44 @@
+/**
+ * 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.sql;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+
+public class HiveUDFOperatorNotInfer extends HiveUDFOperator {
--- End diff --
renamed as HiveUDFOperatorWithoutInference
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56455344
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
---
@@ -122,6 +127,13 @@ public int size(){
}
public void register(DrillOperatorTable operatorTable) {
+registerForInference(operatorTable);
+registerForDefault(operatorTable);
--- End diff --
Renamed one as registerOperatorsWithoutInference; and the other as
registerOperatorsWithInference
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56457961
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -26,34 +33,88 @@
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.server.options.SystemOptionManager;
import java.util.List;
+import java.util.Map;
+/**
+ * Implementation of {@link SqlOperatorTable} that contains standard
operators and functions provided through
+ * {@link #inner SqlStdOperatorTable}, and Drill User Defined Functions.
+ */
public class DrillOperatorTable extends SqlStdOperatorTable {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
-
+// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
private static final SqlOperatorTable inner =
SqlStdOperatorTable.instance();
- private List operators;
- private ArrayListMultimap opMap =
ArrayListMultimap.create();
+ private final List operatorsCalcite = Lists.newArrayList();
--- End diff --
ok
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56453928
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperatorNotInfer.java
---
@@ -0,0 +1,76 @@
+/**
+ * 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.sql;
+
+import com.google.common.base.Preconditions;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
+
+import java.util.ArrayList;
+
+public class DrillSqlOperatorNotInfer extends DrillSqlOperator {
--- End diff --
Addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56454893
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -26,34 +33,88 @@
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.server.options.SystemOptionManager;
import java.util.List;
+import java.util.Map;
+/**
+ * Implementation of {@link SqlOperatorTable} that contains standard
operators and functions provided through
+ * {@link #inner SqlStdOperatorTable}, and Drill User Defined Functions.
+ */
public class DrillOperatorTable extends SqlStdOperatorTable {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
-
+// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
private static final SqlOperatorTable inner =
SqlStdOperatorTable.instance();
- private List operators;
- private ArrayListMultimap opMap =
ArrayListMultimap.create();
+ private final List operatorsCalcite = Lists.newArrayList();
+ private final List operatorsDefault = Lists.newArrayList();
--- End diff --
We want to safely fall back to original behavior (which does not do
inference). So the safest way is to have the ones which do not do inference to
have their own operators.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56439803
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperatorNotInfer.java
---
@@ -0,0 +1,76 @@
+/**
+ * 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.sql;
+
+import com.google.common.base.Preconditions;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
+
+import java.util.ArrayList;
+
+public class DrillSqlOperatorNotInfer extends DrillSqlOperator {
+ private static final TypeProtos.MajorType NONE =
TypeProtos.MajorType.getDefaultInstance();
+ private final TypeProtos.MajorType returnType;
+
+ public DrillSqlOperatorNotInfer(String name, int argCount,
TypeProtos.MajorType returnType, boolean isDeterminisitic) {
+super(name,
+new ArrayList< DrillFuncHolder>(),
+argCount,
+argCount,
+isDeterminisitic,
+DynamicReturnType.INSTANCE);
+this.returnType = Preconditions.checkNotNull(returnType);
+ }
+
+ protected RelDataType getReturnDataType(final RelDataTypeFactory
factory) {
+if (TypeProtos.MinorType.BIT.equals(returnType.getMinorType())) {
+ return factory.createSqlType(SqlTypeName.BOOLEAN);
+}
+return
factory.createTypeWithNullability(factory.createSqlType(SqlTypeName.ANY), true);
+ }
+
+ private RelDataType getNullableReturnDataType(final RelDataTypeFactory
factory) {
+return factory.createTypeWithNullability(getReturnDataType(factory),
true);
+ }
+
+ @Override
+ public RelDataType deriveType(SqlValidator validator, SqlValidatorScope
scope, SqlCall call) {
+if (NONE.equals(returnType)) {
+ return validator.getTypeFactory().createSqlType(SqlTypeName.ANY);
--- End diff --
Similar to the one in DrillSqlAggOperatorNotInfer.java. Please see [1]
[1]
https://github.com/apache/drill/blob/d7eebec41a1636055be1b2c79b693d76c52d8932/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java#L71
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56459723
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java
---
@@ -64,4 +108,47 @@ public boolean isDeterministic() {
public List getFunctions() {
return functions;
}
+
+ public static class DrillSqlOperatorBuilder {
+private String name;
+private final List functions = Lists.newArrayList();
+private int argCountMin = Integer.MAX_VALUE;
+private int argCountMax = Integer.MIN_VALUE;
+private boolean isDeterministic = true;
+
+public DrillSqlOperatorBuilder setName(final String name) {
+ this.name = name;
+ return this;
+}
+
+public DrillSqlOperatorBuilder
addFunctions(Collection functions) {
+ this.functions.addAll(functions);
+ return this;
+}
+
+public DrillSqlOperatorBuilder setArgumentCount(final int argCountMin,
final int argCountMax) {
+ this.argCountMin = Math.min(this.argCountMin, argCountMin);
+ this.argCountMax = Math.max(this.argCountMax, argCountMax);
+ return this;
+}
+
+public DrillSqlOperatorBuilder setDeterministic(boolean
isDeterministic) {
+ if(this.isDeterministic) {
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56455160
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
---
@@ -204,4 +220,46 @@ private HiveFuncHolder matchAndCreateUDFHolder(String
udfName,
return null;
}
+ public class HiveSqlReturnTypeInference implements
SqlReturnTypeInference {
+private HiveSqlReturnTypeInference() {
+
+}
+
+@Override
+public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
+ for (RelDataType type : opBinding.collectOperandTypes()) {
+final TypeProtos.MinorType minorType =
TypeInferenceUtils.getDrillTypeFromCalciteType(type);
+if(minorType == TypeProtos.MinorType.LATE) {
+ return opBinding.getTypeFactory()
+ .createTypeWithNullability(
+
opBinding.getTypeFactory().createSqlType(SqlTypeName.ANY),
+ true);
+}
+ }
+
+ final FunctionCall functionCall =
TypeInferenceUtils.convertSqlOperatorBindingToFunctionCall(opBinding);
+ final HiveFuncHolder hiveFuncHolder = getFunction(functionCall);
+ if(hiveFuncHolder == null) {
+String operandTypes = "";
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56434976
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlAggOperatorNotInfer.java
---
@@ -0,0 +1,43 @@
+/**
+ * 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.sql;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
+
+import java.util.ArrayList;
+
+public class DrillSqlAggOperatorNotInfer extends DrillSqlAggOperator {
+ public DrillSqlAggOperatorNotInfer(String name, int argCount) {
+super(name, new ArrayList(), argCount, argCount,
DynamicReturnType.INSTANCE);
+ }
+
+ @Override
+ public RelDataType deriveType(SqlValidator validator, SqlValidatorScope
scope, SqlCall call) {
+return getAny(validator.getTypeFactory());
+ }
+
+ private RelDataType getAny(RelDataTypeFactory factory){
+return factory.createSqlType(SqlTypeName.ANY);
--- End diff --
DrillSqlAggOperatorNotInfer.java corresponds to the DrillSqlAggOperator in
the old code[1]. When the inference option is being turned off, we want to fall
back to the original behavior as close as possible. With that in mind, I was
trying to not change the logic too much.
[1]https://github.com/apache/drill/blob/d7eebec41a1636055be1b2c79b693d76c52d8932/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlAggOperator.java#L50
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56459546
--- Diff:
contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
---
@@ -43,4 +43,32 @@ public void testEncode() throws Exception {
.baselineValues(new Object[] { null })
.go();
}
+
+ @Test
+ public void testReflect() throws Exception {
+final String query = "select reflect('java.lang.Math', 'round', cast(2
as float)) as col \n" +
--- End diff --
Addressed. But I choose a function which type can be resolved in planning
time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56453956
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperatorNotInfer.java
---
@@ -0,0 +1,76 @@
+/**
+ * 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.sql;
+
+import com.google.common.base.Preconditions;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
+
+import java.util.ArrayList;
+
+public class DrillSqlOperatorNotInfer extends DrillSqlOperator {
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56704717
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
---
@@ -92,23 +114,100 @@ public DrillFunctionRegistry(ScanResult
classpathScan) {
}
public int size(){
-return methods.size();
+return registeredFunctions.size();
}
/** Returns functions with given name. Function name is case
insensitive. */
public List getMethods(String name) {
-return this.methods.get(name.toLowerCase());
+return this.registeredFunctions.get(name.toLowerCase());
}
public void register(DrillOperatorTable operatorTable) {
+registerOperatorsWithInference(operatorTable);
+registerOperatorsWithoutInference(operatorTable);
+ }
+
+ private void registerOperatorsWithInference(DrillOperatorTable
operatorTable) {
+final Map map =
Maps.newHashMap();
+final Map
mapAgg = Maps.newHashMap();
+for (Entry> function :
registeredFunctions.asMap().entrySet()) {
+ final ArrayListMultimap, DrillFuncHolder>
functions = ArrayListMultimap.create();
+ final ArrayListMultimap aggregateFunctions
= ArrayListMultimap.create();
+ final String name = function.getKey().toUpperCase();
+ boolean isDeterministic = true;
+ for (DrillFuncHolder func : function.getValue()) {
+final int paramCount = func.getParamCount();
+if(func.isAggregating()) {
+ aggregateFunctions.put(paramCount, func);
+} else {
+ final Pair argNumberRange;
+ if(registeredFuncNameToArgRange.containsKey(name)) {
+argNumberRange = registeredFuncNameToArgRange.get(name);
+ } else {
+argNumberRange = Pair.of(func.getParamCount(),
func.getParamCount());
+ }
+ functions.put(argNumberRange, func);
+}
+
+if(!func.isDeterministic()) {
+ isDeterministic = false;
+}
+ }
+ for (Entry, Collection>
entry : functions.asMap().entrySet()) {
+final Pair range = entry.getKey();
+final int max = range.getRight();
+final int min = range.getLeft();
+if(!map.containsKey(name)) {
+ map.put(name, new DrillSqlOperator.DrillSqlOperatorBuilder()
+ .setName(name));
+}
+
+final DrillSqlOperator.DrillSqlOperatorBuilder
drillSqlOperatorBuilder = map.get(name);
+drillSqlOperatorBuilder
+.addFunctions(entry.getValue())
+.setArgumentCount(min, max)
+.setDeterministic(isDeterministic);
+ }
+ for (Entry> entry :
aggregateFunctions.asMap().entrySet()) {
+if(mapAgg.containsKey(name)) {
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56456528
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillAvgVarianceConvertlet.java
---
@@ -40,7 +43,16 @@
public class DrillAvgVarianceConvertlet implements SqlRexConvertlet {
private final SqlAvgAggFunction.Subtype subtype;
- private static final DrillSqlOperator CastHighOp = new
DrillSqlOperator("CastHigh", 1, false);
+ private static final DrillSqlOperator CastHighOp = new
DrillSqlOperator("CastHigh", 1, false,
+ new SqlReturnTypeInference() {
+@Override
+public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
+ return TypeInferenceUtils.createCalciteTypeWithNullability(
+ opBinding.getTypeFactory(),
+ SqlTypeName.ANY,
--- End diff --
1.This is the legacy code. In theory, we should not create DrillSqlOperator
outside of DrillFunctionRegistry.
2. Convertlet follows after validation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56453886
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -62,11 +123,70 @@ public void lookupOperatorOverloads(SqlIdentifier
opName, SqlFunctionCategory ca
@Override
public List getOperatorList() {
-return operators;
+final List sqlOperators = Lists.newArrayList();
+sqlOperators.addAll(operatorsCalcite);
+if(isEnableInference()) {
+ sqlOperators.addAll(operatorsInferernce);
+} else {
+ sqlOperators.addAll(operatorsDefault);
+}
+
+return sqlOperators;
}
// Get the list of SqlOperator's with the given name.
public List getSqlOperator(String name) {
-return opMap.get(name.toLowerCase());
+if(isEnableInference()) {
+ return opMapInferernce.get(name.toLowerCase());
+} else {
+ return opMapDefault.get(name.toLowerCase());
+}
+ }
+
+ private void populateWrappedCalciteOperators() {
+for(SqlOperator calciteOperator : inner.getOperatorList()) {
+ final SqlOperator wrapper;
+ if(calciteOperator instanceof SqlAggFunction) {
+wrapper = new DrillCalciteSqlAggFunctionWrapper((SqlAggFunction)
calciteOperator,
+getFunctionListWithInference(calciteOperator.getName()));
+ } else if(calciteOperator instanceof SqlFunction) {
+wrapper = new DrillCalciteSqlFunctionWrapper((SqlFunction)
calciteOperator,
+getFunctionListWithInference(calciteOperator.getName()));
+ } else {
+final String drillOpName =
FunctionCallFactory.replaceOpWithFuncName(calciteOperator.getName());
+final List drillFuncHolders =
getFunctionListWithInference(drillOpName);
+if(drillFuncHolders.isEmpty() || calciteOperator ==
SqlStdOperatorTable.UNARY_MINUS || calciteOperator ==
SqlStdOperatorTable.UNARY_PLUS) {
+ continue;
+}
+
+wrapper = new DrillCalciteSqlOperatorWrapper(calciteOperator,
drillOpName, drillFuncHolders);
+ }
+ calciteToWrapper.put(calciteOperator, wrapper);
+}
+ }
+
+ private List getFunctionListWithInference(String name) {
+final List functions = Lists.newArrayList();
+for(SqlOperator sqlOperator : opMapInferernce.get(name.toLowerCase()))
{
+ if(sqlOperator instanceof DrillSqlOperator) {
+final List list = ((DrillSqlOperator)
sqlOperator).getFunctions();
+if(list != null) {
+ functions.addAll(list);
+}
+ }
+
+ if(sqlOperator instanceof DrillSqlAggOperator) {
+final List list = ((DrillSqlAggOperator)
sqlOperator).getFunctions();
+if(list != null) {
+ functions.addAll(list);
+}
+ }
+}
+return functions;
+ }
+
+ private boolean isEnableInference() {
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56527875
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
---
@@ -364,4 +401,35 @@ private static SchemaPlus rootSchema(SchemaPlus
schema) {
}
}
+ private static class DrillRexBuilder extends RexBuilder {
+private DrillRexBuilder(RelDataTypeFactory typeFactory) {
+ super(typeFactory);
+}
+
+@Override
+public RexNode ensureType(
+RelDataType type,
+RexNode node,
+boolean matchNullability) {
+ RelDataType targetType = type;
+ if (matchNullability) {
+targetType = matchNullability(type, node);
+ }
+ if (targetType.getSqlTypeName() == SqlTypeName.ANY) {
+return node;
+ }
+ if (!node.getType().equals(targetType)) {
+if(!targetType.isStruct()) {
+ final RelDataType anyType =
TypeInferenceUtils.createCalciteTypeWithNullability(
--- End diff --
I thought Calcite might have mechanism to further catch the cases where
implicit casting is not added in.
Let me try if we can find any issue when there is no such a cast to ANY
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56456844
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java
---
@@ -64,4 +108,47 @@ public boolean isDeterministic() {
public List getFunctions() {
return functions;
}
+
+ public static class DrillSqlOperatorBuilder {
+private String name;
+private final List functions = Lists.newArrayList();
+private int argCountMin = Integer.MAX_VALUE;
+private int argCountMax = Integer.MIN_VALUE;
+private boolean isDeterministic = true;
+
+public DrillSqlOperatorBuilder setName(final String name) {
+ this.name = name;
+ return this;
+}
+
+public DrillSqlOperatorBuilder
addFunctions(Collection functions) {
+ this.functions.addAll(functions);
+ return this;
+}
+
+public DrillSqlOperatorBuilder setArgumentCount(final int argCountMin,
final int argCountMax) {
+ this.argCountMin = Math.min(this.argCountMin, argCountMin);
+ this.argCountMax = Math.max(this.argCountMax, argCountMax);
+ return this;
+}
+
+public DrillSqlOperatorBuilder setDeterministic(boolean
isDeterministic) {
+ if(this.isDeterministic) {
--- End diff --
I think we had this discussion on the review procedure before. Say, in the
Collection we have some DrillFuncHolder which are
deterministic and the other are non-deterministic.
By the logic here, we will group the entire Collection as
a DrillSqlOperator and claim it is non-deterministic.
In fact, separating them into two DrillSqlOperator (one being deterministic
and the other being non-deterministic does not help).
Please see DrillOperatorTable.lookupOperatorOverloads(). As you can see,
parameter list is not passed in. So even if we have two DrillSqlOperator,
DrillOperatorTable.lookupOperatorOverloads() does not have the enough
information to pick the one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56454823
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java
---
@@ -64,4 +108,47 @@ public boolean isDeterministic() {
public List getFunctions() {
return functions;
}
+
+ public static class DrillSqlOperatorBuilder {
+private String name;
+private final List functions = Lists.newArrayList();
+private int argCountMin = Integer.MAX_VALUE;
+private int argCountMax = Integer.MIN_VALUE;
+private boolean isDeterministic = true;
+
+public DrillSqlOperatorBuilder setName(final String name) {
+ this.name = name;
+ return this;
+}
+
+public DrillSqlOperatorBuilder
addFunctions(Collection functions) {
+ this.functions.addAll(functions);
+ return this;
+}
+
+public DrillSqlOperatorBuilder setArgumentCount(final int argCountMin,
final int argCountMax) {
+ this.argCountMin = Math.min(this.argCountMin, argCountMin);
+ this.argCountMax = Math.max(this.argCountMax, argCountMax);
+ return this;
+}
+
+public DrillSqlOperatorBuilder setDeterministic(boolean
isDeterministic) {
+ if(this.isDeterministic) {
+this.isDeterministic = isDeterministic;
+ }
+ return this;
+}
+
+public DrillSqlOperator build() {
+ return new DrillSqlOperator(
+ name,
+ functions,
+ argCountMin,
+ argCountMax,
+ isDeterministic,
+ TypeInferenceUtils.getDrillSqlReturnTypeInference(
+ name,
--- End diff --
Add a check in the build()
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56527238
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
---
@@ -364,4 +401,35 @@ private static SchemaPlus rootSchema(SchemaPlus
schema) {
}
}
+ private static class DrillRexBuilder extends RexBuilder {
+private DrillRexBuilder(RelDataTypeFactory typeFactory) {
+ super(typeFactory);
+}
+
+@Override
+public RexNode ensureType(
+RelDataType type,
+RexNode node,
+boolean matchNullability) {
+ RelDataType targetType = type;
+ if (matchNullability) {
+targetType = matchNullability(type, node);
+ }
+ if (targetType.getSqlTypeName() == SqlTypeName.ANY) {
+return node;
+ }
+ if (!node.getType().equals(targetType)) {
+if(!targetType.isStruct()) {
+ final RelDataType anyType =
TypeInferenceUtils.createCalciteTypeWithNullability(
--- End diff --
As you can see line 418, if one side is ANY, then even if the types are
different, Calcite would not complain, nor fail.
Here, what we want to do is "not letting Calcite doing anything" even when
the types are not matched. So I think we should borrow the solution in line 418.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56455114
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
---
@@ -92,23 +118,106 @@ public DrillFunctionRegistry(ScanResult
classpathScan) {
}
public int size(){
-return methods.size();
+return registeredFunctions.size();
}
/** Returns functions with given name. Function name is case
insensitive. */
public List getMethods(String name) {
-return this.methods.get(name.toLowerCase());
+return this.registeredFunctions.get(name.toLowerCase());
}
public void register(DrillOperatorTable operatorTable) {
+registerForInference(operatorTable);
+registerForDefault(operatorTable);
+ }
+
+ public void registerForInference(DrillOperatorTable operatorTable) {
+final Map map =
Maps.newHashMap();
+final Map
mapAgg = Maps.newHashMap();
+for (Entry> function :
registeredFunctions.asMap().entrySet()) {
+ final ArrayListMultimap, DrillFuncHolder>
functions = ArrayListMultimap.create();
+ final ArrayListMultimap aggregateFunctions
= ArrayListMultimap.create();
+ final String name = function.getKey().toUpperCase();
+ boolean isDeterministic = true;
+ for (DrillFuncHolder func : function.getValue()) {
+final int paramCount = func.getParamCount();
+if(func.isAggregating()) {
+ aggregateFunctions.put(paramCount, func);
+} else {
+ final Pair argNumberRange;
+ if(drillFuncToRange.containsKey(name)) {
+argNumberRange = drillFuncToRange.get(name);
+ } else {
+argNumberRange = Pair.of(func.getParamCount(),
func.getParamCount());
+ }
+ functions.put(argNumberRange, func);
+}
+
+if(!func.isDeterministic()) {
+ isDeterministic = false;
+}
+ }
+ for (Entry, Collection>
entry : functions.asMap().entrySet()) {
+final Pair range = entry.getKey();
+final int max = range.getRight();
+final int min = range.getLeft();
+if(map.containsKey(name)) {
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56458586
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
---
@@ -364,4 +401,35 @@ private static SchemaPlus rootSchema(SchemaPlus
schema) {
}
}
+ private static class DrillRexBuilder extends RexBuilder {
+private DrillRexBuilder(RelDataTypeFactory typeFactory) {
+ super(typeFactory);
+}
+
+@Override
+public RexNode ensureType(
+RelDataType type,
+RexNode node,
+boolean matchNullability) {
+ RelDataType targetType = type;
+ if (matchNullability) {
+targetType = matchNullability(type, node);
+ }
+ if (targetType.getSqlTypeName() == SqlTypeName.ANY) {
+return node;
+ }
+ if (!node.getType().equals(targetType)) {
+if(!targetType.isStruct()) {
+ final RelDataType anyType =
TypeInferenceUtils.createCalciteTypeWithNullability(
--- End diff --
This is to avoid implicit casting added by Calcite.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56454445
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
---
@@ -183,10 +190,40 @@ public SchemaPlus getDefaultSchema() {
}
private class DrillValidator extends SqlValidatorImpl {
+private final Set identitySet =
Sets.newIdentityHashSet();
+
protected DrillValidator(SqlOperatorTable opTab,
SqlValidatorCatalogReader catalogReader,
RelDataTypeFactory typeFactory, SqlConformance conformance) {
super(opTab, catalogReader, typeFactory, conformance);
}
+
+@Override
+public SqlValidatorScope getSelectScope(final SqlSelect select) {
--- End diff --
will move this logic to calcite.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56454322
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
---
@@ -17,40 +17,66 @@
*/
package org.apache.drill.exec.expr.fn;
-import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.commons.lang3.tuple.Pair;
import
org.apache.drill.common.scanner.persistence.AnnotatedClassDescriptor;
import org.apache.drill.common.scanner.persistence.ScanResult;
-import org.apache.drill.exec.expr.DrillFunc;
+import org.apache.drill.common.types.TypeProtos;
import org.apache.drill.exec.planner.logical.DrillConstExecutor;
import org.apache.drill.exec.planner.sql.DrillOperatorTable;
import org.apache.drill.exec.planner.sql.DrillSqlAggOperator;
+import org.apache.drill.exec.planner.sql.DrillSqlAggOperatorNotInfer;
import org.apache.drill.exec.planner.sql.DrillSqlOperator;
import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.Sets;
+import org.apache.drill.exec.planner.sql.DrillSqlOperatorNotInfer;
+/**
+ * Registry of Drill functions.
+ */
public class DrillFunctionRegistry {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillFunctionRegistry.class);
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillFunctionRegistry.class);
- private ArrayListMultimap methods =
ArrayListMultimap.create();
+ // key: function name (lowercase) value: list of functions with that name
+ private final ArrayListMultimap
registeredFunctions = ArrayListMultimap.create();
- /* Hash map to prevent registering functions with exactly matching
signatures
- * key: Function Name + Input's Major Type
- * Value: Class name where function is implemented
- */
- private HashMap functionSignatureMap = new HashMap<>();
+ private static final ImmutableMap>
drillFuncToRange = ImmutableMap.> builder()
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56453885
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlAggOperatorNotInfer.java
---
@@ -0,0 +1,43 @@
+/**
+ * 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.sql;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
+
+import java.util.ArrayList;
+
+public class DrillSqlAggOperatorNotInfer extends DrillSqlAggOperator {
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56454989
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -26,34 +33,88 @@
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.server.options.SystemOptionManager;
import java.util.List;
+import java.util.Map;
+/**
+ * Implementation of {@link SqlOperatorTable} that contains standard
operators and functions provided through
+ * {@link #inner SqlStdOperatorTable}, and Drill User Defined Functions.
+ */
public class DrillOperatorTable extends SqlStdOperatorTable {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
-
+// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
private static final SqlOperatorTable inner =
SqlStdOperatorTable.instance();
- private List operators;
- private ArrayListMultimap opMap =
ArrayListMultimap.create();
+ private final List operatorsCalcite = Lists.newArrayList();
+ private final List operatorsDefault = Lists.newArrayList();
+ private final List operatorsInferernce =
Lists.newArrayList();
+ private final Map calciteToWrapper =
Maps.newIdentityHashMap();
+
+ private final ArrayListMultimap opMapDefault =
ArrayListMultimap.create();
+ private final ArrayListMultimap opMapInferernce =
ArrayListMultimap.create();
+
+ private final SystemOptionManager systemOptionManager;
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56431658
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java ---
@@ -135,6 +135,16 @@ public RuleSet getRules(OptimizerRulesContext context,
Collection
}
},
+ SUM_CONVERSION("Convert SUM to $SUM0") {
--- End diff --
Yes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56458266
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java
---
@@ -140,6 +140,10 @@ public DrillSqlOperatorBuilder
setDeterministic(boolean isDeterministic) {
}
public DrillSqlOperator build() {
+ if(name == null || functions.isEmpty()) {
+throw new AssertionError("The fields, name and functions, need to
be set before build DrillSqlAggOperator");
--- End diff --
build gets called before fields are properly set.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4510: Force Union-All to happen in a sin...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/433 DRILL-4510: Force Union-All to happen in a single node You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4510 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/433.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #433 commit 6c025c90ad23d16b0aefca5f03c033362f93 Author: Hsuan-Yi Chu Date: 2016-03-16T04:52:17Z DRILL-4510: Force Union-All to happen in a single node --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56523015
--- Diff:
contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
---
@@ -43,4 +43,32 @@ public void testEncode() throws Exception {
.baselineValues(new Object[] { null })
.go();
}
+
+ @Test
+ public void testReflect() throws Exception {
+final String query = "select reflect('java.lang.Math', 'round', cast(2
as float)) as col \n" +
--- End diff --
BTW, the correct way to verify this is to check the plan produced by your
limit 0 shortcut.
But apparently, we can enhance that after yours gets in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56704712
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java
---
@@ -209,6 +212,10 @@ public static long getInitialPlanningMemorySize() {
return INITIAL_OFF_HEAP_ALLOCATION_IN_BYTES;
}
+ public boolean isTypeInferenceEnabled() {
+return options.getOption(TYPE_INFERENCE.getOptionName()).bool_val;
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56454252
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HiveUDFOperator.java
---
@@ -18,28 +18,20 @@
package org.apache.drill.exec.planner.sql;
-import com.fasterxml.jackson.databind.type.TypeFactory;
-import org.apache.calcite.rel.type.RelDataType;
-import org.apache.calcite.rel.type.RelDataTypeFactory;
-import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlCallBinding;
import org.apache.calcite.sql.SqlFunction;
import org.apache.calcite.sql.SqlFunctionCategory;
import org.apache.calcite.sql.SqlIdentifier;
import org.apache.calcite.sql.SqlOperandCountRange;
import org.apache.calcite.sql.SqlOperator;
-import org.apache.calcite.sql.SqlOperatorBinding;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.type.SqlOperandCountRanges;
import org.apache.calcite.sql.type.SqlOperandTypeChecker;
-import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.sql.validate.SqlValidator;
-import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.calcite.sql.type.SqlReturnTypeInference;
public class HiveUDFOperator extends SqlFunction {
-
- public HiveUDFOperator(String name) {
-super(new SqlIdentifier(name, SqlParserPos.ZERO),
DynamicReturnType.INSTANCE, null, new ArgChecker(), null,
+ public HiveUDFOperator(String name, SqlReturnTypeInference
sqlReturnTypeInference) {
+super(new SqlIdentifier(name, SqlParserPos.ZERO),
sqlReturnTypeInference, null, new ArgChecker(), null,
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56454010
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlAggOperator.java
---
@@ -17,47 +17,76 @@
*/
package org.apache.drill.exec.planner.sql;
-import java.util.List;
-
-import org.apache.calcite.rel.type.RelDataType;
-import org.apache.calcite.rel.type.RelDataTypeFactory;
+import com.google.common.collect.Lists;
import org.apache.calcite.sql.SqlAggFunction;
-import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlFunctionCategory;
import org.apache.calcite.sql.SqlIdentifier;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.parser.SqlParserPos;
-import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.sql.validate.SqlValidator;
-import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.calcite.sql.type.SqlReturnTypeInference;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
-import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.List;
public class DrillSqlAggOperator extends SqlAggFunction {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillSqlAggOperator.class);
+ // private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillSqlAggOperator.class);
+ private final List functions;
-
- public DrillSqlAggOperator(String name, int argCount) {
-super(name, new SqlIdentifier(name, SqlParserPos.ZERO),
SqlKind.OTHER_FUNCTION, DynamicReturnType.INSTANCE, null, new
Checker(argCount), SqlFunctionCategory.USER_DEFINED_FUNCTION);
+ protected DrillSqlAggOperator(String name, List
functions, int argCountMin, int argCountMax, SqlReturnTypeInference
sqlReturnTypeInference) {
+super(name,
+new SqlIdentifier(name, SqlParserPos.ZERO),
+SqlKind.OTHER_FUNCTION,
+sqlReturnTypeInference,
+null,
+Checker.getChecker(argCountMin, argCountMax),
+SqlFunctionCategory.USER_DEFINED_FUNCTION);
+this.functions = functions;
}
- @Override
- public RelDataType deriveType(SqlValidator validator, SqlValidatorScope
scope, SqlCall call) {
-return getAny(validator.getTypeFactory());
+ private DrillSqlAggOperator(String name, List
functions, int argCountMin, int argCountMax) {
+this(name,
+functions,
+argCountMin,
+argCountMax,
+TypeInferenceUtils.getDrillSqlReturnTypeInference(
+name,
+functions));
}
- private RelDataType getAny(RelDataTypeFactory factory){
-return factory.createSqlType(SqlTypeName.ANY);
-//return new RelDataTypeDrillImpl(new RelDataTypeHolder(), factory);
+ public List getFunctions() {
+return functions;
}
-// @Override
-// public List getParameterTypes(RelDataTypeFactory
typeFactory) {
-//return ImmutableList.of(typeFactory.createSqlType(SqlTypeName.ANY));
-// }
-//
-// @Override
-// public RelDataType getReturnType(RelDataTypeFactory typeFactory) {
-//return getAny(typeFactory);
-// }
+ public static class DrillSqlAggOperatorBuilder {
+private String name;
+private final List functions = Lists.newArrayList();
+private int argCountMin = Integer.MAX_VALUE;
+private int argCountMax = Integer.MIN_VALUE;
+private boolean isDeterministic = true;
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56455697
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
---
@@ -92,23 +118,106 @@ public DrillFunctionRegistry(ScanResult
classpathScan) {
}
public int size(){
-return methods.size();
+return registeredFunctions.size();
}
/** Returns functions with given name. Function name is case
insensitive. */
public List getMethods(String name) {
-return this.methods.get(name.toLowerCase());
+return this.registeredFunctions.get(name.toLowerCase());
}
public void register(DrillOperatorTable operatorTable) {
+registerForInference(operatorTable);
--- End diff --
addressed as above
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56528868
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -26,34 +33,88 @@
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.server.options.SystemOptionManager;
import java.util.List;
+import java.util.Map;
+/**
+ * Implementation of {@link SqlOperatorTable} that contains standard
operators and functions provided through
+ * {@link #inner SqlStdOperatorTable}, and Drill User Defined Functions.
+ */
public class DrillOperatorTable extends SqlStdOperatorTable {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
-
+// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
private static final SqlOperatorTable inner =
SqlStdOperatorTable.instance();
- private List operators;
- private ArrayListMultimap opMap =
ArrayListMultimap.create();
+ private final List operatorsCalcite = Lists.newArrayList();
+ private final List operatorsDefault = Lists.newArrayList();
+ private final List operatorsInferernce =
Lists.newArrayList();
+ private final Map calciteToWrapper =
Maps.newIdentityHashMap();
+
+ private final ArrayListMultimap opMapDefault =
ArrayListMultimap.create();
+ private final ArrayListMultimap opMapInferernce =
ArrayListMultimap.create();
+
+ private final SystemOptionManager systemOptionManager;
public DrillOperatorTable(FunctionImplementationRegistry registry) {
-operators = Lists.newArrayList();
-operators.addAll(inner.getOperatorList());
+this(registry, null);
+ }
+ public DrillOperatorTable(FunctionImplementationRegistry registry,
SystemOptionManager systemOptionManager) {
registry.register(this);
+operatorsCalcite.addAll(inner.getOperatorList());
+populateWrappedCalciteOperators();
+this.systemOptionManager = systemOptionManager;
+ }
+
+ public void addDefault(String name, SqlOperator op) {
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56454944
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
---
@@ -26,34 +33,88 @@
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.server.options.SystemOptionManager;
import java.util.List;
+import java.util.Map;
+/**
+ * Implementation of {@link SqlOperatorTable} that contains standard
operators and functions provided through
+ * {@link #inner SqlStdOperatorTable}, and Drill User Defined Functions.
+ */
public class DrillOperatorTable extends SqlStdOperatorTable {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
-
+// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
private static final SqlOperatorTable inner =
SqlStdOperatorTable.instance();
- private List operators;
- private ArrayListMultimap opMap =
ArrayListMultimap.create();
+ private final List operatorsCalcite = Lists.newArrayList();
+ private final List operatorsDefault = Lists.newArrayList();
+ private final List operatorsInferernce =
Lists.newArrayList();
+ private final Map calciteToWrapper =
Maps.newIdentityHashMap();
+
+ private final ArrayListMultimap opMapDefault =
ArrayListMultimap.create();
+ private final ArrayListMultimap opMapInferernce =
ArrayListMultimap.create();
+
+ private final SystemOptionManager systemOptionManager;
public DrillOperatorTable(FunctionImplementationRegistry registry) {
-operators = Lists.newArrayList();
-operators.addAll(inner.getOperatorList());
+this(registry, null);
--- End diff --
I removed this constructor
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56440041
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperatorNotInfer.java
---
@@ -0,0 +1,76 @@
+/**
+ * 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.sql;
+
+import com.google.common.base.Preconditions;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
+
+import java.util.ArrayList;
+
+public class DrillSqlOperatorNotInfer extends DrillSqlOperator {
+ private static final TypeProtos.MajorType NONE =
TypeProtos.MajorType.getDefaultInstance();
+ private final TypeProtos.MajorType returnType;
+
+ public DrillSqlOperatorNotInfer(String name, int argCount,
TypeProtos.MajorType returnType, boolean isDeterminisitic) {
+super(name,
+new ArrayList< DrillFuncHolder>(),
+argCount,
+argCount,
+isDeterminisitic,
+DynamicReturnType.INSTANCE);
--- End diff --
This is legacy code [1] (DrillSqlOperatorNotInfer corresponds to the
DrillSqlOperator in the old code).
[1]
https://github.com/apache/drill/blob/d7eebec41a1636055be1b2c79b693d76c52d8932/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java#L48
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r56455347
--- Diff:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HiveUDFOperatorNotInfer.java
---
@@ -0,0 +1,44 @@
+/**
+ * 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.sql;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+
+public class HiveUDFOperatorNotInfer extends HiveUDFOperator {
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/407#issuecomment-195794797 Comments were added to the code; Also passed all the pre commit test. Can you help commit this patch ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4490: Ensure the count generated by Conv...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/423#discussion_r55730852
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java
---
@@ -460,4 +466,23 @@ public void testGroupBySystemFuncFileSystemTable()
throws Exception {
public void test4443() throws Exception {
test("SELECT MIN(columns[1]) FROM dfs_test.`%s/agg/4443.csv` GROUP BY
columns[0]", TEST_RES_PATH);
}
+
+ @Test
+ public void testCountStarRequired() throws Exception {
+final String query = "select count(*) as col from
cp.`tpch/region.parquet` limit 0";
--- End diff --
Only schema is being validated in this case.
It might be confusing if the query returns the records but we only inspect
the schema.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4490: Ensure the count generated by Conv...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/423 DRILL-4490: Ensure the count generated by ConvertCountToDirectScan is⦠⦠non-nullable You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4490 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/423.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #423 commit 0bf3f1c6419582e750258b8b986d97ff55730199 Author: Hsuan-Yi Chu Date: 2016-03-10T01:25:11Z DRILL-4490: Ensure the count generated by ConvertCountToDirectScan is non-nullable --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4490: Ensure the count generated by Conv...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/423#issuecomment-194618931 @jinfengni Can you help review? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/407#issuecomment-194598924 @amansinha100 I consolidated the two logic into one: https://github.com/hsuanyi/incubator-drill/blob/DRILL-4476/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java#L597 Can you help review again? Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/407#discussion_r55605493
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java
---
@@ -491,6 +556,25 @@ private void inferOutputFieldsFromLeftSide() {
}
}
+private void inferOutputFieldsFromRightSide() {
--- End diff --
Inferencing happens only once when Drill receives the very first batches
from left and right. (Schema-change is not yet supported for Union-All).
Let me summarize the inference in four different situations:
First of all, the field names are always determined by the left side (even
when the left side is from empty file, we have the column names. Please see the
comment above.)
1. Left: non-empty; Right: non-empty=> types determined by both sides with
implicit casting involved
2. Left: empty; Right: non-empty=> type from the right
3. Left: non-empty; Right: empty=> types from the left
4. Left: empty; Right: empty=> types are nullable integer
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/407#discussion_r55597867
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java
---
@@ -294,13 +313,41 @@ public UnionAllInput(UnionAllRecordBatch
unionAllRecordBatch, RecordBatch left,
rightSide = new OneSideInput(right);
}
+private boolean isBothSideEmpty() {
+ return leftIsFinish && rightIsFinish;
+}
+
public IterOutcome nextBatch() throws SchemaChangeException {
if(upstream == RecordBatch.IterOutcome.NOT_YET) {
IterOutcome iterLeft = leftSide.nextBatch();
switch(iterLeft) {
case OK_NEW_SCHEMA:
-break;
+whileLoop:
+while(leftSide.getRecordBatch().getRecordCount() == 0) {
+ iterLeft = leftSide.nextBatch();
+
+ switch(iterLeft) {
+case STOP:
+case OUT_OF_MEMORY:
+ return iterLeft;
+
+case NONE:
--- End diff --
The while is for the case where the first few batches are empty and then
union-all receive NONE.
So we need to replicate this logic to right side too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/407#discussion_r55597640
--- Diff: exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java
---
@@ -527,6 +532,52 @@ public void testUnionAllRightEmptyJson() throws
Exception {
.build().run();
}
+ @Test
+ public void testUnionAllLeftEmptyJson() throws Exception {
--- End diff --
Added a few for both union-all and union-distinct
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/407#discussion_r55596051
--- Diff: exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java
---
@@ -527,6 +532,52 @@ public void testUnionAllRightEmptyJson() throws
Exception {
.build().run();
}
+ @Test
+ public void testUnionAllLeftEmptyJson() throws Exception {
+final String rootEmpty =
FileUtils.getResourceAsFile("/project/pushdown/empty.json").toURI().toString();
+final String rootSimple =
FileUtils.getResourceAsFile("/store/json/booleanData.json").toURI().toString();
+
+final String queryRightEmpty = String.format(
--- End diff --
addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/407#discussion_r55595195
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java
---
@@ -162,6 +162,25 @@ private IterOutcome doWork() throws
ClassTransformationException, IOException, S
allocationVectors = Lists.newArrayList();
transfers.clear();
+// If both sides of Union-All are empty
+if(unionAllInput.isBothSideEmpty()) {
+ for(int i = 0; i < outputFields.size(); ++i) {
--- End diff --
For example, you could do:
select a, b, c from empty union all select a, b, c from empty
Then, the output should be (a, b, c) as column names.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
Github user hsuanyi commented on the pull request: https://github.com/apache/drill/pull/407#issuecomment-192523111 @amansinha100 can you help review this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4476: Allow UnionAllRecordBatch to manag...
GitHub user hsuanyi opened a pull request: https://github.com/apache/drill/pull/407 DRILL-4476: Allow UnionAllRecordBatch to manager situations where lef⦠â¦t input side or both sides come(s) from empty source(s). You can merge this pull request into a Git repository by running: $ git pull https://github.com/hsuanyi/incubator-drill DRILL-4476 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/407.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #407 commit 5a384a14f23827aee8b20a8ac6529b33bc66ee2e Author: Hsuan-Yi Chu Date: 2016-03-04T21:50:02Z DRILL-4476: Allow UnionAllRecordBatch to manager situations where left input side or both sides come(s) from empty source(s). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r54965064
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
---
@@ -17,40 +17,60 @@
*/
package org.apache.drill.exec.expr.fn;
-import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Map.Entry;
-import java.util.Set;
-import org.apache.calcite.sql.SqlOperator;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import org.apache.commons.lang3.tuple.Pair;
import
org.apache.drill.common.scanner.persistence.AnnotatedClassDescriptor;
import org.apache.drill.common.scanner.persistence.ScanResult;
-import org.apache.drill.exec.expr.DrillFunc;
import org.apache.drill.exec.planner.logical.DrillConstExecutor;
import org.apache.drill.exec.planner.sql.DrillOperatorTable;
import org.apache.drill.exec.planner.sql.DrillSqlAggOperator;
import org.apache.drill.exec.planner.sql.DrillSqlOperator;
import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.Sets;
+/**
+ * Registry of Drill functions.
+ */
public class DrillFunctionRegistry {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillFunctionRegistry.class);
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillFunctionRegistry.class);
- private ArrayListMultimap methods =
ArrayListMultimap.create();
+ // key: function name (lowercase) value: list of functions with that name
+ private final ArrayListMultimap
registeredFunctions = ArrayListMultimap.create();
- /* Hash map to prevent registering functions with exactly matching
signatures
- * key: Function Name + Input's Major Type
- * Value: Class name where function is implemented
- */
- private HashMap functionSignatureMap = new HashMap<>();
+ private static final Map>
drillFuncToRange = Maps.newHashMap();
+ static {
+// CONCAT is allowed to take [1, infinity) number of arguments.
+// Currently, this flexibility is offered by DrillOptiq to rewrite it
as
+// a nested structure
+drillFuncToRange.put("CONCAT", Pair.of(1, Integer.MAX_VALUE));
+
+// When LENGTH is given two arguments, this function relies on
DrillOptiq to rewrite it as
+// another function based on the second argument (encodingType)
+drillFuncToRange.put("LENGTH", Pair.of(1, 2));
+
+// Dummy functions
--- End diff --
Concat is due to the flexibility of variable # of arguments
LENGTH is.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 review
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/397#discussion_r54964989
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java
---
@@ -0,0 +1,568 @@
+/**
+ * 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.sql;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+
+import com.google.common.collect.Maps;
+import org.apache.calcite.avatica.util.TimeUnit;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlCharStringLiteral;
+import org.apache.calcite.sql.SqlDynamicParam;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.type.SqlReturnTypeInference;
+import org.apache.calcite.sql.type.SqlTypeName;
+
+import org.apache.drill.common.expression.ExpressionPosition;
+import org.apache.drill.common.expression.FunctionCall;
+import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.expression.MajorTypeInLogicalExpression;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.common.types.Types;
+import org.apache.drill.exec.expr.TypeHelper;
+import org.apache.drill.exec.expr.fn.DrillFuncHolder;
+import org.apache.drill.exec.planner.logical.DrillConstExecutor;
+import org.apache.drill.exec.resolver.FunctionResolver;
+import org.apache.drill.exec.resolver.FunctionResolverFactory;
+import org.apache.drill.exec.resolver.TypeCastRules;
+
+import java.util.List;
+import java.util.Map;
+
+public class TypeInferenceUtils {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(TypeInferenceUtils.class);
+
+ public static final TypeProtos.MajorType UNKNOWN_TYPE =
TypeProtos.MajorType.getDefaultInstance();
+ private static ImmutableMap
DRILL_TO_CALCITE_TYPE_MAPPING =
+ ImmutableMap. builder()
+ .put(TypeProtos.MinorType.INT, SqlTypeName.INTEGER)
+ .put(TypeProtos.MinorType.BIGINT, SqlTypeName.BIGINT)
+ .put(TypeProtos.MinorType.FLOAT4, SqlTypeName.FLOAT)
+ .put(TypeProtos.MinorType.FLOAT8, SqlTypeName.DOUBLE)
+ .put(TypeProtos.MinorType.VARCHAR, SqlTypeName.VARCHAR)
+ .put(TypeProtos.MinorType.BIT, SqlTypeName.BOOLEAN)
+ .put(TypeProtos.MinorType.DATE, SqlTypeName.DATE)
+ .put(TypeProtos.MinorType.DECIMAL9, SqlTypeName.DECIMAL)
+ .put(TypeProtos.MinorType.DECIMAL18, SqlTypeName.DECIMAL)
+ .put(TypeProtos.MinorType.DECIMAL28SPARSE, SqlTypeName.DECIMAL)
+ .put(TypeProtos.MinorType.DECIMAL38SPARSE, SqlTypeName.DECIMAL)
+ .put(TypeProtos.MinorType.TIME, SqlTypeName.TIME)
+ .put(TypeProtos.MinorType.TIMESTAMP, SqlTypeName.TIMESTAMP)
+ .put(TypeProtos.MinorType.VARBINARY, SqlTypeName.VARBINARY)
+ .put(TypeProtos.MinorType.INTERVALYEAR,
SqlTypeName.INTERVAL_YEAR_MONTH)
+ .put(TypeProtos.MinorType.INTERVALDAY,
SqlTypeName.INTERVAL_DAY_TIME)
+ .put(TypeProtos.MinorType.MAP, SqlTypeName.MAP)
+ .put(TypeProtos.MinorType.LIST, SqlTypeName.ARRAY)
+ .put(TypeProtos.MinorType.LATE, SqlTypeName.ANY)
+ .build();
+
+ private static ImmutableMap
CALCITE_TO_DRILL_MAPPING =
+ ImmutableMap. builder()
+ .put(SqlTypeName.INTEGER, TypeProtos.MinorType.INT)
+ .put(SqlTypeName.BIGINT, TypeProtos.MinorType.BIGINT)
+ .put(SqlTypeName.FLOAT, TypeProtos.MinorType.FLOAT4)
+ .put(SqlTypeName.DOUBLE, TypeProtos.MinorType.FLOAT8)
+ .put
