You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by su...@apache.org on 2016/10/10 21:46:50 UTC
[1/2] drill git commit: DRILL-4862: Fix binary_string to use an
injected buffer as out buffer
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 <cs...@maprtech.com>
Authored: Thu Aug 25 10:23:53 2016 -0700
Committer: Sudheesh Katkam <sk...@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"}
[2/2] drill git commit: DRILL-4618: Correct the usage of random flag
in Hive function registry
Posted by su...@apache.org.
DRILL-4618: Correct the usage of random flag in Hive function registry
+ Function visitor should not use previous function holder if this function is non-deterministic
closes #509
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/50dea898
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/50dea898
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/50dea898
Branch: refs/heads/master
Commit: 50dea8984bf145337dd34bf08c99ea3dff70e791
Parents: 4edabe7
Author: chunhui-shi <cs...@maprtech.com>
Authored: Wed May 25 23:22:01 2016 -0700
Committer: Sudheesh Katkam <sk...@maprtech.com>
Committed: Mon Oct 10 11:57:37 2016 -0700
----------------------------------------------------------------------
.../exec/expr/fn/HiveFunctionRegistry.java | 22 +++++++++++++++++++-
.../drill/exec/fn/hive/TestInbuiltHiveUDFs.java | 10 +++++++++
.../apache/drill/exec/expr/EqualityVisitor.java | 3 +++
.../drill/exec/expr/EvaluationVisitor.java | 2 +-
.../org/apache/drill/TestFunctionsQuery.java | 11 ++++++++++
5 files changed, 46 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/50dea898/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
index c716e9e..8d8707e 100644
--- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
+++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
@@ -18,6 +18,7 @@
package org.apache.drill.exec.expr.fn;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
import org.apache.calcite.rel.type.RelDataType;
@@ -73,6 +74,25 @@ public class HiveFunctionRegistry implements PluggableFunctionRegistry{
for (Class<? extends UDF> clazz : udfClasses) {
register(clazz, methodsUDF);
}
+
+ if (logger.isTraceEnabled()) {
+ StringBuilder allHiveFunctions = new StringBuilder();
+ for (Map.Entry<String, Class<? extends GenericUDF>> method : methodsGenericUDF.entries()) {
+ allHiveFunctions.append(method.toString()).append("\n");
+ }
+ logger.trace("Registered Hive GenericUDFs: [\n{}]", allHiveFunctions);
+
+ StringBuilder allUDFs = new StringBuilder();
+ for (Map.Entry<String, Class<? extends UDF>> method : methodsUDF.entries()) {
+ allUDFs.append(method.toString()).append("\n");
+ }
+ logger.trace("Registered Hive UDFs: [\n{}]", allUDFs);
+ StringBuilder allNonDeterministic = new StringBuilder();
+ for (Class<?> clz : nonDeterministicUDFs) {
+ allNonDeterministic.append(clz.toString()).append("\n");
+ }
+ logger.trace("Registered Hive nonDeterministicUDFs: [\n{}]", allNonDeterministic);
+ }
}
@Override
@@ -96,7 +116,7 @@ public class HiveFunctionRegistry implements PluggableFunctionRegistry{
}
UDFType type = clazz.getAnnotation(UDFType.class);
- if (type != null && type.deterministic()) {
+ if (type != null && !type.deterministic()) {
nonDeterministicUDFs.add(clazz);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/50dea898/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java b/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
index 9e13844..0eb4116 100644
--- a/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
+++ b/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
@@ -94,4 +94,14 @@ public class TestInbuiltHiveUDFs extends HiveTestBase {
.go();
}
+ @Test // DRILL-4618
+ public void testRand() throws Exception {
+ String query = "select 2*rand()=2*rand() col1 from (values (1))";
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("col1")
+ .baselineValues(false)
+ .go();
+ }
}
http://git-wip-us.apache.org/repos/asf/drill/blob/50dea898/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java
index 7945bb4..433e95f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java
@@ -75,6 +75,9 @@ class EqualityVisitor extends AbstractExprVisitor<Boolean,LogicalExpression,Runt
if (!holder.getName().equals(((FunctionHolderExpression) value).getName())) {
return false;
}
+ if (holder.isRandom()) {
+ return false;
+ }
return checkChildren(holder, value);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/50dea898/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
index 055ab84..53bd8b8 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
@@ -811,7 +811,7 @@ public class EvaluationVisitor {
@Override
public HoldingContainer visitFunctionHolderExpression(FunctionHolderExpression holder, ClassGenerator<?> generator) throws RuntimeException {
HoldingContainer hc = getPrevious(holder, generator.getMappingSet());
- if (hc == null) {
+ if (hc == null || holder.isRandom()) {
hc = super.visitFunctionHolderExpression(holder, generator);
put(holder, hc, generator.getMappingSet());
}
http://git-wip-us.apache.org/repos/asf/drill/blob/50dea898/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 475d08a..8be8781 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
@@ -912,4 +912,15 @@ public class TestFunctionsQuery extends BaseTestQuery {
.baselineValues("foo")
.go();
}
+
+ @Test
+ public void testRandom() throws Exception {
+ String query = "select 2*random()=2*random() as col1 from (values (1))";
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("col1")
+ .baselineValues(false)
+ .go();
+ }
}