Repository: phoenix Updated Branches: refs/heads/4.0 40df40fe4 -> c75eae1a9
PHOENIX-1001 Using NEXT VALUE FOR 'sequence' as an input to a function cause a NPE (Thomas D'Silva) Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/9487b501 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/9487b501 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/9487b501 Branch: refs/heads/4.0 Commit: 9487b50135e63b249245da812715f4b4ad83ee7b Parents: e8f3444 Author: James Taylor <jtay...@salesforce.com> Authored: Sat Jun 7 21:23:36 2014 -0700 Committer: James Taylor <jtay...@salesforce.com> Committed: Sat Jun 7 21:23:36 2014 -0700 ---------------------------------------------------------------------- .../org/apache/phoenix/end2end/SequenceIT.java | 22 ++++++++ .../phoenix/compile/ExpressionCompiler.java | 48 +++++++---------- .../expression/BaseCompoundExpression.java | 2 + .../expression/ComparisonExpression.java | 5 +- .../phoenix/expression/InListExpression.java | 9 ++-- .../phoenix/expression/IsNullExpression.java | 3 +- .../phoenix/expression/LiteralExpression.java | 4 +- .../expression/function/FunctionExpression.java | 2 + .../org/apache/phoenix/util/ExpressionUtil.java | 57 ++++++++++++++++++++ 9 files changed, 111 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceIT.java index 02288ed..84bfece 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceIT.java @@ -587,6 +587,28 @@ public class SequenceIT extends BaseClientManagedTimeIT { conn.close(); } + @Test + public void testSelectNextValueAsInput() throws Exception { + nextConnection(); + conn.createStatement().execute("CREATE SEQUENCE foo.bar START WITH 3 INCREMENT BY 2"); + nextConnection(); + String query = "SELECT COALESCE(NEXT VALUE FOR foo.bar,1) FROM SYSTEM.\"SEQUENCE\""; + ResultSet rs = conn.prepareStatement(query).executeQuery(); + assertTrue(rs.next()); + assertEquals(3, rs.getInt(1)); + } + + @Test + public void testSelectNextValueInArithmetic() throws Exception { + nextConnection(); + conn.createStatement().execute("CREATE SEQUENCE foo.bar START WITH 3 INCREMENT BY 2"); + nextConnection(); + String query = "SELECT NEXT VALUE FOR foo.bar+1 FROM SYSTEM.\"SEQUENCE\""; + ResultSet rs = conn.prepareStatement(query).executeQuery(); + assertTrue(rs.next()); + assertEquals(4, rs.getInt(1)); + } + private void nextConnection() throws Exception { if (conn != null) conn.close(); long ts = nextTimestamp(); http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java index 4a05c0f..2d5e461 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java @@ -99,6 +99,7 @@ import org.apache.phoenix.schema.RowKeyValueAccessor; import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.TableRef; import org.apache.phoenix.schema.TypeMismatchException; +import org.apache.phoenix.util.ExpressionUtil; import org.apache.phoenix.util.IndexUtil; import org.apache.phoenix.util.SchemaUtil; @@ -272,7 +273,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio protected Expression addExpression(Expression expression) { return context.getExpressionManager().addIfAbsent(expression); } - + @Override /** * @param node a function expression node @@ -282,15 +283,9 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio children = node.validate(children, context); Expression expression = node.create(children, context); ImmutableBytesWritable ptr = context.getTempPtr(); - if (node.isStateless()) { - Object value = null; - PDataType type = expression.getDataType(); - if (expression.evaluate(null, ptr)) { - value = type.toObject(ptr); - } - return LiteralExpression.newConstant(value, type, expression.isDeterministic()); + if (ExpressionUtil.isConstant(expression)) { + return ExpressionUtil.getConstantExpression(expression, ptr); } - boolean isDeterministic = true; BuiltInFunctionInfo info = node.getInfo(); for (int i = 0; i < info.getRequiredArgCount(); i++) { // Optimization to catch cases where a required argument is null resulting in the function @@ -298,9 +293,8 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio // we can get the proper type to use. if (node.evalToNullIfParamIsNull(context, i)) { Expression child = children.get(i); - isDeterministic &= child.isDeterministic(); - if (child.isStateless() && (!child.evaluate(null, ptr) || ptr.getLength() == 0)) { - return LiteralExpression.newConstant(null, expression.getDataType(), isDeterministic); + if (ExpressionUtil.isNull(child, ptr)) { + return ExpressionUtil.getNullExpression(expression); } } } @@ -411,7 +405,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio context.getBindManager().addParamMetaData((BindParseNode)childNode, new DelegateDatum(caseExpression)); } } - if (node.isStateless()) { + if (ExpressionUtil.isConstant(caseExpression)) { ImmutableBytesWritable ptr = context.getTempPtr(); int index = caseExpression.evaluateIndexOf(null, ptr); if (index < 0) { @@ -473,7 +467,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio } } Expression expression = new LikeExpression(children); - if (node.isStateless()) { + if (ExpressionUtil.isConstant(expression)) { ImmutableBytesWritable ptr = context.getTempPtr(); if (!expression.evaluate(null, ptr)) { return LiteralExpression.newConstant(null, expression.isDeterministic()); @@ -658,12 +652,10 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio ImmutableBytesWritable ptr = context.getTempPtr(); // If all children are literals, just evaluate now - if (expression.isStateless()) { - if (!expression.evaluate(null,ptr) || ptr.getLength() == 0) { - return LiteralExpression.newConstant(null, expression.getDataType(), expression.isDeterministic()); - } - return LiteralExpression.newConstant(expression.getDataType().toObject(ptr), expression.getDataType(), expression.isDeterministic()); - } else if (isNull) { + if (ExpressionUtil.isConstant(expression)) { + return ExpressionUtil.getConstantExpression(expression, ptr); + } + else if (isNull) { return LiteralExpression.newConstant(null, expression.getDataType(), expression.isDeterministic()); } // Otherwise create and return the expression @@ -1065,11 +1057,8 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio } } ImmutableBytesWritable ptr = context.getTempPtr(); - if (expression.isStateless()) { - if (!expression.evaluate(null,ptr) || ptr.getLength() == 0) { - return LiteralExpression.newConstant(null, expression.getDataType(), expression.isDeterministic()); - } - return LiteralExpression.newConstant(expression.getDataType().toObject(ptr), expression.getDataType(), expression.isDeterministic()); + if (ExpressionUtil.isConstant(expression)) { + return ExpressionUtil.getConstantExpression(expression, ptr); } return wrapGroupByExpression(expression); } @@ -1147,21 +1136,20 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio } ImmutableBytesWritable ptr = context.getTempPtr(); Object[] elements = new Object[children.size()]; - if (node.isStateless()) { - boolean isDeterministic = true; + + ArrayConstructorExpression arrayExpression = new ArrayConstructorExpression(children, arrayElemDataType); + if (ExpressionUtil.isConstant(arrayExpression)) { for (int i = 0; i < children.size(); i++) { Expression child = children.get(i); - isDeterministic &= child.isDeterministic(); child.evaluate(null, ptr); Object value = arrayElemDataType.toObject(ptr, child.getDataType(), child.getSortOrder()); elements[i] = LiteralExpression.newConstant(value, child.getDataType(), child.isDeterministic()).getValue(); } Object value = PArrayDataType.instantiatePhoenixArray(arrayElemDataType, elements); return LiteralExpression.newConstant(value, - PDataType.fromTypeId(arrayElemDataType.getSqlType() + PDataType.ARRAY_TYPE_BASE), isDeterministic); + PDataType.fromTypeId(arrayElemDataType.getSqlType() + PDataType.ARRAY_TYPE_BASE), true); } - ArrayConstructorExpression arrayExpression = new ArrayConstructorExpression(children, arrayElemDataType); return wrapGroupByExpression(arrayExpression); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java index f840c08..03df653 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java @@ -21,6 +21,7 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.apache.hadoop.io.WritableUtils; @@ -37,6 +38,7 @@ public abstract class BaseCompoundExpression extends BaseExpression { private boolean requiresFinalEvaluation; public BaseCompoundExpression() { + init(Collections.<Expression>emptyList()); } public BaseCompoundExpression(List<Expression> children) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java index be32b55..008ae7b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java @@ -35,6 +35,7 @@ import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.TypeMismatchException; import org.apache.phoenix.schema.tuple.Tuple; import org.apache.phoenix.util.ByteUtil; +import org.apache.phoenix.util.ExpressionUtil; import org.apache.phoenix.util.StringUtil; import com.google.common.collect.Lists; @@ -59,8 +60,8 @@ public class ComparisonExpression extends BaseCompoundExpression { } private static void addEqualityExpression(Expression lhs, Expression rhs, List<Expression> andNodes, ImmutableBytesWritable ptr) throws SQLException { - boolean isLHSNull = lhs.isStateless() && (!lhs.evaluate(null, ptr) || ptr.getLength()==0); - boolean isRHSNull = rhs.isStateless() && (!rhs.evaluate(null, ptr) || ptr.getLength()==0); + boolean isLHSNull = ExpressionUtil.isNull(lhs, ptr); + boolean isRHSNull = ExpressionUtil.isNull(rhs, ptr); if (isLHSNull && isRHSNull) { // null == null will end up making the query degenerate andNodes.add(LiteralExpression.newConstant(false, PDataType.BOOLEAN)); } else if (isLHSNull) { // AND rhs IS NULL http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java index a7977f1..979af0c 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java @@ -36,6 +36,7 @@ import org.apache.phoenix.schema.PDataType; import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.tuple.Tuple; import org.apache.phoenix.util.ByteUtil; +import org.apache.phoenix.util.ExpressionUtil; import com.google.common.collect.Lists; import com.google.common.collect.Sets; @@ -93,12 +94,8 @@ public class InListExpression extends BaseSingleExpression { if (isNegate) { expression = NotExpression.create(expression, ptr); } - if (expression.isStateless()) { - if (!expression.evaluate(null, ptr) || ptr.getLength() == 0) { - return LiteralExpression.newConstant(null,expression.getDataType(), expression.isDeterministic()); - } - Object value = expression.getDataType().toObject(ptr); - return LiteralExpression.newConstant(value, expression.getDataType(), expression.isDeterministic()); + if (ExpressionUtil.isConstant(expression)) { + return ExpressionUtil.getConstantExpression(expression, ptr); } return expression; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/main/java/org/apache/phoenix/expression/IsNullExpression.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/IsNullExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/IsNullExpression.java index 9dc4500..4162dfc 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/IsNullExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/IsNullExpression.java @@ -27,6 +27,7 @@ import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.phoenix.expression.visitor.ExpressionVisitor; import org.apache.phoenix.schema.PDataType; import org.apache.phoenix.schema.tuple.Tuple; +import org.apache.phoenix.util.ExpressionUtil; /** @@ -43,7 +44,7 @@ public class IsNullExpression extends BaseSingleExpression { if (!child.isNullable()) { return LiteralExpression.newConstant(negate, PDataType.BOOLEAN, child.isDeterministic()); } - if (child.isStateless()) { + if (ExpressionUtil.isConstant(child)) { boolean evaluated = child.evaluate(null, ptr); return LiteralExpression.newConstant(negate ^ (!evaluated || ptr.getLength() == 0), PDataType.BOOLEAN, child.isDeterministic()); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java index 9c1c604..91a3125 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java @@ -48,8 +48,8 @@ import com.google.common.base.Preconditions; * @since 0.1 */ public class LiteralExpression extends BaseTerminalExpression { - public static final LiteralExpression NULL_EXPRESSION = new LiteralExpression(null, false); - private static final LiteralExpression ND_NULL_EXPRESSION = new LiteralExpression(null, true); + public static final LiteralExpression NULL_EXPRESSION = new LiteralExpression(null, true); + private static final LiteralExpression ND_NULL_EXPRESSION = new LiteralExpression(null, false); private static final LiteralExpression[] TYPED_NULL_EXPRESSIONS = new LiteralExpression[PDataType.values().length * 2]; static { for (int i = 0; i < PDataType.values().length; i++) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FunctionExpression.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FunctionExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FunctionExpression.java index 472c39d..45c3e15 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FunctionExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FunctionExpression.java @@ -59,6 +59,8 @@ public abstract class FunctionExpression extends BaseCompoundExpression { @Override public final String toString() { StringBuilder buf = new StringBuilder(getName() + "("); + if (children.size()==0) + return buf.append(")").toString(); for (int i = 0; i < children.size() - 1; i++) { buf.append(children.get(i) + ", "); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/9487b501/phoenix-core/src/main/java/org/apache/phoenix/util/ExpressionUtil.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ExpressionUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/ExpressionUtil.java new file mode 100644 index 0000000..227a385 --- /dev/null +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ExpressionUtil.java @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE + * file distributed with this work for additional information regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by + * applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +package org.apache.phoenix.util; + +import java.sql.SQLException; +import java.util.List; + +import org.apache.hadoop.hbase.io.ImmutableBytesWritable; +import org.apache.phoenix.expression.Expression; +import org.apache.phoenix.expression.LiteralExpression; +import org.apache.phoenix.expression.function.CurrentDateFunction; +import org.apache.phoenix.expression.function.CurrentTimeFunction; +import org.apache.phoenix.expression.function.FunctionExpression; +import org.apache.phoenix.schema.PDataType; + +import com.google.common.collect.Lists; + +public class ExpressionUtil { + + @SuppressWarnings("unchecked") + private static final List<Class<? extends FunctionExpression>> OVERRIDE_LITERAL_FUNCTIONS = Lists + .<Class<? extends FunctionExpression>> newArrayList(CurrentDateFunction.class, CurrentTimeFunction.class); + + private ExpressionUtil() {} + + public static boolean isConstant(Expression expression) { + return (expression.isStateless() && expression.isDeterministic() || OVERRIDE_LITERAL_FUNCTIONS + .contains(expression.getClass())); + } + + public static LiteralExpression getConstantExpression(Expression expression, ImmutableBytesWritable ptr) + throws SQLException { + Object value = null; + PDataType type = expression.getDataType(); + if (expression.evaluate(null, ptr) && ptr.getLength() != 0) { + value = type.toObject(ptr); + } + return LiteralExpression.newConstant(value, type, expression.isDeterministic()); + } + + public static boolean isNull(Expression expression, ImmutableBytesWritable ptr) { + return expression.isStateless() && expression.isDeterministic() + && (!expression.evaluate(null, ptr) || ptr.getLength() == 0); + } + + public static LiteralExpression getNullExpression(Expression expression) throws SQLException { + return LiteralExpression.newConstant(null, expression.getDataType(), expression.isDeterministic()); + } + +}