phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestay...@apache.org
Subject git commit: PHOENIX-1001 Using NEXT VALUE FOR 'sequence' as an input to a function cause a NPE (Thomas D'Silva)
Date Sun, 08 Jun 2014 04:26:02 GMT
Repository: phoenix
Updated Branches:
  refs/heads/3.0 b8b930245 -> 90a518e50


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/90a518e5
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/90a518e5
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/90a518e5

Branch: refs/heads/3.0
Commit: 90a518e50cc90839fdc7c0268851a20d274ed9af
Parents: b8b9302
Author: James Taylor <jtaylor@salesforce.com>
Authored: Sat Jun 7 21:27:34 2014 -0700
Committer: James Taylor <jtaylor@salesforce.com>
Committed: Sat Jun 7 21:27:34 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/90a518e5/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/90a518e5/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/90a518e5/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/90a518e5/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/90a518e5/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/90a518e5/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/90a518e5/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/90a518e5/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/90a518e5/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());
+    }
+
+}


Mime
View raw message