This is an automated email from the ASF dual-hosted git repository.
arina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
commit e8d9503c6574ceef2876c902117fa2f35e3dd974
Author: Paul Rogers <par0...@yahoo.com>
AuthorDate: Sat Oct 19 23:48:39 2019 -0700
DRILL-7412: Minor unit test improvements
Many tests intentionally trigger errors. A debug-only log setting
sent those errors to stdout. The resulting stack dumps simply cluttered
the test output, so disabled error output to the console.
Drill can apply bounds checks to vectors. Tests run via Maven
enable bounds checking. Now, bounds checking is also enabled in
"debug mode" (when assertions are enabled, as in an IDE.)
Drill contains two test frameworks. The older BaseTestQuery was
marked as deprecated, but many tests still use it and are unlikely
to be changed soon. So, removed the deprecated marker to reduce the
number of spurious warnings.
Also includes a number of minor clean-ups.
closes #1876
---
.../drill/common/exceptions/DrillException.java | 8 +-
.../drill/common/exceptions/UserException.java | 10 +-
common/src/test/resources/logback-test.xml | 2 +-
.../apache/drill/exec/work/foreman/Foreman.java | 2 -
.../drill/exec/work/foreman/ForemanException.java | 4 +-
.../exec/work/foreman/QueryStateProcessor.java | 9 +-
.../drill/exec/coord/zk/TestEphemeralStore.java | 5 +-
.../drill/exec/record/vector/TestValueVector.java | 7 +-
.../java/org/apache/drill/test/BaseTestQuery.java | 19 ++--
.../java/org/apache/drill/test/ConfigBuilder.java | 20 ++--
.../org/apache/drill/test/QueryRowSetIterator.java | 5 +-
.../java-exec/src/test/resources/drill-module.conf | 108 ++++++++++-----------
.../apache/drill/exec/memory/BoundsChecking.java | 29 ++++--
.../main/codegen/templates/FixedValueVectors.java | 11 +--
.../org/apache/drill/exec/vector/BitVector.java | 63 ++++++------
.../org/apache/drill/exec/vector/ValueVector.java | 10 +-
.../apache/drill/exec/vector/VectorTrimmer.java | 6 --
.../exec/vector/complex/RepeatedValueVector.java | 29 +++---
18 files changed, 177 insertions(+), 170 deletions(-)
diff --git
a/common/src/main/java/org/apache/drill/common/exceptions/DrillException.java
b/common/src/main/java/org/apache/drill/common/exceptions/DrillException.java
index 5acee3e..c3d1071 100644
---
a/common/src/main/java/org/apache/drill/common/exceptions/DrillException.java
+++
b/common/src/main/java/org/apache/drill/common/exceptions/DrillException.java
@@ -17,10 +17,10 @@
*/
package org.apache.drill.common.exceptions;
+@SuppressWarnings("serial")
public class DrillException extends Exception {
- public DrillException() {
- super();
- }
+
+ public DrillException() { }
public DrillException(String message, Throwable cause, boolean
enableSuppression, boolean writableStackTrace) {
super(message, cause, enableSuppression, writableStackTrace);
@@ -37,6 +37,4 @@ public class DrillException extends Exception {
public DrillException(Throwable cause) {
super(cause);
}
-
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillException.class);
}
diff --git
a/common/src/main/java/org/apache/drill/common/exceptions/UserException.java
b/common/src/main/java/org/apache/drill/common/exceptions/UserException.java
index eccdb8e..1ba8671 100644
--- a/common/src/main/java/org/apache/drill/common/exceptions/UserException.java
+++ b/common/src/main/java/org/apache/drill/common/exceptions/UserException.java
@@ -585,6 +585,7 @@ public class UserException extends DrillRuntimeException {
return this;
}
+ private static final File SPIN_FILE = new File("/tmp/drill/spin");
/**
* builds a user exception or returns the wrapped one. If the error is a
system error, the error message is logged
* to the given {@link Logger}.
@@ -596,14 +597,13 @@ public class UserException extends DrillRuntimeException {
// To allow for debugging:
//
- // A spinner code to make the execution stop here while the file
'/tmp/drill/spin' exists
+ // A spinner code to make the execution stop here while the file
'/tmp/drill/spin' exists
// Can be used to attach a debugger, use jstack, etc
// (do "clush -a touch /tmp/drill/spin" to turn this on across all the
cluster nodes, and to
- // release the spinning threads do "clush -a rm /tmp/drill/spin")
+ // release the spinning threads do "clush -a rm /tmp/drill/spin")
// The processID of the spinning thread (along with the error message)
should then be found
// in a file like /tmp/drill/spin4148663301172491613.tmp
- final File spinFile = new File("/tmp/drill/spin");
- if ( spinFile.exists() ) {
+ if (SPIN_FILE.exists()) {
final File tmpDir = new File("/tmp/drill");
File outErr = null;
try {
@@ -617,7 +617,7 @@ public class UserException extends DrillRuntimeException {
} catch (final Exception ex) {
logger.warn("Failed creating a spinner tmp message file: {}", ex);
}
- while (spinFile.exists()) {
+ while (SPIN_FILE.exists()) {
try { sleep(1_000); } catch (final Exception ex) { /* ignore
interruptions */ }
}
try { outErr.delete(); } catch (final Exception ex) { } // cleanup -
remove err msg file
diff --git a/common/src/test/resources/logback-test.xml
b/common/src/test/resources/logback-test.xml
index 1cd04dc..06004a6 100644
--- a/common/src/test/resources/logback-test.xml
+++ b/common/src/test/resources/logback-test.xml
@@ -56,7 +56,7 @@
<appender-ref ref="SOCKET"/>
</then>
</if>
- <appender-ref ref="STDOUT"/>
+ <!-- <appender-ref ref="STDOUT"/> -->
</root>
</configuration>
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
index 804254b..962d74e 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
@@ -180,7 +180,6 @@ public class Foreman implements Runnable {
return queryText;
}
-
/**
* Get the QueryContext created for the query.
*
@@ -874,7 +873,6 @@ public class Foreman implements Runnable {
}
}
-
public RuntimeFilterRouter getRuntimeFilterRouter() {
return runtimeFilterRouter;
}
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/ForemanException.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/ForemanException.java
index d5cf28c..6bf8baa 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/ForemanException.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/ForemanException.java
@@ -21,10 +21,8 @@ import
org.apache.drill.common.exceptions.ExecutionSetupException;
public class ForemanException extends ExecutionSetupException {
private static final long serialVersionUID = -6943409010231014085L;
-// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(ForemanException.class);
- public ForemanException() {
- }
+ public ForemanException() { }
public ForemanException(final String message, final Throwable cause, final
boolean enableSuppression,
final boolean writableStackTrace) {
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStateProcessor.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStateProcessor.java
index 0ef4f37..5c6836f 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStateProcessor.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStateProcessor.java
@@ -17,7 +17,6 @@
*/
package org.apache.drill.exec.work.foreman;
-import com.codahale.metrics.Counter;
import org.apache.drill.common.EventProcessor;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.metrics.DrillMetrics;
@@ -25,6 +24,8 @@ import
org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
import org.apache.drill.exec.server.DrillbitContext;
import org.apache.drill.exec.work.foreman.Foreman.ForemanResult;
+import com.codahale.metrics.Counter;
+
/**
* Is responsible for query transition from one state to another,
* incrementing / decrementing query statuses counters.
@@ -217,6 +218,8 @@ public class QueryStateProcessor implements AutoCloseable {
case CANCELLATION_REQUESTED:
wrapUpCancellation();
return;
+ default:
+ break;
}
checkCommonStates(newState, exception);
}
@@ -231,6 +234,8 @@ public class QueryStateProcessor implements AutoCloseable {
case CANCELLATION_REQUESTED:
wrapUpCancellation();
return;
+ default:
+ break;
}
checkCommonStates(newState, exception);
}
@@ -324,6 +329,8 @@ public class QueryStateProcessor implements AutoCloseable {
foremanResult.setFailed(exception);
foremanResult.close();
return;
+ default:
+ break;
}
throw new IllegalStateException(String.format("Failure trying to change
states: %s --> %s", state.name(), newState.name()));
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/exec/coord/zk/TestEphemeralStore.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/coord/zk/TestEphemeralStore.java
index 03baf3f..2cd79ca 100644
---
a/exec/java-exec/src/test/java/org/apache/drill/exec/coord/zk/TestEphemeralStore.java
+++
b/exec/java-exec/src/test/java/org/apache/drill/exec/coord/zk/TestEphemeralStore.java
@@ -59,7 +59,7 @@ public class TestEphemeralStore {
}
}
-
+ @SuppressWarnings("unchecked")
@Before
public void setUp() throws Exception {
ZookeeperTestUtil.setZookeeperSaslTestConfigProps();
@@ -106,6 +106,7 @@ public class TestEphemeralStore {
*/
@Test
public void testStoreRegistersDispatcherAndStartsItsClient() throws
Exception {
+ @SuppressWarnings("resource")
final StoreWithMockClient<String> store = new
StoreWithMockClient<>(config, curator);
final PathChildrenCache cache = Mockito.mock(PathChildrenCache.class);
@@ -114,6 +115,7 @@ public class TestEphemeralStore {
.when(client.getCache())
.thenReturn(cache);
+ @SuppressWarnings("unchecked")
final ListenerContainer<PathChildrenCacheListener> container =
Mockito.mock(ListenerContainer.class);
Mockito
.when(cache.getListenable())
@@ -143,5 +145,4 @@ public class TestEphemeralStore {
final String actual = store.get(path);
Assert.assertEquals("value mismatch", value, actual);
}
-
}
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestValueVector.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestValueVector.java
index fc1d5c7..3c890c2 100644
---
a/exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestValueVector.java
+++
b/exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestValueVector.java
@@ -63,14 +63,13 @@ import org.apache.drill.exec.vector.complex.ListVector;
import org.apache.drill.exec.vector.complex.MapVector;
import org.apache.drill.exec.vector.complex.RepeatedListVector;
import org.apache.drill.exec.vector.complex.RepeatedMapVector;
+import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableMap;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
-import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
-import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableMap;
-
import io.netty.buffer.DrillBuf;
@Category(VectorTest.class)
@@ -142,7 +141,7 @@ public class TestValueVector extends ExecTest {
// common: value count < MAX_VALUE_ALLOCATION
try {
vector.allocateNew(expectedValueCapacity);
- for (int i=0; i<3;i++) {
+ for (int i = 0; i < 3; i++) {
vector.reAlloc(); // expand buffer size
}
assertEquals(Integer.MAX_VALUE, vector.getValueCapacity());
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/test/BaseTestQuery.java
b/exec/java-exec/src/test/java/org/apache/drill/test/BaseTestQuery.java
index 6ca50f5..dad5c46 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/test/BaseTestQuery.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/test/BaseTestQuery.java
@@ -34,8 +34,6 @@ import java.util.List;
import java.util.Properties;
import java.util.concurrent.atomic.AtomicInteger;
-import org.apache.drill.exec.store.SchemaFactory;
-import org.apache.drill.test.DrillTestWrapper.TestServices;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.config.DrillProperties;
import org.apache.drill.common.exceptions.UserException;
@@ -53,6 +51,7 @@ import
org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
import org.apache.drill.exec.proto.UserBitShared.QueryType;
import org.apache.drill.exec.proto.UserProtos.PreparedStatementHandle;
import org.apache.drill.exec.record.RecordBatchLoader;
+import org.apache.drill.exec.record.VectorWrapper;
import org.apache.drill.exec.rpc.ConnectionThrottle;
import org.apache.drill.exec.rpc.user.AwaitableUserResultsListener;
import org.apache.drill.exec.rpc.user.QueryDataBatch;
@@ -60,22 +59,24 @@ import org.apache.drill.exec.rpc.user.UserResultsListener;
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.SchemaFactory;
import org.apache.drill.exec.store.StoragePluginRegistry;
import org.apache.drill.exec.util.StoragePluginTestUtils;
import org.apache.drill.exec.util.VectorUtil;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
+import org.apache.drill.exec.vector.ValueVector;
import org.apache.drill.shaded.guava.com.google.common.base.Charsets;
import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
import org.apache.drill.shaded.guava.com.google.common.io.Resources;
-
-import org.apache.drill.exec.record.VectorWrapper;
-import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.test.DrillTestWrapper.TestServices;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
/**
- * @deprecated Use {@link ClusterTest} instead.
+ * deprecated Use {@link ClusterTest} instead.
+ *
+ * But, not marked as deprecated because it is still widely used.
*/
-@Deprecated
+//@Deprecated
public class BaseTestQuery extends ExecTest {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(BaseTestQuery.class);
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/test/ConfigBuilder.java
b/exec/java-exec/src/test/java/org/apache/drill/test/ConfigBuilder.java
index 5b38182..2d4dd78 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/test/ConfigBuilder.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/test/ConfigBuilder.java
@@ -17,20 +17,20 @@
*/
package org.apache.drill.test;
-import java.util.Collection;
-import java.util.Map.Entry;
-import java.util.Properties;
-
+import com.typesafe.config.Config;
+import com.typesafe.config.ConfigValue;
+import com.typesafe.config.ConfigValueFactory;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.map.CaseInsensitiveMap;
import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.memory.BoundsChecking;
import org.apache.drill.exec.physical.impl.BaseRootExec;
import org.apache.drill.exec.server.options.OptionDefinition;
import org.apache.drill.exec.server.options.SystemOptionManager;
-import com.typesafe.config.Config;
-import com.typesafe.config.ConfigValue;
-import com.typesafe.config.ConfigValueFactory;
+import java.util.Collection;
+import java.util.Map.Entry;
+import java.util.Properties;
/**
* Builds a {@link DrillConfig} for use in tests. Use this when a config
@@ -46,7 +46,7 @@ public class ConfigBuilder {
* Use the given configuration properties as overrides.
* @param configProps a collection of config properties
* @return this builder
- * @see {@link #put(String, Object)}
+ * @see #put(String, Object)
*/
public ConfigBuilder configProps(Properties configProps) {
if (hasResource()) {
@@ -87,7 +87,7 @@ public class ConfigBuilder {
* @param configResource path to the file that contains the
* config file to be read
* @return this builder
- * @see {@link #put(String, Object)}
+ * @see #put(String, Object)
*/
public ConfigBuilder resource(String configResource) {
@@ -134,7 +134,7 @@ public class ConfigBuilder {
properties.put(ExecConstants.USE_DYNAMIC_UDFS_KEY, "false");
properties.put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE,
"false");
properties.put(BaseRootExec.ENABLE_BATCH_DUMP_CONFIG, "false");
-
+ properties.put(BoundsChecking.ENABLE_UNSAFE_BOUNDS_CHECK_PROPERTY, "true");
return properties;
}
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
b/exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
index 41b65b3..03ae625 100644
---
a/exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
+++
b/exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
@@ -21,6 +21,7 @@ import java.util.Iterator;
import org.apache.drill.exec.exception.SchemaChangeException;
import org.apache.drill.exec.memory.BufferAllocator;
+import org.apache.drill.exec.physical.rowSet.DirectRowSet;
import org.apache.drill.exec.physical.rowSet.RowSetFormatter;
import org.apache.drill.exec.proto.UserBitShared.QueryId;
import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
@@ -29,7 +30,6 @@ import org.apache.drill.exec.record.RecordBatchLoader;
import org.apache.drill.exec.record.VectorContainer;
import org.apache.drill.exec.rpc.user.QueryDataBatch;
import org.apache.drill.test.BufferingQueryEventListener.QueryEvent;
-import org.apache.drill.exec.physical.rowSet.DirectRowSet;
public class QueryRowSetIterator implements Iterator<DirectRowSet>,
Iterable<DirectRowSet> {
private final BufferingQueryEventListener listener;
@@ -57,8 +57,7 @@ public class QueryRowSetIterator implements
Iterator<DirectRowSet>, Iterable<Dir
QueryEvent event = listener.get();
state = event.state;
batch = null;
- switch (event.type)
- {
+ switch (event.type) {
case BATCH:
batchCount++;
recordCount += event.batch.getHeader().getRowCount();
diff --git a/exec/java-exec/src/test/resources/drill-module.conf
b/exec/java-exec/src/test/resources/drill-module.conf
index f7e7b8b..3ddc2ef 100644
--- a/exec/java-exec/src/test/resources/drill-module.conf
+++ b/exec/java-exec/src/test/resources/drill-module.conf
@@ -12,63 +12,63 @@ drill: {
}
test.query.printing.silent : false,
exec: {
- cluster-id: "drillbits1"
- rpc: {
- user: {
- server: {
- port: 31010
- threads: 1
- }
- client: {
- threads: 1
- }
+ cluster-id: "drillbits1"
+ rpc: {
+ user: {
+ server: {
+ port: 31010
+ threads: 1
+ }
+ client: {
+ threads: 1
+ }
+ },
+ bit: {
+ server: {
+ port : 31011,
+ retry:{
+ count: 7200,
+ delay: 500
+ },
+ threads: 1
+ }
+ },
+ use.ip : false
+ },
+ sys.store.provider.local.path: "file:/tmp/drill/tests",
+ udf.directory.fs: "local",
+ operator: {
+ packages += "org.apache.drill.exec.physical.config"
+ },
+ optimizer: {
+ implementation: "org.apache.drill.exec.opt.IdentityOptimizer"
},
- bit: {
- server: {
- port : 31011,
- retry:{
- count: 7200,
- delay: 500
- },
- threads: 1
+ functions: ["org.apache.drill.expr.fn.impl"],
+ storage: {
+ packages += "org.apache.drill.exec.store"
+ },
+ metrics : {
+ context: "drillbit",
+ log.enabled: true
+ },
+ zk: {
+ connect: "localhost:2181",
+ root: "drill/happy",
+ refresh: 5,
+ timeout: 5000,
+ retry: {
+ count: 7200,
+ delay: 500
}
},
- use.ip : false
- },
- sys.store.provider.local.path: "file:/tmp/drill/tests",
- udf.directory.fs: "local",
- operator: {
- packages += "org.apache.drill.exec.physical.config"
- },
- optimizer: {
- implementation: "org.apache.drill.exec.opt.IdentityOptimizer"
- },
- functions: ["org.apache.drill.expr.fn.impl"],
- storage: {
- packages += "org.apache.drill.exec.store"
- },
- metrics : {
- context: "drillbit",
- log.enabled: true
- },
- zk: {
- connect: "localhost:2181",
- root: "drill/happy",
- refresh: 5,
- timeout: 5000,
- retry: {
- count: 7200,
- delay: 500
+ functions: ["org.apache.drill.expr.fn.impl"],
+ network: {
+ start: 35000
+ },
+ work: {
+ max.width.per.endpoint: 2,
+ global.max.width: 100,
+ executor.threads: 4
}
- },
- functions: ["org.apache.drill.expr.fn.impl"],
- network: {
- start: 35000
- },
- work: {
- max.width.per.endpoint: 2,
- global.max.width: 100,
- executor.threads: 4
- }
}
}
diff --git
a/exec/memory/base/src/main/java/org/apache/drill/exec/memory/BoundsChecking.java
b/exec/memory/base/src/main/java/org/apache/drill/exec/memory/BoundsChecking.java
index bd8701d..098b20a 100644
---
a/exec/memory/base/src/main/java/org/apache/drill/exec/memory/BoundsChecking.java
+++
b/exec/memory/base/src/main/java/org/apache/drill/exec/memory/BoundsChecking.java
@@ -20,7 +20,10 @@ package org.apache.drill.exec.memory;
import java.lang.reflect.Field;
import java.util.Formatter;
+import org.apache.drill.exec.util.AssertionUtil;
import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import io.netty.buffer.AbstractByteBuf;
import io.netty.buffer.DrillBuf;
@@ -29,16 +32,27 @@ import io.netty.util.IllegalReferenceCountException;
import static org.apache.drill.exec.util.SystemPropertyUtil.getBoolean;
public class BoundsChecking {
- private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(BoundsChecking.class);
+ private static final Logger logger =
LoggerFactory.getLogger(BoundsChecking.class);
public static final String ENABLE_UNSAFE_BOUNDS_CHECK_PROPERTY =
"drill.exec.memory.enable_unsafe_bounds_check";
- // for backward compatibility check "drill.enable_unsafe_memory_access"
property and enable bounds checking when
- // unsafe memory access is explicitly disabled
public static final String ENABLE_UNSAFE_MEMORY_ACCESS_PROPERTY =
"drill.enable_unsafe_memory_access";
- public static final boolean BOUNDS_CHECKING_ENABLED =
- getBoolean(ENABLE_UNSAFE_BOUNDS_CHECK_PROPERTY,
!getBoolean(ENABLE_UNSAFE_MEMORY_ACCESS_PROPERTY, true));
+ public static final boolean BOUNDS_CHECKING_ENABLED = boundsCheckEnabled();
private static final boolean checkAccessible =
getStaticBooleanField(AbstractByteBuf.class, "checkAccessible", false);
+ /**
+ * Bounds checking is on either if it is explicitly turned on via a
+ * supported option, or if we're running with assertions enabled, typically
+ * in an IDE.
+ */
+ private static boolean boundsCheckEnabled() {
+ // for backward compatibility check "drill.enable_unsafe_memory_access"
property and enable bounds checking when
+ // unsafe memory access is explicitly disabled.
+ // Enable bounds checking if assertions are enabled (as they are in IDE
runs.)
+ return AssertionUtil.isAssertionsEnabled() ||
+ getBoolean(ENABLE_UNSAFE_BOUNDS_CHECK_PROPERTY,
+ !getBoolean(ENABLE_UNSAFE_MEMORY_ACCESS_PROPERTY, true));
+ }
+
static {
if (BOUNDS_CHECKING_ENABLED) {
logger.warn("Drill is running with direct memory bounds checking
enabled. If this is a production system, disable it.");
@@ -47,10 +61,9 @@ public class BoundsChecking {
}
}
- private BoundsChecking() {
- }
+ private BoundsChecking() { }
- private static boolean getStaticBooleanField(Class cls, String name, boolean
def) {
+ private static boolean getStaticBooleanField(Class<?> cls, String name,
boolean def) {
try {
Field field = cls.getDeclaredField(name);
field.setAccessible(true);
diff --git a/exec/vector/src/main/codegen/templates/FixedValueVectors.java
b/exec/vector/src/main/codegen/templates/FixedValueVectors.java
index 5741555..6d408b8 100644
--- a/exec/vector/src/main/codegen/templates/FixedValueVectors.java
+++ b/exec/vector/src/main/codegen/templates/FixedValueVectors.java
@@ -816,7 +816,7 @@ public final class ${minor.class}Vector extends
BaseDataValueVector implements F
@Override
public void setValueCount(int valueCount) {
final int currentValueCapacity = getValueCapacity();
- final int idx = (VALUE_WIDTH * valueCount);
+ final int idx = VALUE_WIDTH * valueCount;
while(valueCount > getValueCapacity()) {
reAlloc();
}
@@ -825,12 +825,11 @@ public final class ${minor.class}Vector extends
BaseDataValueVector implements F
} else if (allocationMonitor > 0) {
allocationMonitor = 0;
}
- VectorTrimmer.trim(data, idx);
- data.writerIndex(valueCount * VALUE_WIDTH);
+ data.writerIndex(idx);
}
}
-
<#if minor.class == "Int" || minor.class == "UInt4" || minor.class ==
"UInt1">
+
/**
* Helper class to buffer container mutation as a means to optimize native
memory copy operations.
*
@@ -867,8 +866,8 @@ public final class ${minor.class}Vector extends
BaseDataValueVector implements F
this.currentIdx = startIdx;
this.parent = parent;
}
-
<#if minor.class == "Int" || minor.class == "UInt4">
+
public void setSafe(int value) {
if (buffer.remaining() < 4) {
flush();
@@ -909,8 +908,8 @@ public final class ${minor.class}Vector extends
BaseDataValueVector implements F
DrillBuf.putInt(buffer, pos, val);
}
</#if> <#-- minor.class -->
-
<#if minor.class == "UInt1">
+
public void setSafe(byte value) {
if (buffer.remaining() < 1) {
flush();
diff --git
a/exec/vector/src/main/java/org/apache/drill/exec/vector/BitVector.java
b/exec/vector/src/main/java/org/apache/drill/exec/vector/BitVector.java
index 7b2cd28..24ecc13 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/vector/BitVector.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/BitVector.java
@@ -17,19 +17,21 @@
*/
package org.apache.drill.exec.vector;
-import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
-import io.netty.buffer.DrillBuf;
-
+import org.apache.drill.exec.exception.OutOfMemoryException;
import org.apache.drill.exec.exception.OversizedAllocationException;
import org.apache.drill.exec.expr.holders.BitHolder;
import org.apache.drill.exec.expr.holders.NullableBitHolder;
import org.apache.drill.exec.memory.BufferAllocator;
-import org.apache.drill.exec.exception.OutOfMemoryException;
import org.apache.drill.exec.proto.UserBitShared.SerializedField;
import org.apache.drill.exec.record.MaterializedField;
import org.apache.drill.exec.record.TransferPair;
import org.apache.drill.exec.vector.complex.impl.BitReaderImpl;
import org.apache.drill.exec.vector.complex.reader.FieldReader;
+import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import io.netty.buffer.DrillBuf;
/**
* Bit implements a vector of bit-width values. Elements in the vector are
accessed by position from the logical start
@@ -37,7 +39,7 @@ import
org.apache.drill.exec.vector.complex.reader.FieldReader;
* or '1'.
*/
public final class BitVector extends BaseDataValueVector implements
FixedWidthVector {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(BitVector.class);
+ static final Logger logger = LoggerFactory.getLogger(BitVector.class);
/**
* Width of each fixed-width value.
@@ -57,7 +59,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
/**
* Maximum number of values that this fixed-width vector can hold
* and stay below the maximum vector size limit and/or stay below
- * the maximum item count. This lis the limit enforced when the
+ * the maximum item count. This is the limit enforced when the
* vector is used to hold required or nullable values.
*/
@@ -77,7 +79,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
private int valueCount;
private int allocationSizeInBytes = INITIAL_VALUE_ALLOCATION;
- private int allocationMonitor = 0;
+ private int allocationMonitor;
public BitVector(MaterializedField field, BufferAllocator allocator) {
super(field, allocator);
@@ -98,17 +100,17 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
return getSizeFromCount(valueCount);
}
- private int getSizeFromCount(int valueCount) {
- return (int) Math.ceil(valueCount / 8.0);
+ public static int getSizeFromCount(int valueCount) {
+ return (valueCount + 7) / 8;
}
@Override
public int getValueCapacity() {
- return (int) Math.min((long)Integer.MAX_VALUE, data.capacity() * 8L);
+ return (int) Math.min(Integer.MAX_VALUE, data.capacity() * 8L);
}
private int getByteIndex(int index) {
- return (int) Math.floor(index / 8.0);
+ return index / 8;
}
@Override
@@ -181,7 +183,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
public void reAlloc() {
long newAllocationSize = allocationSizeInBytes * 2L;
- // Some operations, such as Value Vector#exchange, can be change DrillBuf
data field without corresponding allocation size changes.
+ // Some operations, such as Value Vector#exchange, can change DrillBuf
data field without corresponding allocation size changes.
// Check that the size of the allocation is sufficient to copy the old
buffer.
while (newAllocationSize < data.capacity()) {
newAllocationSize *= 2L;
@@ -203,6 +205,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
// This version uses the base version because this vector appears to not be
// used, so not worth the effort to avoid zero-fill.
+ @Override
public DrillBuf reallocRaw(int newAllocationSize) {
while (allocationSizeInBytes < newAllocationSize) {
reAlloc();
@@ -241,8 +244,8 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
@Override
public void load(SerializedField metadata, DrillBuf buffer) {
-
Preconditions.checkArgument(this.field.getName().equals(metadata.getNamePart().getName()),
- "The field %s doesn't match the provided
metadata %s.", this.field, metadata);
+
Preconditions.checkArgument(field.getName().equals(metadata.getNamePart().getName()),
+ "The field %s doesn't match the provided
metadata %s.", field, metadata);
final int valueCount = metadata.getValueCount();
final int expectedLength = getSizeFromCount(valueCount);
final int actualLength = metadata.getBufferLength();
@@ -315,7 +318,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
// TODO maybe do this one word at a time, rather than byte?
byte byteI, byteIPlus1 = 0;
- for(int i = 0; i < numBytesHoldingSourceBits - 1; i++) {
+ for (int i = 0; i < numBytesHoldingSourceBits - 1; i++) {
byteI = this.data.getByte(firstByteIndex + i);
byteIPlus1 = this.data.getByte(firstByteIndex + i + 1);
// Extract higher-X bits from first byte i and lower-Y bits from byte
(i + 1), where X + Y = 8 bits
@@ -355,6 +358,14 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
target.getMutator().setValueCount(length);
}
+ @Override
+ public void exchange(ValueVector other) {
+ super.exchange(other);
+ int temp = valueCount;
+ valueCount = ((BitVector) other).valueCount;
+ ((BitVector) other).valueCount = temp;
+ }
+
private class TransferImpl implements TransferPair {
BitVector to;
@@ -387,13 +398,6 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
}
}
- private void decrementAllocationMonitor() {
- if (allocationMonitor > 0) {
- allocationMonitor = 0;
- }
- --allocationMonitor;
- }
-
private void incrementAllocationMonitor() {
++allocationMonitor;
}
@@ -447,8 +451,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
*/
public class Mutator extends BaseMutator {
- private Mutator() {
- }
+ private Mutator() { }
/**
* Set the bit at the given index to the specified value.
@@ -481,21 +484,21 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
}
public void setSafe(int index, int value) {
- while(index >= getValueCapacity()) {
+ while (index >= getValueCapacity()) {
reAlloc();
}
set(index, value);
}
public void setSafe(int index, BitHolder holder) {
- while(index >= getValueCapacity()) {
+ while (index >= getValueCapacity()) {
reAlloc();
}
set(index, holder.value);
}
public void setSafe(int index, NullableBitHolder holder) {
- while(index >= getValueCapacity()) {
+ while (index >= getValueCapacity()) {
reAlloc();
}
set(index, holder.value);
@@ -506,7 +509,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
int currentValueCapacity = getValueCapacity();
BitVector.this.valueCount = valueCount;
int idx = getSizeFromCount(valueCount);
- while(valueCount > getValueCapacity()) {
+ while (valueCount > getValueCapacity()) {
reAlloc();
}
if (valueCount > 0 && currentValueCapacity > valueCount * 2) {
@@ -520,7 +523,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
@Override
public final void generateTestData(int values) {
boolean even = true;
- for(int i = 0; i < values; i++, even = !even) {
+ for (int i = 0; i < values; i++, even = !even) {
if (even) {
set(i, 1);
}
@@ -531,7 +534,7 @@ public final class BitVector extends BaseDataValueVector
implements FixedWidthVe
@Override
public void clear() {
- this.valueCount = 0;
+ valueCount = 0;
super.clear();
}
diff --git
a/exec/vector/src/main/java/org/apache/drill/exec/vector/ValueVector.java
b/exec/vector/src/main/java/org/apache/drill/exec/vector/ValueVector.java
index 2659810..6fda66d 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/vector/ValueVector.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/ValueVector.java
@@ -20,17 +20,17 @@ package org.apache.drill.exec.vector;
import java.io.Closeable;
import java.util.Set;
-import io.netty.buffer.DrillBuf;
-
import org.apache.drill.exec.exception.OutOfMemoryException;
-import org.apache.drill.exec.memory.AllocationManager.BufferLedger;
import org.apache.drill.exec.memory.AllocationManager;
+import org.apache.drill.exec.memory.AllocationManager.BufferLedger;
import org.apache.drill.exec.memory.BufferAllocator;
import org.apache.drill.exec.proto.UserBitShared.SerializedField;
import org.apache.drill.exec.record.MaterializedField;
import org.apache.drill.exec.record.TransferPair;
import org.apache.drill.exec.vector.complex.reader.FieldReader;
+import io.netty.buffer.DrillBuf;
+
/**
* An abstraction that is used to store a sequence of values in an individual
* column.
@@ -266,7 +266,7 @@ public interface ValueVector extends Closeable,
Iterable<ValueVector> {
void toNullable(ValueVector nullableVector);
/**
- * An abstraction that is used to read from this vector instance.
+ * Reads from this vector instance.
*/
interface Accessor {
/**
@@ -289,7 +289,7 @@ public interface ValueVector extends Closeable,
Iterable<ValueVector> {
}
/**
- * An abstraction that is used to write into this vector instance.
+ * Writes into this vector instance.
*/
interface Mutator {
/**
diff --git
a/exec/vector/src/main/java/org/apache/drill/exec/vector/VectorTrimmer.java
b/exec/vector/src/main/java/org/apache/drill/exec/vector/VectorTrimmer.java
index 0fd50cb..da85535 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/vector/VectorTrimmer.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/VectorTrimmer.java
@@ -18,16 +18,10 @@
package org.apache.drill.exec.vector;
import io.netty.buffer.ByteBuf;
-import io.netty.buffer.DrillBuf;
public class VectorTrimmer {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(VectorTrimmer.class);
public static void trim(ByteBuf data, int idx) {
data.writerIndex(idx);
- if (data instanceof DrillBuf) {
- // data.capacity(idx);
- data.writerIndex(idx);
- }
}
}
diff --git
a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/RepeatedValueVector.java
b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/RepeatedValueVector.java
index 0552f77..4b3fecc 100644
---
a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/RepeatedValueVector.java
+++
b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/RepeatedValueVector.java
@@ -21,25 +21,27 @@ import org.apache.drill.exec.vector.UInt4Vector;
import org.apache.drill.exec.vector.ValueVector;
/**
- * An abstraction representing repeated value vectors.
- *
- * A repeated vector contains values that may either be flat or nested. A
value consists of zero or more cells(inner values).
- * Current design maintains data and offsets vectors. Each cell is stored in
the data vector. Repeated vector
- * uses the offset vector to determine the sequence of cells pertaining to an
individual value.
- *
+ * Represents repeated (AKA "array") value vectors.
+ * <p>
+ * A repeated vector contains values that may either be flat or nested. A value
+ * consists of zero or more cells(inner values). Current design maintains data
+ * and offsets vectors. Each cell is stored in the data vector. Repeated vector
+ * uses the offset vector to determine the sequence of cells pertaining to an
+ * individual value.
*/
+
public interface RepeatedValueVector extends ValueVector, ContainerVectorLike {
int DEFAULT_REPEAT_PER_RECORD = 5;
/**
- * Returns the underlying offset vector or null if none exists.
+ * @return the underlying offset vector or null if none exists.
*/
UInt4Vector getOffsetVector();
/**
- * Returns the underlying data vector or null if none exists.
+ * @return the underlying data vector or null if none exists.
*/
ValueVector getDataVector();
@@ -51,22 +53,19 @@ public interface RepeatedValueVector extends ValueVector,
ContainerVectorLike {
interface RepeatedAccessor extends ValueVector.Accessor {
/**
- * Returns total number of cells that vector contains.
- *
+ * @return total number of cells that vector contains.
* The result includes empty, null valued cells.
*/
int getInnerValueCount();
-
/**
- * Returns number of cells that the value at the given index contains.
+ * @return number of cells that the value at the given index contains.
*/
int getInnerValueCountAt(int index);
/**
- * Returns true if the value at the given index is empty, false otherwise.
- *
* @param index value index
+ * @return true if the value at the given index is empty, false otherwise.
*/
boolean isEmpty(int index);
}
@@ -78,7 +77,5 @@ public interface RepeatedValueVector extends ValueVector,
ContainerVectorLike {
* @param index index of new value to start
*/
void startNewValue(int index);
-
-
}
}
