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:3854: Ensure the readIndex of DrillBuf[] returned by ValueVector.getBuffers points at 0
Date Wed, 25 Nov 2015 00:14:11 GMT
Repository: drill
Updated Branches:
  refs/heads/master 758cd917b -> bd39d3002


DRILL:3854: Ensure the readIndex of DrillBuf[] returned by ValueVector.getBuffers points at
0


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

Branch: refs/heads/master
Commit: bd39d30024431cf7eb2939f5b336b82c3b1dbe93
Parents: 758cd91
Author: Hsuan-Yi Chu <hsuanyi@usc.edu>
Authored: Tue Nov 17 11:38:34 2015 -0800
Committer: Hsuan-Yi Chu <hsuanyi@usc.edu>
Committed: Tue Nov 24 14:20:26 2015 -0800

----------------------------------------------------------------------
 .../drill/exec/memory/TestAllocators.java       | 63 ++++++++++++++++++++
 .../physical/impl/TestConvertFunctions.java     | 35 +++++++++++
 .../drill/exec/vector/BaseDataValueVector.java  |  2 +-
 3 files changed, 99 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/bd39d300/exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java
index 26b2a4f..350e02b 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java
@@ -19,7 +19,10 @@
 package org.apache.drill.exec.memory;
 
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
+
+import com.google.common.collect.Lists;
 import io.netty.buffer.DrillBuf;
 
 import java.util.Iterator;
@@ -27,6 +30,7 @@ import java.util.List;
 import java.util.Properties;
 
 import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.types.TypeProtos;
 import org.apache.drill.common.util.FileUtils;
 import org.apache.drill.exec.exception.OutOfMemoryException;
 import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry;
@@ -40,10 +44,12 @@ import org.apache.drill.exec.planner.PhysicalPlanReader;
 import org.apache.drill.exec.planner.PhysicalPlanReaderTestFactory;
 import org.apache.drill.exec.proto.BitControl;
 import org.apache.drill.exec.proto.UserBitShared;
+import org.apache.drill.exec.record.MaterializedField;
 import org.apache.drill.exec.server.Drillbit;
 import org.apache.drill.exec.server.DrillbitContext;
 import org.apache.drill.exec.server.RemoteServiceSet;
 import org.apache.drill.exec.store.StoragePluginRegistry;
+import org.apache.drill.exec.vector.IntVector;
 import org.apache.drill.test.DrillTest;
 import org.junit.Test;
 
@@ -62,6 +68,63 @@ public class TestAllocators extends DrillTest {
 
   private final static String planFile="/physical_allocator_test.json";
 
+  /**
+   * Contract for DrillBuf[] returned from getBuffers() is that buffers are returned in a
reader appropriate state
+   * (i.e., readIndex = 0)
+   *
+   * Before this patch, the following scenario breaks this contract:
+   * As data being read from DrillBuf, readIndex will be pushed forward. And, later on,
+   * when DrillBuf[] are read from the ValueVector, readIndex will point at the location
of the most recent reading
+   *
+   * This unit test is added to ensure that the readIndex points at zero under this scenario
+   */
+  @Test // DRILL-3854
+  public void ensureDrillBufReadIndexIsZero() throws Exception {
+    final int length = 10;
+
+    final Properties props = new Properties() {
+      {
+        put(TopLevelAllocator.TOP_LEVEL_MAX_ALLOC, "1000000");
+        put(TopLevelAllocator.ERROR_ON_MEMORY_LEAK, "true");
+      }
+    };
+    final DrillConfig config = DrillConfig.create(props);
+
+    final BufferAllocator allc = RootAllocatorFactory.newRoot(config);
+    final TypeProtos.MajorType.Builder builder = TypeProtos.MajorType.newBuilder();
+    builder.setMinorType(TypeProtos.MinorType.INT);
+    builder.setMode(TypeProtos.DataMode.REQUIRED);
+
+    final IntVector iv = new IntVector(MaterializedField.create("Field", builder.build()),
allc);
+    iv.allocateNew();
+
+    // Write data to DrillBuf
+    for(int i = 0; i < length; ++i) {
+      iv.getBuffer().writeInt(i);
+    }
+
+    // Read data to DrillBuf
+    for(int i = 0; i < length; ++i) {
+      assertEquals(i, iv.getBuffer().readInt());
+    }
+
+    for(DrillBuf drillBuf : iv.getBuffers(false)) {
+      assertEquals(0, drillBuf.readInt());
+    }
+
+    final List<DrillBuf> toBeClean = Lists.newArrayList();
+    for(DrillBuf drillBuf : iv.getBuffers(true)) {
+      assertEquals(0, drillBuf.readInt());
+      toBeClean.add(drillBuf);
+    }
+
+    for(DrillBuf drillBuf : toBeClean) {
+      drillBuf.release();
+    }
+
+    allc.close();
+  }
+
   @Test
   public void testTransfer() throws Exception {
     final Properties props = new Properties() {

http://git-wip-us.apache.org/repos/asf/drill/blob/bd39d300/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 a911e29..aab087d 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
@@ -33,6 +33,7 @@ import mockit.Injectable;
 
 import org.apache.drill.BaseTestQuery;
 import org.apache.drill.TestBuilder;
+import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.compile.ClassTransformer;
 import org.apache.drill.exec.compile.ClassTransformer.ScalarReplacementOption;
 import org.apache.drill.exec.expr.fn.impl.DateUtility;
@@ -75,6 +76,40 @@ public class TestConvertFunctions extends BaseTestQuery {
 
   String textFileContent;
 
+  @Test // DRILL-3854
+  public void testConvertFromConvertToInt() throws Exception {
+    final OptionValue srOption = setupScalarReplacementOption(bits[0], ScalarReplacementOption.OFF);
+    try {
+      final String newTblName = "testConvertFromConvertToInt_tbl";
+      final String ctasQuery = String.format("CREATE TABLE %s.%s as \n" +
+          "SELECT convert_to(r_regionkey, 'INT') as ct \n" +
+          "FROM cp.`tpch/region.parquet`",
+          TEMP_SCHEMA, newTblName);
+      final String query = String.format("SELECT convert_from(ct, 'INT') as cf \n" +
+          "FROM %s.%s \n" +
+          "ORDER BY ct",
+          TEMP_SCHEMA, newTblName);
+
+      test("alter session set `planner.slice_target` = 1");
+      test(ctasQuery);
+      testBuilder()
+          .sqlQuery(query)
+          .ordered()
+          .baselineColumns("cf")
+          .baselineValues(0)
+          .baselineValues(1)
+          .baselineValues(2)
+          .baselineValues(3)
+          .baselineValues(4)
+          .build()
+          .run();
+    } finally {
+      // restore the system option
+      restoreScalarReplacementOption(bits[0], srOption);
+      test("alter session set `planner.slice_target` = " + ExecConstants.SLICE_TARGET_DEFAULT);
+    }
+  }
+
   @Test
   public void test_JSON_convertTo_empty_list_drill_1416() throws Exception {
 

http://git-wip-us.apache.org/repos/asf/drill/blob/bd39d300/exec/vector/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
----------------------------------------------------------------------
diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
b/exec/vector/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
index 2fc741c..f812209 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
@@ -59,8 +59,8 @@ public abstract class BaseDataValueVector extends BaseValueVector {
       out = new DrillBuf[0];
     } else {
       out = new DrillBuf[]{data};
+      data.readerIndex(0);
       if (clear) {
-        data.readerIndex(0);
         data.retain(1);
       }
     }


Mime
View raw message