You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ja...@apache.org on 2015/05/14 06:42:10 UTC

[5/8] drill git commit: DRILL-3062: Rtrim the fixed char strings in the IN list for comparison with varchar columns.

DRILL-3062:  Rtrim the fixed char strings in the IN list for comparison with varchar columns.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/49940b88
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/49940b88
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/49940b88

Branch: refs/heads/master
Commit: 49940b887aa8c31c7f5a435ce365294b7a082870
Parents: 97a6316
Author: Aman Sinha <as...@maprtech.com>
Authored: Wed May 13 18:31:33 2015 -0700
Committer: Jacques Nadeau <ja...@apache.org>
Committed: Wed May 13 19:34:01 2015 -0700

----------------------------------------------------------------------
 .../drill/exec/planner/logical/DrillValuesRel.java  |  7 ++++++-
 .../physical/impl/filter/TestLargeInClause.java     | 16 ++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/49940b88/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillValuesRel.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillValuesRel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillValuesRel.java
index cd259d7..a6c4661 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillValuesRel.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillValuesRel.java
@@ -183,7 +183,12 @@ public class DrillValuesRel extends AbstractRelNode implements DrillRel {
       if (isLiteralNull(literal)) {
         out.writeVarcharNull();
       }else{
-        out.writeVarChar(((NlsString)literal.getValue()).getValue());
+        // Since Calcite treats string literals as fixed char and adds trailing spaces to the strings to make them the
+        // same length, here we do an rtrim() to get the string without the trailing spaces. If we don't rtrim, the comparison
+        // with Drill's varchar column values would not return a match.
+        // TODO: However, note that if the user had explicitly added spaces in the string literals then even those would get
+        // trimmed, so this exposes another issue that needs to be resolved.
+        out.writeVarChar(((NlsString)literal.getValue()).rtrim().getValue());
       }
       return ;
 

http://git-wip-us.apache.org/repos/asf/drill/blob/49940b88/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java
index 1e29214..281a946 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java
@@ -59,4 +59,20 @@ public class TestLargeInClause extends BaseTestQuery {
     test("select * from cp.`employee.json` where cast(birth_date as date) in (" + getInDateList(500) + ")");
   }
 
+  @Test // DRILL-3062
+  public void testStringLiterals() throws Exception {
+    String query = "select count(*) as cnt from (select n_name from cp.`tpch/nation.parquet` "
+        + " where n_name in ('ALGERIA', 'ARGENTINA', 'BRAZIL', 'CANADA', 'EGYPT', 'ETHIOPIA', 'FRANCE', "
+        + "'GERMANY', 'INDIA', 'INDONESIA', 'IRAN', 'IRAQ', 'JAPAN', 'JORDAN', 'KENYA', 'MOROCCO', 'MOZAMBIQUE', "
+        + "'PERU', 'CHINA', 'ROMANIA', 'SAUDI ARABIA', 'VIETNAM'))";
+
+    testBuilder()
+      .sqlQuery(query)
+      .unOrdered()
+      .baselineColumns("cnt")
+      .baselineValues(22l)
+      .go();
+
+  }
+
 }