Skip to content

Commit

Permalink
PHOENIX-1870 Fix NPE occurring during regex processing when joni libr…
Browse files Browse the repository at this point in the history
…ary not used (Shuxiong Ye)
  • Loading branch information
jtaylor-sfdc committed Apr 21, 2015
1 parent 5a63c63 commit 3f294aa
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 85 deletions.
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -236,10 +236,10 @@ private void init() {
LiteralExpression likeTypeExpression = (LiteralExpression)children.get(LIKE_TYPE_INDEX); LiteralExpression likeTypeExpression = (LiteralExpression)children.get(LIKE_TYPE_INDEX);
this.likeType = LikeType.valueOf((String)likeTypeExpression.getValue()); this.likeType = LikeType.valueOf((String)likeTypeExpression.getValue());
} }
ImmutableBytesWritable ptr = new ImmutableBytesWritable();
Expression e = getPatternExpression(); Expression e = getPatternExpression();
if (e instanceof LiteralExpression) { if (e.isStateless() && e.getDeterminism() == Determinism.ALWAYS && e.evaluate(null, ptr)) {
LiteralExpression patternExpression = (LiteralExpression)e; String value = (String) PVarchar.INSTANCE.toObject(ptr, e.getDataType(), e.getSortOrder());
String value = (String)patternExpression.getValue();
pattern = compilePattern(value); pattern = compilePattern(value);
} }
} }
Expand Down Expand Up @@ -294,7 +294,7 @@ public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
value = (String) strDataType.toObject(ptr, strSortOrder); value = (String) strDataType.toObject(ptr, strSortOrder);
} }
strDataType.coerceBytes(ptr, strDataType, strSortOrder, SortOrder.ASC); strDataType.coerceBytes(ptr, strDataType, strSortOrder, SortOrder.ASC);
pattern.matches(ptr, ptr); pattern.matches(ptr);
if (logger.isTraceEnabled()) { if (logger.isTraceEnabled()) {
boolean matched = ((Boolean) PBoolean.INSTANCE.toObject(ptr)).booleanValue(); boolean matched = ((Boolean) PBoolean.INSTANCE.toObject(ptr)).booleanValue();
logger.trace("LIKE(value='" + value + "'pattern='" + pattern.pattern() + "' is " + matched); logger.trace("LIKE(value='" + value + "'pattern='" + pattern.pattern() + "' is " + matched);
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import java.util.List; import java.util.List;


import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.phoenix.expression.Determinism;
import org.apache.phoenix.expression.Expression; import org.apache.phoenix.expression.Expression;
import org.apache.phoenix.expression.LiteralExpression;
import org.apache.phoenix.expression.util.regex.AbstractBasePattern; import org.apache.phoenix.expression.util.regex.AbstractBasePattern;
import org.apache.phoenix.parse.FunctionParseNode.Argument; import org.apache.phoenix.parse.FunctionParseNode.Argument;
import org.apache.phoenix.parse.FunctionParseNode.BuiltInFunction; import org.apache.phoenix.parse.FunctionParseNode.BuiltInFunction;
Expand Down Expand Up @@ -57,9 +57,11 @@
public abstract class RegexpReplaceFunction extends ScalarFunction { public abstract class RegexpReplaceFunction extends ScalarFunction {
public static final String NAME = "REGEXP_REPLACE"; public static final String NAME = "REGEXP_REPLACE";


private boolean hasReplaceStr; private static final PVarchar TYPE = PVarchar.INSTANCE;
private byte [] rStrBytes;
private int rStrOffset, rStrLen;
private AbstractBasePattern pattern; private AbstractBasePattern pattern;

public RegexpReplaceFunction() { } public RegexpReplaceFunction() { }


// Expect 1 arguments, the pattern. // Expect 1 arguments, the pattern.
Expand All @@ -71,44 +73,70 @@ public RegexpReplaceFunction(List<Expression> children) {
protected abstract AbstractBasePattern compilePatternSpec(String value); protected abstract AbstractBasePattern compilePatternSpec(String value);


private void init() { private void init() {
hasReplaceStr = ((LiteralExpression)getReplaceStrExpression()).getValue() != null; ImmutableBytesWritable tmpPtr = new ImmutableBytesWritable();
Object patternString = ((LiteralExpression)children.get(1)).getValue(); Expression e = getPatternStrExpression();
if (patternString != null) { if (e.isStateless() && e.getDeterminism() == Determinism.ALWAYS && e.evaluate(null, tmpPtr)) {
pattern = compilePatternSpec((String) patternString); String patternStr = (String) TYPE.toObject(tmpPtr, e.getDataType(), e.getSortOrder());
if (patternStr != null) pattern = compilePatternSpec(patternStr);
}
e = getReplaceStrExpression();
if (e.isStateless() && e.getDeterminism() == Determinism.ALWAYS && e.evaluate(null, tmpPtr)) {
TYPE.coerceBytes(tmpPtr, TYPE, e.getSortOrder(), SortOrder.ASC);
rStrBytes = tmpPtr.get();
rStrOffset = tmpPtr.getOffset();
rStrLen = tmpPtr.getLength();
} else {
rStrBytes = null;
} }
} }


@Override @Override
public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) { public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
// Can't parse if there is no replacement pattern. AbstractBasePattern pattern = this.pattern;
if (pattern == null) { if (pattern == null) {
return false; Expression e = getPatternStrExpression();
} if (!e.evaluate(tuple, ptr)) {
Expression sourceStrExpression = getSourceStrExpression(); return false;
if (!sourceStrExpression.evaluate(tuple, ptr)) { }
return false; String patternStr = (String) TYPE.toObject(ptr, e.getDataType(), e.getSortOrder());
if (patternStr == null) {
return false;
} else {
pattern = compilePatternSpec(patternStr);
}
} }
if (ptr == null) return false;
PVarchar type = PVarchar.INSTANCE; byte[] rStrBytes = this.rStrBytes;
type.coerceBytes(ptr, type, sourceStrExpression.getSortOrder(), SortOrder.ASC); int rStrOffset = this.rStrOffset, rStrLen = this.rStrLen;
ImmutableBytesWritable replacePtr = new ImmutableBytesWritable(); if (rStrBytes == null) {
if (hasReplaceStr) {
Expression replaceStrExpression = getReplaceStrExpression(); Expression replaceStrExpression = getReplaceStrExpression();
if (!replaceStrExpression.evaluate(tuple, replacePtr)) { if (!replaceStrExpression.evaluate(tuple, ptr)) {
return false; return false;
} }
type.coerceBytes(replacePtr, type, replaceStrExpression.getSortOrder(), SortOrder.ASC); TYPE.coerceBytes(ptr, TYPE, replaceStrExpression.getSortOrder(), SortOrder.ASC);
} else { rStrBytes = ptr.get();
replacePtr.set(type.toBytes("")); rStrOffset = ptr.getOffset();
rStrLen = ptr.getLength();
} }
pattern.replaceAll(ptr, replacePtr, ptr);
Expression sourceStrExpression = getSourceStrExpression();
if (!sourceStrExpression.evaluate(tuple, ptr)) {
return false;
}
TYPE.coerceBytes(ptr, TYPE, sourceStrExpression.getSortOrder(), SortOrder.ASC);

pattern.replaceAll(ptr, rStrBytes, rStrOffset, rStrLen);
return true; return true;
} }


private Expression getSourceStrExpression() { private Expression getSourceStrExpression() {
return children.get(0); return children.get(0);
} }


private Expression getPatternStrExpression() {
return children.get(1);
}

private Expression getReplaceStrExpression() { private Expression getReplaceStrExpression() {
return children.get(2); return children.get(2);
} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import java.util.List; import java.util.List;


import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.phoenix.expression.Determinism;
import org.apache.phoenix.expression.Expression; import org.apache.phoenix.expression.Expression;
import org.apache.phoenix.expression.LiteralExpression;
import org.apache.phoenix.expression.util.regex.AbstractBaseSplitter; import org.apache.phoenix.expression.util.regex.AbstractBaseSplitter;
import org.apache.phoenix.parse.FunctionParseNode; import org.apache.phoenix.parse.FunctionParseNode;
import org.apache.phoenix.parse.RegexpSplitParseNode; import org.apache.phoenix.parse.RegexpSplitParseNode;
Expand Down Expand Up @@ -53,6 +53,8 @@ public abstract class RegexpSplitFunction extends ScalarFunction {


public static final String NAME = "REGEXP_SPLIT"; public static final String NAME = "REGEXP_SPLIT";


private static final PVarchar TYPE = PVarchar.INSTANCE;

private AbstractBaseSplitter initializedSplitter = null; private AbstractBaseSplitter initializedSplitter = null;


public RegexpSplitFunction() {} public RegexpSplitFunction() {}
Expand All @@ -63,11 +65,12 @@ public RegexpSplitFunction(List<Expression> children) {
} }


private void init() { private void init() {
Expression patternExpression = children.get(1); ImmutableBytesWritable ptr = new ImmutableBytesWritable();
if (patternExpression instanceof LiteralExpression) { Expression e = getPatternStrExpression();
Object patternValue = ((LiteralExpression) patternExpression).getValue(); if (e.isStateless() && e.getDeterminism() == Determinism.ALWAYS && e.evaluate(null, ptr)) {
if (patternValue != null) { String pattern = (String) TYPE.toObject(ptr, TYPE, e.getSortOrder());
initializedSplitter = compilePatternSpec(patternValue.toString()); if (pattern != null) {
initializedSplitter = compilePatternSpec(pattern);
} }
} }
} }
Expand All @@ -87,31 +90,37 @@ public String getName() {


@Override @Override
public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) { public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
if (!children.get(0).evaluate(tuple, ptr)) {
return false;
}

Expression sourceStrExpression = children.get(0);
PVarchar type = PVarchar.INSTANCE;
type.coerceBytes(ptr, type, sourceStrExpression.getSortOrder(), SortOrder.ASC);

AbstractBaseSplitter splitter = initializedSplitter; AbstractBaseSplitter splitter = initializedSplitter;
if (splitter == null) { if (splitter == null) {
ImmutableBytesWritable tmpPtr = new ImmutableBytesWritable(); Expression e = getPatternStrExpression();
Expression patternExpression = children.get(1); if (e.evaluate(tuple, ptr)) {
if (!patternExpression.evaluate(tuple, tmpPtr)) { String pattern = (String) TYPE.toObject(ptr, TYPE, e.getSortOrder());
if (pattern != null) {
splitter = compilePatternSpec(pattern);
} else {
ptr.set(ByteUtil.EMPTY_BYTE_ARRAY); // set ptr to null
return true;
}
} else {
return false; return false;
} }
if (tmpPtr.getLength() == 0) {
ptr.set(ByteUtil.EMPTY_BYTE_ARRAY);
return true; // set ptr to null
}
String patternStr =
(String) PVarchar.INSTANCE.toObject(tmpPtr, patternExpression.getSortOrder());
splitter = compilePatternSpec(patternStr);
} }


return splitter.split(ptr, ptr); Expression e = getSourceStrExpression();
if (!e.evaluate(tuple, ptr)) {
return false;
}
TYPE.coerceBytes(ptr, TYPE, e.getSortOrder(), SortOrder.ASC);

return splitter.split(ptr);
}

private Expression getSourceStrExpression() {
return children.get(0);
}

private Expression getPatternStrExpression() {
return children.get(1);
} }


@Override @Override
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@


public abstract class AbstractBasePattern { public abstract class AbstractBasePattern {


public abstract void matches(ImmutableBytesWritable srcPtr, ImmutableBytesWritable outPtr); public abstract void matches(ImmutableBytesWritable srcPtr);


public abstract void replaceAll(ImmutableBytesWritable srcPtr, public abstract void replaceAll(ImmutableBytesWritable srcPtr, byte[] rStrBytes,
ImmutableBytesWritable replacePtr, ImmutableBytesWritable outPtr); int rStrOffset, int rStrLen);


public abstract void substr(ImmutableBytesWritable srcPtr, int offsetInStr); public abstract void substr(ImmutableBytesWritable srcPtr, int offsetInStr);


Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@
import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.io.ImmutableBytesWritable;


public abstract interface AbstractBaseSplitter { public abstract interface AbstractBaseSplitter {
public abstract boolean split(ImmutableBytesWritable srcPtr, ImmutableBytesWritable outPtr); public abstract boolean split(ImmutableBytesWritable srcPtr);
} }
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ public GuavaSplitter(String patternString) {
} }


@Override @Override
public boolean split(ImmutableBytesWritable srcPtr, ImmutableBytesWritable outPtr) { public boolean split(ImmutableBytesWritable srcPtr) {
String sourceStr = (String) PVarchar.INSTANCE.toObject(srcPtr); String sourceStr = (String) PVarchar.INSTANCE.toObject(srcPtr);
if (sourceStr == null) { // sourceStr evaluated to null if (sourceStr == null) { // sourceStr evaluated to null
outPtr.set(ByteUtil.EMPTY_BYTE_ARRAY); srcPtr.set(ByteUtil.EMPTY_BYTE_ARRAY);
} else { } else {
List<String> splitStrings = Lists.newArrayList(splitter.split(sourceStr)); List<String> splitStrings = Lists.newArrayList(splitter.split(sourceStr));
PhoenixArray splitArray = new PhoenixArray(PVarchar.INSTANCE, splitStrings.toArray()); PhoenixArray splitArray = new PhoenixArray(PVarchar.INSTANCE, splitStrings.toArray());
outPtr.set(PVarcharArray.INSTANCE.toBytes(splitArray)); srcPtr.set(PVarcharArray.INSTANCE.toBytes(splitArray));
} }
return true; return true;
} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -60,11 +60,10 @@ public JONIPattern(String patternString, int flags, Encoding coding) {
} }


@Override @Override
public void matches(ImmutableBytesWritable srcPtr, ImmutableBytesWritable outPtr) { public void matches(ImmutableBytesWritable srcPtr) {
Preconditions.checkNotNull(srcPtr); Preconditions.checkNotNull(srcPtr);
Preconditions.checkNotNull(outPtr);
boolean ret = matches(srcPtr.get(), srcPtr.getOffset(), srcPtr.getLength()); boolean ret = matches(srcPtr.get(), srcPtr.getOffset(), srcPtr.getLength());
outPtr.set(ret ? PDataType.TRUE_BYTES : PDataType.FALSE_BYTES); srcPtr.set(ret ? PDataType.TRUE_BYTES : PDataType.FALSE_BYTES);
} }


private boolean matches(byte[] bytes, int offset, int len) { private boolean matches(byte[] bytes, int offset, int len) {
Expand All @@ -80,15 +79,14 @@ public String pattern() {
} }


@Override @Override
public void replaceAll(ImmutableBytesWritable srcPtr, ImmutableBytesWritable replacePtr, public void replaceAll(ImmutableBytesWritable srcPtr, byte[] rStrBytes, int rStrOffset,
ImmutableBytesWritable replacedPtr) { int rStrLen) {
Preconditions.checkNotNull(srcPtr); Preconditions.checkNotNull(srcPtr);
Preconditions.checkNotNull(replacePtr); Preconditions.checkNotNull(rStrBytes);
Preconditions.checkNotNull(replacedPtr);
byte[] replacedBytes = byte[] replacedBytes =
replaceAll(srcPtr.get(), srcPtr.getOffset(), srcPtr.getLength(), replacePtr.get(), replaceAll(srcPtr.get(), srcPtr.getOffset(), srcPtr.getLength(), rStrBytes,
replacePtr.getOffset(), replacePtr.getLength()); rStrOffset, rStrLen);
replacedPtr.set(replacedBytes); srcPtr.set(replacedBytes);
} }


private byte[] replaceAll(byte[] srcBytes, int srcOffset, int srcLen, byte[] replaceBytes, private byte[] replaceAll(byte[] srcBytes, int srcOffset, int srcLen, byte[] replaceBytes,
Expand Down Expand Up @@ -154,8 +152,8 @@ private boolean substr(byte[] srcBytes, int offset, int range, ImmutableBytesWri
} }


@Override @Override
public boolean split(ImmutableBytesWritable srcPtr, ImmutableBytesWritable outPtr) { public boolean split(ImmutableBytesWritable srcPtr) {
return split(srcPtr.get(), srcPtr.getOffset(), srcPtr.getLength(), outPtr); return split(srcPtr.get(), srcPtr.getOffset(), srcPtr.getLength(), srcPtr);
} }


private boolean private boolean
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -44,13 +44,12 @@ public JavaPattern(String patternString, int flags) {
} }


@Override @Override
public void matches(ImmutableBytesWritable srcPtr, ImmutableBytesWritable outPtr) { public void matches(ImmutableBytesWritable srcPtr) {
Preconditions.checkNotNull(srcPtr); Preconditions.checkNotNull(srcPtr);
Preconditions.checkNotNull(outPtr);
String matcherSourceStr = (String) PVarchar.INSTANCE.toObject(srcPtr); String matcherSourceStr = (String) PVarchar.INSTANCE.toObject(srcPtr);
if (srcPtr.get().length == 0 && matcherSourceStr == null) matcherSourceStr = ""; if (srcPtr.get().length == 0 && matcherSourceStr == null) matcherSourceStr = "";
boolean ret = pattern.matcher(matcherSourceStr).matches(); boolean ret = pattern.matcher(matcherSourceStr).matches();
outPtr.set(ret ? PDataType.TRUE_BYTES : PDataType.FALSE_BYTES); srcPtr.set(ret ? PDataType.TRUE_BYTES : PDataType.FALSE_BYTES);
} }


@Override @Override
Expand All @@ -59,17 +58,16 @@ public String pattern() {
} }


@Override @Override
public void replaceAll(ImmutableBytesWritable srcPtr, ImmutableBytesWritable replacePtr, public void replaceAll(ImmutableBytesWritable srcPtr, byte[] rStrBytes, int rStrOffset,
ImmutableBytesWritable replacedPtr) { int rStrLen) {
Preconditions.checkNotNull(srcPtr); Preconditions.checkNotNull(srcPtr);
Preconditions.checkNotNull(replacePtr); Preconditions.checkNotNull(rStrBytes);
Preconditions.checkNotNull(replacedPtr);
String sourceStr = (String) PVarchar.INSTANCE.toObject(srcPtr); String sourceStr = (String) PVarchar.INSTANCE.toObject(srcPtr);
String replaceStr = (String) PVarchar.INSTANCE.toObject(replacePtr); String replaceStr = (String) PVarchar.INSTANCE.toObject(rStrBytes, rStrOffset, rStrLen);
if (srcPtr.get().length == 0 && sourceStr == null) sourceStr = ""; if (srcPtr.getLength() == 0 && sourceStr == null) sourceStr = "";
if (replacePtr.get().length == 0 && replaceStr == null) replaceStr = ""; if (rStrLen == 0 && replaceStr == null) replaceStr = "";
String replacedStr = pattern.matcher(sourceStr).replaceAll(replaceStr); String replacedStr = pattern.matcher(sourceStr).replaceAll(replaceStr);
replacedPtr.set(PVarchar.INSTANCE.toBytes(replacedStr)); srcPtr.set(PVarchar.INSTANCE.toBytes(replacedStr));
} }


@Override @Override
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ private void testReplaceAll(ImmutableBytesWritable replacePtr, AbstractBasePatte
String name) { String name) {
timer.reset(); timer.reset();
for (int i = 0; i < maxTimes; ++i) { for (int i = 0; i < maxTimes; ++i) {
pattern.replaceAll(dataPtr[i % 3], replacePtr, resultPtr); ImmutableBytesWritable ptr = dataPtr[i % 3];
resultPtr.set(ptr.get(), ptr.getOffset(), ptr.getLength());
pattern.replaceAll(resultPtr, replacePtr.get(), replacePtr.getOffset(),
replacePtr.getLength());
if (ENABLE_ASSERT) { if (ENABLE_ASSERT) {
String result = (String) PVarchar.INSTANCE.toObject(resultPtr); String result = (String) PVarchar.INSTANCE.toObject(resultPtr);
assertTrue((i % 3 == 1 && ":".equals(result)) assertTrue((i % 3 == 1 && ":".equals(result))
Expand All @@ -83,7 +86,9 @@ public void testReplaceAll() {
private void testLike(AbstractBasePattern pattern, String name) { private void testLike(AbstractBasePattern pattern, String name) {
timer.reset(); timer.reset();
for (int i = 0; i < maxTimes; ++i) { for (int i = 0; i < maxTimes; ++i) {
pattern.matches(dataPtr[i % 3], resultPtr); ImmutableBytesWritable ptr = dataPtr[i % 3];
resultPtr.set(ptr.get(), ptr.getOffset(), ptr.getLength());
pattern.matches(resultPtr);
if (ENABLE_ASSERT) { if (ENABLE_ASSERT) {
Boolean b = (Boolean) PBoolean.INSTANCE.toObject(resultPtr); Boolean b = (Boolean) PBoolean.INSTANCE.toObject(resultPtr);
assertTrue(i % 3 != 2 || b.booleanValue()); assertTrue(i % 3 != 2 || b.booleanValue());
Expand Down Expand Up @@ -120,7 +125,9 @@ public void testSubstr() {
private void testSplit(AbstractBaseSplitter pattern, String name) throws SQLException { private void testSplit(AbstractBaseSplitter pattern, String name) throws SQLException {
timer.reset(); timer.reset();
for (int i = 0; i < maxTimes; ++i) { for (int i = 0; i < maxTimes; ++i) {
boolean ret = pattern.split(dataPtr[i % 3], resultPtr); ImmutableBytesWritable ptr = dataPtr[i % 3];
resultPtr.set(ptr.get(), ptr.getOffset(), ptr.getLength());
boolean ret = pattern.split(resultPtr);
if (ENABLE_ASSERT) { if (ENABLE_ASSERT) {
PhoenixArray array = (PhoenixArray) PVarcharArray.INSTANCE.toObject(resultPtr); PhoenixArray array = (PhoenixArray) PVarcharArray.INSTANCE.toObject(resultPtr);
assertTrue(ret && (i % 3 != 1 || ((String[]) array.getArray()).length == 2)); assertTrue(ret && (i % 3 != 1 || ((String[]) array.getArray()).length == 2));
Expand Down

0 comments on commit 3f294aa

Please sign in to comment.