Repository: drill Updated Branches: refs/heads/master 2506ecf15 -> b6577feda
DRILL-3464: Fix IOOB in DrillOptiq while converting concat to Drill logical expression Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/b6577fed Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/b6577fed Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/b6577fed Branch: refs/heads/master Commit: b6577feda6763ef87d72d5913c7324c9aa2cfc3d Parents: 2506ecf Author: Mehant Baid <[email protected]> Authored: Tue Jul 7 13:40:24 2015 -0700 Committer: Mehant Baid <[email protected]> Committed: Wed Jul 8 09:44:58 2015 -0700 ---------------------------------------------------------------------- .../drill/exec/planner/logical/DrillOptiq.java | 54 ++++++++++++++------ .../org/apache/drill/TestFunctionsQuery.java | 30 +++++++++++ 2 files changed, 67 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/b6577fed/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java index 1b675bb..11b9c9e 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java @@ -19,6 +19,7 @@ package org.apache.drill.exec.planner.logical; import java.math.BigDecimal; import java.util.GregorianCalendar; +import java.util.LinkedList; import java.util.List; import org.apache.drill.common.exceptions.UserException; @@ -293,9 +294,12 @@ public class DrillOptiq { private LogicalExpression getDrillFunctionFromOptiqCall(RexCall call) { List<LogicalExpression> args = Lists.newArrayList(); + for(RexNode n : call.getOperands()){ args.add(n.accept(this)); } + + int argsSize = args.size(); String functionName = call.getOperator().getName().toLowerCase(); // TODO: once we have more function rewrites and a patter emerges from different rewrites, factor this out in a better fashion @@ -347,14 +351,14 @@ public class DrillOptiq { return FunctionCallFactory.createExpression(trimFunc, trimArgs); } else if (functionName.equals("ltrim") || functionName.equals("rtrim") || functionName.equals("btrim")) { - if (args.size() == 1) { + if (argsSize == 1) { args.add(ValueExpressions.getChar(" ")); } return FunctionCallFactory.createExpression(functionName, args); } else if (functionName.equals("date_part")) { // Rewrite DATE_PART functions as extract functions // assert that the function has exactly two arguments - assert args.size() == 2; + assert argsSize == 2; /* Based on the first input to the date_part function we rewrite the function as the * appropriate extract function. For example @@ -367,24 +371,40 @@ public class DrillOptiq { return FunctionCallFactory.createExpression("extract" + functionPostfix, args.subList(1, 2)); } else if (functionName.equals("concat")) { - // Cast arguments to VARCHAR - List<LogicalExpression> concatArgs = Lists.newArrayList(); - concatArgs.add(args.get(0)); - concatArgs.add(args.get(1)); - - LogicalExpression first = FunctionCallFactory.createExpression(functionName, concatArgs); + if (argsSize == 1) { + /* + * We treat concat with one argument as a special case. Since we don't have a function + * implementation of concat that accepts one argument. We simply add another dummy argument + * (empty string literal) to the list of arguments. + */ + List<LogicalExpression> concatArgs = new LinkedList<>(args); + concatArgs.add(new QuotedString("", ExpressionPosition.UNKNOWN)); + + return FunctionCallFactory.createExpression(functionName, concatArgs); + + } else if (argsSize > 2) { + List<LogicalExpression> concatArgs = Lists.newArrayList(); + + /* stack concat functions on top of each other if we have more than two arguments + * Eg: concat(col1, col2, col3) => concat(concat(col1, col2), col3) + */ + concatArgs.add(args.get(0)); + concatArgs.add(args.get(1)); + + LogicalExpression first = FunctionCallFactory.createExpression(functionName, concatArgs); + + for (int i = 2; i < argsSize; i++) { + concatArgs = Lists.newArrayList(); + concatArgs.add(first); + concatArgs.add(args.get(i)); + first = FunctionCallFactory.createExpression(functionName, concatArgs); + } - for (int i = 2; i < args.size(); i++) { - concatArgs = Lists.newArrayList(); - concatArgs.add(first); - concatArgs.add(args.get(i)); - first = FunctionCallFactory.createExpression(functionName, concatArgs); + return first; } - - return first; } else if (functionName.equals("length")) { - if (args.size() == 2) { + if (argsSize == 2) { // Second argument should always be a literal specifying the encoding format assert args.get(1) instanceof ValueExpressions.QuotedString; @@ -399,7 +419,7 @@ public class DrillOptiq { return FunctionCallFactory.createConvert(functionName, ((QuotedString)args.get(1)).value, args.get(0), ExpressionPosition.UNKNOWN); } else if ((functionName.equalsIgnoreCase("rpad")) || functionName.equalsIgnoreCase("lpad")) { // If we have only two arguments for rpad/lpad append a default QuotedExpression as an argument which will be used to pad the string - if (args.size() == 2) { + if (argsSize == 2) { String spaceFill = " "; LogicalExpression fill = ValueExpressions.getChar(spaceFill); args.add(fill); http://git-wip-us.apache.org/repos/asf/drill/blob/b6577fed/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java index a31e8db..e81c661 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java @@ -853,4 +853,34 @@ public class TestFunctionsQuery extends BaseTestQuery { .baselineValues(2001l, 1.2d) .go(); } + + @Test + public void testConcatSingleInput() throws Exception { + String query = "select concat(employee_id) as col1 from cp.`employee.json` where employee_id = 1"; + + testBuilder() + .sqlQuery(query) + .unOrdered() + .baselineColumns("col1") + .baselineValues("1") + .go(); + + query = "select concat(null_column) as col1 from cp.`employee.json` where employee_id = 1"; + + testBuilder() + .sqlQuery(query) + .unOrdered() + .baselineColumns("col1") + .baselineValues("") + .go(); + + query = "select concat('foo') as col1 from cp.`employee.json` where employee_id = 1"; + + testBuilder() + .sqlQuery(query) + .unOrdered() + .baselineColumns("col1") + .baselineValues("foo") + .go(); + } }
