drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From meh...@apache.org
Subject drill git commit: DRILL-3464: Fix IOOB in DrillOptiq while converting concat to Drill logical expression
Date Wed, 08 Jul 2015 18:32:40 GMT
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 <mehantr@gmail.com>
Authored: Tue Jul 7 13:40:24 2015 -0700
Committer: Mehant Baid <mehantr@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();
+  }
 }


Mime
View raw message