Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-1.1 40936ad0a -> ee5683603


PHOENIX-3006 Fix all ScalarFunctions to implement clone(List) or <init>(List). 
(James Taylor & Lars Hofhansl)


Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/ee568360
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/ee568360
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/ee568360

Branch: refs/heads/4.x-HBase-1.1
Commit: ee5683603fd29c4d49a92cbb67c8bd75c33d45ea
Parents: 40936ad
Author: Lars Hofhansl <la...@apache.org>
Authored: Sat Jun 18 22:10:55 2016 -0700
Committer: Lars Hofhansl <la...@apache.org>
Committed: Sat Jun 18 22:11:55 2016 -0700

----------------------------------------------------------------------
 .../expression/function/CeilDateExpression.java |  2 +-
 .../function/CeilDecimalExpression.java         | 12 ++++------
 .../function/CeilTimestampExpression.java       | 14 +++++------
 .../function/FloorDateExpression.java           |  2 +-
 .../function/FloorDecimalExpression.java        | 12 ++++------
 .../function/RoundDateExpression.java           |  2 +-
 .../function/RoundDecimalExpression.java        |  2 +-
 .../function/RoundTimestampExpression.java      |  2 +-
 .../BuiltinFunctionConstructorTest.java         | 25 ++++++++++----------
 9 files changed, 35 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDateExpression.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDateExpression.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDateExpression.java
index a3bf6f8..e3cd985 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDateExpression.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDateExpression.java
@@ -76,7 +76,7 @@ public class CeilDateExpression extends RoundDateExpression {
         
     }
     
-    CeilDateExpression(List<Expression> children) {
+    public CeilDateExpression(List<Expression> children) {
         super(children);
     }
     

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDecimalExpression.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDecimalExpression.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDecimalExpression.java
index 208d5ee..9c64896 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDecimalExpression.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilDecimalExpression.java
@@ -17,6 +17,7 @@
  */
 package org.apache.phoenix.expression.function;
 
+import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.sql.SQLException;
 import java.util.List;
@@ -24,16 +25,13 @@ import java.util.List;
 import org.apache.phoenix.expression.Determinism;
 import org.apache.phoenix.expression.Expression;
 import org.apache.phoenix.expression.LiteralExpression;
-import org.apache.phoenix.schema.types.PDecimal;
-
-import com.google.common.collect.Lists;
-
-import java.math.BigDecimal;
-
 import org.apache.phoenix.query.KeyRange;
+import org.apache.phoenix.schema.types.PDecimal;
 import org.apache.phoenix.schema.types.PInteger;
 import org.apache.phoenix.schema.types.PLong;
 
+import com.google.common.collect.Lists;
+
 /**
  *
  * Class encapsulating the CEIL operation on a {@link 
org.apache.phoenix.schema.types.PDecimal}
@@ -45,7 +43,7 @@ public class CeilDecimalExpression extends 
RoundDecimalExpression {
 
     public CeilDecimalExpression() {}
 
-    private CeilDecimalExpression(List<Expression> children) {
+    public CeilDecimalExpression(List<Expression> children) {
         super(children);
     }
 

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilTimestampExpression.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilTimestampExpression.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilTimestampExpression.java
index c2d7daf..f69f2f8 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilTimestampExpression.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilTimestampExpression.java
@@ -22,19 +22,19 @@ import java.sql.Timestamp;
 import java.util.List;
 
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
-
-import com.google.common.collect.Lists;
 import org.apache.phoenix.expression.CoerceExpression;
 import org.apache.phoenix.expression.Expression;
 import org.apache.phoenix.expression.LiteralExpression;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.tuple.Tuple;
+import org.apache.phoenix.schema.types.PDataType;
+import org.apache.phoenix.schema.types.PDataType.PDataCodec;
 import org.apache.phoenix.schema.types.PDate;
 import org.apache.phoenix.schema.types.PTimestamp;
 import org.apache.phoenix.schema.types.PUnsignedDate;
 import org.apache.phoenix.schema.types.PUnsignedTimestamp;
-import org.apache.phoenix.schema.SortOrder;
-import org.apache.phoenix.schema.types.PDataType;
-import org.apache.phoenix.schema.types.PDataType.PDataCodec;
-import org.apache.phoenix.schema.tuple.Tuple;
+
+import com.google.common.collect.Lists;
 
 /**
  * 
@@ -49,7 +49,7 @@ public class CeilTimestampExpression extends 
CeilDateExpression {
     
     public CeilTimestampExpression() {}
     
-    private CeilTimestampExpression(List<Expression> children) {
+    public CeilTimestampExpression(List<Expression> children) {
         super(children);
     }
     

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDateExpression.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDateExpression.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDateExpression.java
index 4dc840e..c0eca8a 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDateExpression.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDateExpression.java
@@ -46,7 +46,7 @@ public class FloorDateExpression extends RoundDateExpression {
     
     public FloorDateExpression() {}
     
-    protected FloorDateExpression(List<Expression> children) {
+    public FloorDateExpression(List<Expression> children) {
         super(children);
     }
     

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDecimalExpression.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDecimalExpression.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDecimalExpression.java
index 2d81c2e..7351cca 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDecimalExpression.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorDecimalExpression.java
@@ -17,6 +17,7 @@
  */
 package org.apache.phoenix.expression.function;
 
+import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.sql.SQLException;
 import java.util.List;
@@ -24,16 +25,13 @@ import java.util.List;
 import org.apache.phoenix.expression.Determinism;
 import org.apache.phoenix.expression.Expression;
 import org.apache.phoenix.expression.LiteralExpression;
-import org.apache.phoenix.schema.types.PDecimal;
-
-import com.google.common.collect.Lists;
-
-import java.math.BigDecimal;
-
 import org.apache.phoenix.query.KeyRange;
+import org.apache.phoenix.schema.types.PDecimal;
 import org.apache.phoenix.schema.types.PInteger;
 import org.apache.phoenix.schema.types.PLong;
 
+import com.google.common.collect.Lists;
+
 /**
  *
  * Class encapsulating the FLOOR operation on 
@@ -46,7 +44,7 @@ public class FloorDecimalExpression extends 
RoundDecimalExpression {
 
     public FloorDecimalExpression() {}
 
-    private FloorDecimalExpression(List<Expression> children) {
+    public FloorDecimalExpression(List<Expression> children) {
         super(children);
     }
 

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java
index 132bb51..c560b43 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java
@@ -119,7 +119,7 @@ public class RoundDateExpression extends ScalarFunction {
         return LiteralExpression.newConstant(multiplier, PInteger.INSTANCE, 
Determinism.ALWAYS);
     }
     
-    RoundDateExpression(List<Expression> children) {
+    public RoundDateExpression(List<Expression> children) {
         super(children.subList(0, 1));
         int numChildren = children.size();
         Object timeUnitValue = ((LiteralExpression)children.get(1)).getValue();

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java
index 7f44082..65ffacb 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java
@@ -92,7 +92,7 @@ public class RoundDecimalExpression extends ScalarFunction {
 
     public RoundDecimalExpression() {}
 
-    protected RoundDecimalExpression(List<Expression> children) {
+    public RoundDecimalExpression(List<Expression> children) {
         super(children);
         LiteralExpression scaleChild = (LiteralExpression)children.get(1);
         PDataType scaleType = scaleChild.getDataType();

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundTimestampExpression.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundTimestampExpression.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundTimestampExpression.java
index 9129285..7b5ba93 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundTimestampExpression.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundTimestampExpression.java
@@ -54,7 +54,7 @@ public class RoundTimestampExpression extends 
RoundDateExpression {
 
     public RoundTimestampExpression() {}
     
-    private RoundTimestampExpression(List<Expression> children) {
+    public RoundTimestampExpression(List<Expression> children) {
         super(children);
     }
     

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ee568360/phoenix-core/src/test/java/org/apache/phoenix/expression/function/BuiltinFunctionConstructorTest.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/test/java/org/apache/phoenix/expression/function/BuiltinFunctionConstructorTest.java
 
b/phoenix-core/src/test/java/org/apache/phoenix/expression/function/BuiltinFunctionConstructorTest.java
index 69f39fb..4db6628 100644
--- 
a/phoenix-core/src/test/java/org/apache/phoenix/expression/function/BuiltinFunctionConstructorTest.java
+++ 
b/phoenix-core/src/test/java/org/apache/phoenix/expression/function/BuiltinFunctionConstructorTest.java
@@ -17,12 +17,14 @@
  */
 package org.apache.phoenix.expression.function;
 
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
+import java.lang.reflect.Modifier;
 import java.util.List;
 
-import org.apache.phoenix.expression.function.ScalarFunction;
 import org.apache.phoenix.expression.Expression;
 import org.apache.phoenix.expression.ExpressionType;
 import org.junit.Test;
@@ -30,23 +32,22 @@ import org.junit.Test;
 public class BuiltinFunctionConstructorTest {
 
     @Test
-    public void testChildrenListConstructors() {
+    public void testChildrenListConstructors() throws Exception {
         ExpressionType[] types = ExpressionType.values();
-        List<Expression> children = new ArrayList<>();
         for(int i = 0; i < types.length; i++) {
             try {
-                
if((ScalarFunction.class.isAssignableFrom(types[i].getExpressionClass())) && 
(types[i].getExpressionClass() != UDFExpression.class)) {
-                       Method cloneMethod = 
types[i].getExpressionClass().getMethod("clone", List.class);
+                Class<? extends Expression> expressionClass= 
types[i].getExpressionClass();
+                if(!Modifier.isAbstract( expressionClass.getModifiers() ) && 
(ScalarFunction.class.isAssignableFrom(expressionClass)) && (expressionClass != 
UDFExpression.class)) {
+                       Method cloneMethod = expressionClass.getMethod("clone", 
List.class);
+                       assertNotNull(cloneMethod);
                        // ScalarFunctions that implement 
clone(List<Expression>) don't need to implement a constructor that takes a 
List<Expression>  
-                       if (cloneMethod==null) {
-                           Constructor cons = 
types[i].getExpressionClass().getDeclaredConstructor(List.class);
-                           cons.setAccessible(true);
-                           cons.newInstance(children);
+                       if (cloneMethod.getDeclaringClass() == 
ScalarFunction.class) {
+                           Constructor cons = 
expressionClass.getDeclaredConstructor(List.class);
+                           assertTrue("Constructor for " + expressionClass + " 
is not public", Modifier.isPublic(cons.getModifiers()));
                        }
                 }
-            } catch (NoSuchMethodException e) {
-                throw new RuntimeException(e);
             } catch (Exception e) {
+                throw new RuntimeException("Unable to find required 
List<Expression> constructor " + types[i].getExpressionClass().getName(), e);
             }
         }
     }

Reply via email to