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.