drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sudhe...@apache.org
Subject [1/2] drill git commit: DRILL-4862: Fix binary_string to use an injected buffer as out buffer
Date Mon, 10 Oct 2016 21:46:50 GMT
Repository: drill
Updated Branches:
  refs/heads/master 4edabe7a3 -> 46b424cbd


DRILL-4862: Fix binary_string to use an injected buffer as out buffer

+ Previously, binary_string used the input buffer as output buffer. So after calling binary_string,
the original content was destroyed. Other expressions/ functions that need to access the original
input buffer get wrong results.
+ This fix also sets readerIndex and writerIndex correctly for the output buffer, otherwise
the consumer of the output buffer will hit issues.

closes #604


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

Branch: refs/heads/master
Commit: 46b424cbd7747685052ad512f24e38a94ac61202
Parents: 50dea89
Author: chunhui-shi <cshi@maprtech.com>
Authored: Thu Aug 25 10:23:53 2016 -0700
Committer: Sudheesh Katkam <skatkam@maprtech.com>
Committed: Mon Oct 10 11:57:37 2016 -0700

----------------------------------------------------------------------
 .../drill/common/util/DrillStringUtils.java     | 13 ++++-----
 .../exec/expr/fn/impl/StringFunctions.java      |  7 +++--
 .../physical/impl/TestConvertFunctions.java     | 30 ++++++++++++++++++++
 .../src/test/resources/functions/conv/conv.json |  4 +++
 4 files changed, 44 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/46b424cb/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java b/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
index 4dad397..4e4042f 100644
--- a/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
+++ b/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
@@ -134,7 +134,7 @@ public class DrillStringUtils {
    */
   public static String toBinaryString(byte[] buf, int strStart, int strEnd) {
     StringBuilder result = new StringBuilder();
-    for (int i = strStart; i < strEnd ; ++i) {
+    for (int i = strStart; i < strEnd; ++i) {
       appendByte(result, buf[i]);
     }
     return result.toString();
@@ -153,17 +153,16 @@ public class DrillStringUtils {
   }
 
   /**
-   * In-place parsing of a hex encoded binary string.
+   * parsing a hex encoded binary string and write to an output buffer.
    *
    * This function does not modify  the {@code readerIndex} and {@code writerIndex}
    * of the byte buffer.
    *
    * @return Index in the byte buffer just after the last written byte.
    */
-  public static int parseBinaryString(ByteBuf str, int strStart, int strEnd) {
-    int length = (strEnd - strStart);
-    int dstEnd = strStart;
-    for (int i = strStart; i < strStart+length ; i++) {
+  public static int parseBinaryString(ByteBuf str, int strStart, int strEnd, ByteBuf out)
{
+    int dstEnd = 0;
+    for (int i = strStart; i < strEnd; i++) {
       byte b = str.getByte(i);
       if (b == '\\'
           && strEnd > i+3
@@ -177,7 +176,7 @@ public class DrillStringUtils {
           i += 3; // skip 3
         }
       }
-      str.setByte(dstEnd++, b);
+      out.setByte(dstEnd++, b);
     }
     return dstEnd;
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/46b424cb/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
index 50ff435..8196728 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
@@ -1540,15 +1540,16 @@ public class StringFunctions{
   public static class BinaryString implements DrillSimpleFunc {
     @Param  VarCharHolder in;
     @Output VarBinaryHolder out;
+    @Inject DrillBuf buffer;
 
     @Override
     public void setup() {}
 
     @Override
     public void eval() {
-      out.buffer = in.buffer;
-      out.start = in.start;
-      out.end = org.apache.drill.common.util.DrillStringUtils.parseBinaryString(in.buffer,
in.start, in.end);
+      out.buffer = buffer.reallocIfNeeded(in.end - in.start);
+      out.start = out.end = 0;
+      out.end = org.apache.drill.common.util.DrillStringUtils.parseBinaryString(in.buffer,
in.start, in.end, out.buffer);
       out.buffer.setIndex(out.start, out.end);
     }
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/46b424cb/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
index 80189d5..a0013a0 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
@@ -655,6 +655,36 @@ public class TestConvertFunctions extends BaseTestQuery {
     buffer.release();
   }
 
+  @Test // DRILL-4862
+  public void testBinaryString() throws Exception {
+    // TODO(DRILL-2326) temporary until we fix the scalar replacement bug for this case
+    final OptionValue srOption = setupScalarReplacementOption(bits[0], ScalarReplacementOption.TRY);
+
+    try {
+      final String[] queries = {
+          "SELECT convert_from(binary_string(key), 'INT_BE') as intkey \n" +
+              "FROM cp.`functions/conv/conv.json`"
+      };
+
+      for (String query: queries) {
+        testBuilder()
+            .sqlQuery(query)
+            .ordered()
+            .baselineColumns("intkey")
+            .baselineValues(1244739896)
+            .baselineValues(new Object[] { null })
+            .baselineValues(1313814865)
+            .baselineValues(1852782897)
+            .build()
+            .run();
+      }
+
+    } finally {
+      // restore the system option
+      restoreScalarReplacementOption(bits[0], srOption);
+    }
+  }
+
   protected <T> void verifySQL(String sql, T expectedResults) throws Throwable {
     verifyResults(sql, expectedResults, getRunResult(QueryType.SQL, sql));
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/46b424cb/exec/java-exec/src/test/resources/functions/conv/conv.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/functions/conv/conv.json b/exec/java-exec/src/test/resources/functions/conv/conv.json
new file mode 100644
index 0000000..9e8e667
--- /dev/null
+++ b/exec/java-exec/src/test/resources/functions/conv/conv.json
@@ -0,0 +1,4 @@
+{"row": "0", "key": "\\x4a\\x31\\x39\\x38", "key2": "4a313938", "kp1": "4a31", "kp2": "38"}
+{"row": "1", "key": null, "key2": null, "kp1": null, "kp2": null}
+{"row": "2", "key": "\\x4e\\x4f\\x39\\x51", "key2": "4e4f3951", "kp1": "4e4f", "kp2": "51"}
+{"row": "3", "key": "\\x6e\\x6f\\x39\\x31", "key2": "6e6f3931", "kp1": "6e6f", "kp2": "31"}


Mime
View raw message