You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by go...@apache.org on 2015/11/25 10:03:28 UTC
hive git commit: HIVE-12463: VectorMapJoinFastKeyStore has Array OOB
errors (Gopal V, reviewed by Sergey Shelukhin)
Repository: hive
Updated Branches:
refs/heads/master 63251225c -> f18037995
HIVE-12463: VectorMapJoinFastKeyStore has Array OOB errors (Gopal V, reviewed by Sergey Shelukhin)
Signed-off-by: Gopal V <go...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/f1803799
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/f1803799
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/f1803799
Branch: refs/heads/master
Commit: f180379955bce04e81c4f799f16d184d74f7ef4c
Parents: 6325122
Author: Gopal V <go...@apache.org>
Authored: Wed Nov 25 01:01:15 2015 -0800
Committer: Gopal V <go...@apache.org>
Committed: Wed Nov 25 01:01:15 2015 -0800
----------------------------------------------------------------------
.../mapjoin/fast/VectorMapJoinFastKeyStore.java | 17 ++---
.../apache/hadoop/hive/serde2/WriteBuffers.java | 69 ++++++++++----------
2 files changed, 38 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/f1803799/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/mapjoin/fast/VectorMapJoinFastKeyStore.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/mapjoin/fast/VectorMapJoinFastKeyStore.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/mapjoin/fast/VectorMapJoinFastKeyStore.java
index 58af4eb..efdcd43 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/mapjoin/fast/VectorMapJoinFastKeyStore.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/mapjoin/fast/VectorMapJoinFastKeyStore.java
@@ -30,7 +30,6 @@ public class VectorMapJoinFastKeyStore {
private WriteBuffers writeBuffers;
- private WriteBuffers.ByteSegmentRef byteSegmentRef;
private WriteBuffers.Position readPos;
/**
@@ -141,17 +140,11 @@ public class VectorMapJoinFastKeyStore {
}
// Our reading is positioned to the key.
- writeBuffers.getByteSegmentRefToCurrent(byteSegmentRef, keyLength, readPos);
-
- byte[] currentBytes = byteSegmentRef.getBytes();
- int currentStart = (int) byteSegmentRef.getOffset();
-
- for (int i = 0; i < keyLength; i++) {
- if (currentBytes[currentStart + i] != keyBytes[keyStart + i]) {
- // LOG.debug("VectorMapJoinFastKeyStore equalKey no match on bytes");
- return false;
- }
+ if (!writeBuffers.isEqual(keyBytes, keyStart, readPos, keyLength)) {
+ // LOG.debug("VectorMapJoinFastKeyStore equalKey no match on bytes");
+ return false;
}
+
// LOG.debug("VectorMapJoinFastKeyStore equalKey match on bytes");
return true;
}
@@ -159,7 +152,6 @@ public class VectorMapJoinFastKeyStore {
public VectorMapJoinFastKeyStore(int writeBuffersSize) {
writeBuffers = new WriteBuffers(writeBuffersSize, AbsoluteKeyOffset.maxSize);
- byteSegmentRef = new WriteBuffers.ByteSegmentRef();
readPos = new WriteBuffers.Position();
}
@@ -167,7 +159,6 @@ public class VectorMapJoinFastKeyStore {
// TODO: Check if maximum size compatible with AbsoluteKeyOffset.maxSize.
this.writeBuffers = writeBuffers;
- byteSegmentRef = new WriteBuffers.ByteSegmentRef();
readPos = new WriteBuffers.Position();
}
}
http://git-wip-us.apache.org/repos/asf/hive/blob/f1803799/serde/src/java/org/apache/hadoop/hive/serde2/WriteBuffers.java
----------------------------------------------------------------------
diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/WriteBuffers.java b/serde/src/java/org/apache/hadoop/hive/serde2/WriteBuffers.java
index b47456e..5900428 100644
--- a/serde/src/java/org/apache/hadoop/hive/serde2/WriteBuffers.java
+++ b/serde/src/java/org/apache/hadoop/hive/serde2/WriteBuffers.java
@@ -282,32 +282,33 @@ public final class WriteBuffers implements RandomAccessOutput {
return true;
}
- /**
- * Compares part of the buffer with a part of an external byte array.
- * Does not modify readPoint.
- */
- public boolean isEqual(byte[] left, int leftLength, long rightOffset, int rightLength) {
- if (rightLength != leftLength) {
- return false;
+ private final boolean isEqual(byte[] left, int leftOffset, int rightIndex, int rightFrom, int length) {
+ if (length == 0) {
+ return true;
}
- int rightIndex = getBufferIndex(rightOffset), rightFrom = getOffset(rightOffset);
+ // invariant: rightLength = leftLength
+ // rightOffset is within the buffers
byte[] rightBuffer = writeBuffers.get(rightIndex);
- if (rightFrom + rightLength <= wbSize) {
+ if (rightFrom + length <= wbSize) {
// TODO: allow using unsafe optionally.
- for (int i = 0; i < leftLength; ++i) {
- if (left[i] != rightBuffer[rightFrom + i]) {
+ // bounds check first, to trigger bugs whether the first byte matches or not
+ if (left[leftOffset + length - 1] != rightBuffer[rightFrom + length - 1]) {
+ return false;
+ }
+ for (int i = 0; i < length; ++i) {
+ if (left[leftOffset + i] != rightBuffer[rightFrom + i]) {
return false;
}
}
return true;
}
- for (int i = 0; i < rightLength; ++i) {
+ for (int i = 0; i < length; ++i) {
if (rightFrom == wbSize) {
++rightIndex;
rightBuffer = writeBuffers.get(rightIndex);
rightFrom = 0;
}
- if (left[i] != rightBuffer[rightFrom++]) {
+ if (left[leftOffset + i] != rightBuffer[rightFrom++]) {
return false;
}
}
@@ -318,32 +319,30 @@ public final class WriteBuffers implements RandomAccessOutput {
* Compares part of the buffer with a part of an external byte array.
* Does not modify readPoint.
*/
- public boolean isEqual(byte[] left, int leftOffset, int leftLength, long rightOffset, int rightLength) {
+ public boolean isEqual(byte[] left, int leftLength, long rightOffset, int rightLength) {
if (rightLength != leftLength) {
return false;
}
- int rightIndex = getBufferIndex(rightOffset), rightFrom = getOffset(rightOffset);
- byte[] rightBuffer = writeBuffers.get(rightIndex);
- if (rightFrom + rightLength <= wbSize) {
- // TODO: allow using unsafe optionally.
- for (int i = 0; i < leftLength; ++i) {
- if (left[leftOffset + i] != rightBuffer[rightFrom + i]) {
- return false;
- }
- }
- return true;
- }
- for (int i = 0; i < rightLength; ++i) {
- if (rightFrom == wbSize) {
- ++rightIndex;
- rightBuffer = writeBuffers.get(rightIndex);
- rightFrom = 0;
- }
- if (left[leftOffset + i] != rightBuffer[rightFrom++]) {
- return false;
- }
+ return isEqual(left, 0, getBufferIndex(rightOffset), getOffset(rightOffset), leftLength);
+ }
+
+ /**
+ * Compares part of the buffer with a part of an external byte array.
+ * Does not modify readPoint.
+ */
+ public boolean isEqual(byte[] left, int leftOffset, int leftLength, long rightOffset, int rightLength) {
+ if (rightLength != leftLength) {
+ return false;
}
- return true;
+ return isEqual(left, leftOffset, getBufferIndex(rightOffset), getOffset(rightOffset), leftLength);
+ }
+
+ /**
+ * Compares the current readPosition of the buffer with the external byte array.
+ * Does not modify readPoint.
+ */
+ public boolean isEqual(byte[] left, int leftOffset, Position readPos, int length) {
+ return isEqual(left, leftOffset, readPos.bufferIndex, readPos.offset, length);
}
public void clear() {