You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by me...@apache.org on 2015/07/08 20:32:40 UTC
drill git commit: DRILL-3464: Fix IOOB in DrillOptiq while converting
concat to Drill logical expression
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 <me...@gmail.com>
Authored: Tue Jul 7 13:40:24 2015 -0700
Committer: Mehant Baid <me...@gmail.com>
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();
+ }
}