This is an automated email from the ASF dual-hosted git repository.
arina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
commit f4f4dc54520e33f0b880334ff97a8a55078e9d1c
Author: Bohdan Kazydub <bohdan.kazy...@gmail.com>
AuthorDate: Thu May 17 13:08:31 2018 +0300
DRILL-4525: Code cleanup
- Fixed failing test
closes #1268
---
.../sql/DrillCalciteSqlBetweenOperatorWrapper.java | 28 ++++++---------
.../drill/exec/planner/sql/DrillOperatorTable.java | 41 +++++++++++-----------
.../drill/TestFunctionsWithTypeExpoQueries.java | 29 +++++++--------
3 files changed, 46 insertions(+), 52 deletions(-)
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillCalciteSqlBetweenOperatorWrapper.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillCalciteSqlBetweenOperatorWrapper.java
index e26da95..f112e57 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillCalciteSqlBetweenOperatorWrapper.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillCalciteSqlBetweenOperatorWrapper.java
@@ -1,4 +1,4 @@
-/**
+/*
* 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
@@ -17,18 +17,14 @@
*/
package org.apache.drill.exec.planner.sql;
-import com.google.common.collect.Lists;
-
-import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.sql.fun.SqlBetweenOperator;
import org.apache.calcite.sql.SqlCallBinding;
import org.apache.calcite.sql.SqlOperator;
-import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.drill.common.types.TypeProtos;
import org.apache.drill.exec.resolver.TypeCastRules;
+import java.util.ArrayList;
import java.util.List;
/**
@@ -55,26 +51,24 @@ public class DrillCalciteSqlBetweenOperatorWrapper extends
SqlBetweenOperator im
}
/**
- *
- * Since Calcite has its rule for type compatibility (see {@link
SqlTypeUtil#isComparable(RelDataType, RelDataType)}),
- * which is usually different from Drill's, this method is overridden here
to avoid Calcite early terminating the
- * queries.
+ * Since Calcite has its rule for type compatibility
+ * (see {@link
org.apache.calcite.sql.type.SqlTypeUtil#isComparable(org.apache.calcite.rel.type.RelDataType,
+ * org.apache.calcite.rel.type.RelDataType)}), which is usually different
from Drill's, this method is overridden here to avoid
+ * Calcite early terminating the queries.
*/
@Override
- public boolean checkOperandTypes(
- SqlCallBinding callBinding,
- boolean throwOnFailure) {
- final List<TypeProtos.MinorType> types = Lists.newArrayList();
- for(int i = 0; i < callBinding.getOperandCount(); ++i) {
+ public boolean checkOperandTypes(SqlCallBinding callBinding, boolean
throwOnFailure) {
+ final List<TypeProtos.MinorType> types = new ArrayList<>();
+ for (int i = 0; i < callBinding.getOperandCount(); i++) {
final TypeProtos.MinorType inMinorType =
TypeInferenceUtils.getDrillTypeFromCalciteType(callBinding.getOperandType(i));
- if(inMinorType == TypeProtos.MinorType.LATE) {
+ if (inMinorType == TypeProtos.MinorType.LATE) {
return true;
}
types.add(inMinorType);
}
final boolean isCompatible = TypeCastRules.getLeastRestrictiveType(types)
!= null;
- if(!isCompatible && throwOnFailure) {
+ if (!isCompatible && throwOnFailure) {
throw callBinding.newValidationSignatureError();
}
return isCompatible;
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
index d533c19..aea73d8 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillOperatorTable.java
@@ -44,7 +44,6 @@ import java.util.Map;
* {@link #inner SqlStdOperatorTable}, and Drill User Defined Functions.
*/
public class DrillOperatorTable extends SqlStdOperatorTable {
-// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillOperatorTable.class);
private static final SqlOperatorTable inner = SqlStdOperatorTable.instance();
private final List<SqlOperator> calciteOperators = Lists.newArrayList();
private final List<SqlOperator> drillOperatorsWithoutInference =
Lists.newArrayList();
@@ -102,8 +101,8 @@ public class DrillOperatorTable extends SqlStdOperatorTable
{
@Override
public void lookupOperatorOverloads(SqlIdentifier opName,
SqlFunctionCategory category,
- SqlSyntax syntax, List<SqlOperator> operatorList) {
- if(isInferenceEnabled()) {
+ SqlSyntax syntax, List<SqlOperator>
operatorList) {
+ if (isInferenceEnabled()) {
populateFromTypeInference(opName, category, syntax, operatorList);
} else {
populateFromWithoutTypeInference(opName, category, syntax, operatorList);
@@ -111,12 +110,12 @@ public class DrillOperatorTable extends
SqlStdOperatorTable {
}
private void populateFromTypeInference(SqlIdentifier opName,
SqlFunctionCategory category,
- SqlSyntax syntax, List<SqlOperator> operatorList) {
+ SqlSyntax syntax, List<SqlOperator>
operatorList) {
final List<SqlOperator> calciteOperatorList = Lists.newArrayList();
inner.lookupOperatorOverloads(opName, category, syntax,
calciteOperatorList);
- if(!calciteOperatorList.isEmpty()) {
- for(SqlOperator calciteOperator : calciteOperatorList) {
- if(calciteToWrapper.containsKey(calciteOperator)) {
+ if (!calciteOperatorList.isEmpty()) {
+ for (SqlOperator calciteOperator : calciteOperatorList) {
+ if (calciteToWrapper.containsKey(calciteOperator)) {
operatorList.add(calciteToWrapper.get(calciteOperator));
} else {
operatorList.add(calciteOperator);
@@ -134,7 +133,7 @@ public class DrillOperatorTable extends SqlStdOperatorTable
{
}
private void populateFromWithoutTypeInference(SqlIdentifier opName,
SqlFunctionCategory category,
- SqlSyntax syntax, List<SqlOperator> operatorList) {
+ SqlSyntax syntax,
List<SqlOperator> operatorList) {
inner.lookupOperatorOverloads(opName, category, syntax, operatorList);
if (operatorList.isEmpty() && (syntax == SqlSyntax.FUNCTION || syntax ==
SqlSyntax.FUNCTION_ID) && opName.isSimple()) {
List<SqlOperator> drillOps =
drillOperatorsWithoutInferenceMap.get(opName.getSimple().toLowerCase());
@@ -148,7 +147,7 @@ public class DrillOperatorTable extends SqlStdOperatorTable
{
public List<SqlOperator> getOperatorList() {
final List<SqlOperator> sqlOperators = Lists.newArrayList();
sqlOperators.addAll(calciteOperators);
- if(isInferenceEnabled()) {
+ if (isInferenceEnabled()) {
sqlOperators.addAll(drillOperatorsWithInference);
} else {
sqlOperators.addAll(drillOperatorsWithoutInference);
@@ -159,7 +158,7 @@ public class DrillOperatorTable extends SqlStdOperatorTable
{
// Get the list of SqlOperator's with the given name.
public List<SqlOperator> getSqlOperator(String name) {
- if(isInferenceEnabled()) {
+ if (isInferenceEnabled()) {
return drillOperatorsWithInferenceMap.get(name.toLowerCase());
} else {
return drillOperatorsWithoutInferenceMap.get(name.toLowerCase());
@@ -167,15 +166,15 @@ public class DrillOperatorTable extends
SqlStdOperatorTable {
}
private void populateWrappedCalciteOperators() {
- for(SqlOperator calciteOperator : inner.getOperatorList()) {
+ for (SqlOperator calciteOperator : inner.getOperatorList()) {
final SqlOperator wrapper;
- if(calciteOperator instanceof SqlAggFunction) {
+ if (calciteOperator instanceof SqlAggFunction) {
wrapper = new DrillCalciteSqlAggFunctionWrapper((SqlAggFunction)
calciteOperator,
getFunctionListWithInference(calciteOperator.getName()));
- } else if(calciteOperator instanceof SqlFunction) {
+ } else if (calciteOperator instanceof SqlFunction) {
wrapper = new DrillCalciteSqlFunctionWrapper((SqlFunction)
calciteOperator,
getFunctionListWithInference(calciteOperator.getName()));
- } else if(calciteOperator instanceof SqlBetweenOperator) {
+ } 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;
@@ -184,14 +183,14 @@ public class DrillOperatorTable extends
SqlStdOperatorTable {
final String drillOpName;
// For UNARY_MINUS (-) or UNARY_PLUS (+), we do not rename them as
function_add or function_subtract.
// Otherwise, Calcite will mix them up with binary operator subtract
(-) or add (+)
- if(calciteOperator == SqlStdOperatorTable.UNARY_MINUS ||
calciteOperator == SqlStdOperatorTable.UNARY_PLUS) {
+ if (calciteOperator == SqlStdOperatorTable.UNARY_MINUS ||
calciteOperator == SqlStdOperatorTable.UNARY_PLUS) {
drillOpName = calciteOperator.getName();
} else {
drillOpName =
FunctionCallFactory.replaceOpWithFuncName(calciteOperator.getName());
}
final List<DrillFuncHolder> drillFuncHolders =
getFunctionListWithInference(drillOpName);
- if(drillFuncHolders.isEmpty()) {
+ if (drillFuncHolders.isEmpty()) {
continue;
}
@@ -203,17 +202,17 @@ public class DrillOperatorTable extends
SqlStdOperatorTable {
private List<DrillFuncHolder> getFunctionListWithInference(String name) {
final List<DrillFuncHolder> functions = Lists.newArrayList();
- for(SqlOperator sqlOperator :
drillOperatorsWithInferenceMap.get(name.toLowerCase())) {
- if(sqlOperator instanceof DrillSqlOperator) {
+ for (SqlOperator sqlOperator :
drillOperatorsWithInferenceMap.get(name.toLowerCase())) {
+ if (sqlOperator instanceof DrillSqlOperator) {
final List<DrillFuncHolder> list = ((DrillSqlOperator)
sqlOperator).getFunctions();
- if(list != null) {
+ if (list != null) {
functions.addAll(list);
}
}
- if(sqlOperator instanceof DrillSqlAggOperator) {
+ if (sqlOperator instanceof DrillSqlAggOperator) {
final List<DrillFuncHolder> list = ((DrillSqlAggOperator)
sqlOperator).getFunctions();
- if(list != null) {
+ if (list != null) {
functions.addAll(list);
}
}
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
index 74676de..1f30e24 100644
---
a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
+++
b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
@@ -31,6 +31,7 @@ import org.junit.Test;
import org.junit.experimental.categories.Category;
import java.nio.file.Paths;
+import java.util.ArrayList;
import java.util.List;
@Category(SqlFunctionTest.class)
@@ -412,9 +413,9 @@ public class TestFunctionsWithTypeExpoQueries extends
BaseTestQuery {
final List<Pair<SchemaPath, TypeProtos.MajorType>> expectedSchema =
Lists.newArrayList();
final TypeProtos.MajorType majorType = TypeProtos.MajorType.newBuilder()
- .setMinorType(TypeProtos.MinorType.TIMESTAMP)
- .setMode(TypeProtos.DataMode.REQUIRED)
- .build();
+ .setMinorType(TypeProtos.MinorType.TIMESTAMP)
+ .setMode(TypeProtos.DataMode.REQUIRED)
+ .build();
expectedSchema.add(Pair.of(SchemaPath.getSimplePath("col"), majorType));
testBuilder()
@@ -546,19 +547,19 @@ public class TestFunctionsWithTypeExpoQueries extends
BaseTestQuery {
final List<Pair<SchemaPath, TypeProtos.MajorType>> expectedSchema =
Lists.newArrayList();
final TypeProtos.MajorType majorType1 = TypeProtos.MajorType.newBuilder()
- .setMinorType(TypeProtos.MinorType.BIGINT)
- .setMode(TypeProtos.DataMode.OPTIONAL)
- .build();
+ .setMinorType(TypeProtos.MinorType.BIGINT)
+ .setMode(TypeProtos.DataMode.OPTIONAL)
+ .build();
final TypeProtos.MajorType majorType2 = TypeProtos.MajorType.newBuilder()
- .setMinorType(TypeProtos.MinorType.FLOAT8)
- .setMode(TypeProtos.DataMode.OPTIONAL)
- .build();
+ .setMinorType(TypeProtos.MinorType.FLOAT8)
+ .setMode(TypeProtos.DataMode.OPTIONAL)
+ .build();
final TypeProtos.MajorType majorType3 = TypeProtos.MajorType.newBuilder()
- .setMinorType(TypeProtos.MinorType.BIGINT)
- .setMode(TypeProtos.DataMode.REQUIRED)
- .build();
+ .setMinorType(TypeProtos.MinorType.BIGINT)
+ .setMode(TypeProtos.DataMode.REQUIRED)
+ .build();
expectedSchema.add(Pair.of(SchemaPath.getSimplePath("col1"), majorType1));
expectedSchema.add(Pair.of(SchemaPath.getSimplePath("col2"), majorType2));
expectedSchema.add(Pair.of(SchemaPath.getSimplePath("col3"), majorType3));
@@ -753,7 +754,7 @@ public class TestFunctionsWithTypeExpoQueries extends
BaseTestQuery {
.setMode(TypeProtos.DataMode.REQUIRED)
.build();
- final List<Pair<SchemaPath, TypeProtos.MajorType>> expectedSchema =
Lists.newArrayList();
+ final List<Pair<SchemaPath, TypeProtos.MajorType>> expectedSchema = new
ArrayList<>();
expectedSchema.add(Pair.of(SchemaPath.getSimplePath("col"), majorType));
testBuilder()
@@ -775,7 +776,7 @@ public class TestFunctionsWithTypeExpoQueries extends
BaseTestQuery {
.setMode(TypeProtos.DataMode.OPTIONAL)
.build();
- final List<Pair<SchemaPath, TypeProtos.MajorType>> expectedSchema =
Lists.newArrayList();
+ final List<Pair<SchemaPath, TypeProtos.MajorType>> expectedSchema = new
ArrayList<>();
expectedSchema.add(Pair.of(SchemaPath.getSimplePath("col"), majorType));
testBuilder()
--
To stop receiving notification emails like this one, please contact
ar...@apache.org.
