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"}
|