phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestay...@apache.org
Subject git commit: PHOENIX-761 Implement getKeyPart for round/ceil/floor of a DECIMAL value (Kyle Buzsaki)
Date Wed, 27 Aug 2014 05:39:20 GMT
Repository: phoenix
Updated Branches:
  refs/heads/3.0 ca86a4763 -> c58532283


PHOENIX-761 Implement getKeyPart for round/ceil/floor of a DECIMAL value (Kyle Buzsaki)


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

Branch: refs/heads/3.0
Commit: c5853228355a2d2042e9c98c1e473d0afe700cea
Parents: ca86a47
Author: James Taylor <jtaylor@salesforce.com>
Authored: Tue Aug 26 22:43:31 2014 -0700
Committer: James Taylor <jtaylor@salesforce.com>
Committed: Tue Aug 26 22:43:31 2014 -0700

----------------------------------------------------------------------
 .../function/CeilDecimalExpression.java         |  89 +--
 .../function/FloorDecimalExpression.java        |  40 +-
 .../function/RoundDecimalExpression.java        | 250 +++++++-
 .../RoundFloorCeilExpressionsTest.java          | 582 +++++++++++++++----
 4 files changed, 800 insertions(+), 161 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/c5853228/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 5e3f2f6..d6b4e45 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
@@ -26,63 +26,80 @@ import org.apache.phoenix.expression.LiteralExpression;
 import org.apache.phoenix.schema.PDataType;
 
 import com.google.common.collect.Lists;
+import java.math.BigDecimal;
+import org.apache.phoenix.query.KeyRange;
+import static org.apache.phoenix.schema.PDataType.DECIMAL;
 
 /**
- * 
+ *
  * Class encapsulating the CEIL operation on a {@link org.apache.phoenix.schema.PDataType#DECIMAL}
  *
- * 
+ *
  * @since 3.0.0
  */
 public class CeilDecimalExpression extends RoundDecimalExpression {
-    
+
     public CeilDecimalExpression() {}
-    
+
     private CeilDecimalExpression(List<Expression> children) {
         super(children);
     }
-    
-  /**
-   * Creates a {@link CeilDecimalExpression} with rounding scale given by @param scale.
-   *
-   */
-   public static Expression create(Expression expr, int scale) throws SQLException {
+
+    /**
+     * Creates a {@link CeilDecimalExpression} with rounding scale given by @param scale.
+     *
+     */
+    public static Expression create(Expression expr, int scale) throws SQLException {
        if (expr.getDataType().isCoercibleTo(PDataType.LONG)) {
-           return expr;
-       }
-       Expression scaleExpr = LiteralExpression.newConstant(scale, PDataType.INTEGER, true);
-       List<Expression> expressions = Lists.newArrayList(expr, scaleExpr);
-       return new CeilDecimalExpression(expressions);
-   }
-   
-   public static Expression create(List<Expression> exprs) throws SQLException {
-       Expression expr = exprs.get(0);
+            return expr;
+        }
+        Expression scaleExpr = LiteralExpression.newConstant(scale, PDataType.INTEGER, true);
+        List<Expression> expressions = Lists.newArrayList(expr, scaleExpr);
+        return new CeilDecimalExpression(expressions);
+    }
+
+    public static Expression create(List<Expression> exprs) throws SQLException {
+        Expression expr = exprs.get(0);
        if (expr.getDataType().isCoercibleTo(PDataType.LONG)) {
-           return expr;
-       }
+            return expr;
+        }
        if (exprs.size() == 1) {
-           Expression scaleExpr = LiteralExpression.newConstant(0, PDataType.INTEGER, true);
-           exprs = Lists.newArrayList(expr, scaleExpr);
-       }
-       return new CeilDecimalExpression(exprs);
-   }
-   
-   /**
-    * Creates a {@link CeilDecimalExpression} with a default scale of 0 used for rounding. 
-    *
-    */
-   public static Expression create(Expression expr) throws SQLException {
-       return create(expr, 0);
-   }
-    
+            Expression scaleExpr = LiteralExpression.newConstant(0, PDataType.INTEGER, true);
+            exprs = Lists.newArrayList(expr, scaleExpr);
+        }
+        return new CeilDecimalExpression(exprs);
+    }
+
+    /**
+     * Creates a {@link CeilDecimalExpression} with a default scale of 0 used for rounding.
+     *
+     */
+    public static Expression create(Expression expr) throws SQLException {
+        return create(expr, 0);
+    }
+
     @Override
     protected RoundingMode getRoundingMode() {
         return RoundingMode.CEILING;
     }
-    
+
     @Override
     public String getName() {
         return CeilFunction.NAME;
     }
     
+    /**
+     * {@inheritDoc }
+     */
+    @Override
+    protected KeyRange getInputRangeProducing(BigDecimal result) {
+        if(!hasEnoughPrecisionToProduce(result)) {
+            throw new IllegalArgumentException("Cannot produce input range for decimal " + result 
+                + ", not enough precision with scale " + getRoundingScale());
+        }
+        byte[] lowerRange = DECIMAL.toBytes(stepPrevInScale(result));
+        byte[] upperRange = DECIMAL.toBytes(result);
+        return KeyRange.getKeyRange(lowerRange, false, upperRange, true);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c5853228/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 539dbfa..b5566ca 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
@@ -26,25 +26,28 @@ import org.apache.phoenix.expression.LiteralExpression;
 import org.apache.phoenix.schema.PDataType;
 
 import com.google.common.collect.Lists;
+import java.math.BigDecimal;
+import org.apache.phoenix.query.KeyRange;
+import static org.apache.phoenix.schema.PDataType.DECIMAL;
 
 /**
- * 
+ *
  * Class encapsulating the FLOOR operation on 
  * a column/literal of type {@link org.apache.phoenix.schema.PDataType#DECIMAL}.
  *
- * 
+ *
  * @since 3.0.0
  */
 public class FloorDecimalExpression extends RoundDecimalExpression {
-    
+
     public FloorDecimalExpression() {}
-    
+
     private FloorDecimalExpression(List<Expression> children) {
         super(children);
     }
-    
+
     /**
-     * Creates a {@link FloorDecimalExpression} with rounding scale given by @param scale. 
+     * Creates a {@link FloorDecimalExpression} with rounding scale given by @param scale.
      *
      */
     public static Expression create(Expression expr, int scale) throws SQLException {
@@ -55,7 +58,7 @@ public class FloorDecimalExpression extends RoundDecimalExpression {
         List<Expression> expressions = Lists.newArrayList(expr, scaleExpr);
         return new FloorDecimalExpression(expressions);
     }
-    
+
     public static Expression create(List<Expression> exprs) throws SQLException {
         Expression expr = exprs.get(0);
         if (expr.getDataType().isCoercibleTo(PDataType.LONG)) {
@@ -67,22 +70,37 @@ public class FloorDecimalExpression extends RoundDecimalExpression {
         }
         return new FloorDecimalExpression(exprs);
     }
-    
+
     /**
-     * Creates a {@link FloorDecimalExpression} with a default scale of 0 used for rounding. 
+     * Creates a {@link FloorDecimalExpression} with a default scale of 0 used for rounding.
      *
      */
     public static Expression create(Expression expr) throws SQLException {
         return create(expr, 0);
     }
-    
+
     @Override
     protected RoundingMode getRoundingMode() {
         return RoundingMode.FLOOR;
     }
-    
+
     @Override
     public String getName() {
         return FloorFunction.NAME;
     }
+
+    /**
+     * {@inheritDoc }
+     */
+    @Override
+    protected KeyRange getInputRangeProducing(BigDecimal result) {
+        if(!hasEnoughPrecisionToProduce(result)) {
+            throw new IllegalArgumentException("Cannot produce input range for decimal " + result 
+                + ", not enough precision with scale " + getRoundingScale());
+        }
+        byte[] lowerRange = DECIMAL.toBytes(result);
+        byte[] upperRange = DECIMAL.toBytes(stepNextInScale(result));
+        return KeyRange.getKeyRange(lowerRange, upperRange);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c5853228/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 64f957e..92756a5 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
@@ -34,22 +34,28 @@ import org.apache.phoenix.schema.PDataType;
 import org.apache.phoenix.schema.tuple.Tuple;
 
 import com.google.common.collect.Lists;
+import java.util.Collections;
+import org.apache.hadoop.hbase.filter.CompareFilter;
+import org.apache.phoenix.compile.KeyPart;
+import static org.apache.phoenix.expression.function.ScalarFunction.evaluateExpression;
+import org.apache.phoenix.query.KeyRange;
+import org.apache.phoenix.schema.PColumn;
+import static org.apache.phoenix.schema.PDataType.DECIMAL;
 
 /**
- * 
- * Class encapsulating the process for rounding off a column/literal of 
- * type {@link org.apache.phoenix.schema.PDataType#DECIMAL}
  *
- * 
+ * Class encapsulating the process for rounding off a column/literal of type
+ * {@link org.apache.phoenix.schema.PDataType#DECIMAL}
+ *
+ *
  * @since 3.0.0
  */
-
 public class RoundDecimalExpression extends ScalarFunction {
-    
+
     private int scale;
-    
+
     /**
-     * Creates a {@link RoundDecimalExpression} with rounding scale given by @param scale. 
+     * Creates a {@link RoundDecimalExpression} with rounding scale given by @param scale.
      *
      */
     public static Expression create(Expression expr, int scale) throws SQLException {
@@ -60,15 +66,15 @@ public class RoundDecimalExpression extends ScalarFunction {
         List<Expression> expressions = Lists.newArrayList(expr, scaleExpr);
         return new RoundDecimalExpression(expressions);
     }
-    
+
     /**
-     * Creates a {@link RoundDecimalExpression} with a default scale of 0 used for rounding. 
+     * Creates a {@link RoundDecimalExpression} with a default scale of 0 used for rounding.
      *
      */
     public static Expression create(Expression expr) throws SQLException {
         return create(expr, 0);
     }
-    
+
     public static Expression create(List<Expression> exprs) throws SQLException {
         Expression expr = exprs.get(0);
         if (expr.getDataType().isCoercibleTo(PDataType.LONG)) {
@@ -80,9 +86,9 @@ public class RoundDecimalExpression extends ScalarFunction {
         }
         return new RoundDecimalExpression(exprs);
     }
-    
+
     public RoundDecimalExpression() {}
-    
+
     protected RoundDecimalExpression(List<Expression> children) {
         super(children);
         LiteralExpression scaleChild = (LiteralExpression)children.get(1);
@@ -97,7 +103,7 @@ public class RoundDecimalExpression extends ScalarFunction {
                 }
             }
             throw new IllegalDataException("Invalid second argument for scale: " + scaleValue + ". The scale must be between 0 and " + PDataType.MAX_PRECISION + " inclusive.");
-        } 
+        }
     }
 
     @Override
@@ -116,10 +122,14 @@ public class RoundDecimalExpression extends ScalarFunction {
     public PDataType getDataType() {
         return PDataType.DECIMAL;
     }
-    
+
     protected RoundingMode getRoundingMode() {
         return RoundingMode.HALF_UP;
     }
+
+    protected final int getRoundingScale() {
+        return scale;
+    }
     
     @Override
     public void readFields(DataInput input) throws IOException {
@@ -137,10 +147,218 @@ public class RoundDecimalExpression extends ScalarFunction {
     public String getName() {
         return RoundFunction.NAME;
     }
-    
+
     @Override
     public OrderPreserving preservesOrder() {
         return OrderPreserving.YES;
     }
 
+    @Override
+    public int getKeyFormationTraversalIndex() {
+        return 0;
+    }
+
+    @Override
+    public KeyPart newKeyPart(final KeyPart childPart) {
+        return new KeyPart() {
+            private final List<Expression> extractNodes = Collections.<Expression>singletonList(RoundDecimalExpression.this);
+
+            @Override
+            public PColumn getColumn() {
+                return childPart.getColumn();
+            }
+
+            @Override
+            public List<Expression> getExtractNodes() {
+                return extractNodes;
+            }
+
+            @Override
+            public KeyRange getKeyRange(CompareFilter.CompareOp op, Expression rhs) {
+                final BigDecimal rhsDecimal = (BigDecimal) DECIMAL.toObject(evaluateExpression(rhs));
+                
+                // equality requires an exact match. if rounding would cut off more precision
+                // than needed for a match, it's impossible for there to be any matches
+                if(op == CompareFilter.CompareOp.EQUAL && !hasEnoughPrecisionToProduce(rhsDecimal)) {
+                    return KeyRange.EMPTY_RANGE;
+                }
+                
+                // if the decimal needs to be rounded, round it such that the given 
+                // operator will still be valid
+                BigDecimal roundedDecimal = roundAndPreserveOperator(rhsDecimal, op);
+                
+                // the range of big decimals that could be rounded to produce the rounded result
+                // alternatively, the "rounding bucket" that this decimal falls into
+                final KeyRange equalityRange = getInputRangeProducing(roundedDecimal);
+                boolean lowerInclusive = equalityRange.isLowerInclusive();
+                boolean upperInclusive = equalityRange.isUpperInclusive();
+                byte[] lowerRange = KeyRange.UNBOUND;
+                byte[] upperRange = KeyRange.UNBOUND;
+                
+                switch(op) {
+                    case EQUAL:
+                        return equalityRange;
+                    case GREATER:
+                        // from the equality range and up, NOT including the equality range
+                        lowerRange = equalityRange.getUpperRange();
+                        lowerInclusive = !equalityRange.isUpperInclusive();
+                        break;
+                    case GREATER_OR_EQUAL:
+                        // from the equality range and up, including the equality range
+                        lowerRange = equalityRange.getLowerRange();
+                        break;
+                    case LESS:
+                        // from the equality range and down, NOT including the equality range
+                        upperRange = equalityRange.getLowerRange();
+                        upperInclusive = !equalityRange.isLowerInclusive();
+                        break;
+                    case LESS_OR_EQUAL:
+                        // from the equality range and down, including the equality range
+                        upperRange = equalityRange.getUpperRange();
+                        break;
+                    default:
+                        throw new AssertionError("Invalid CompareOp: " + op);
+                }
+
+                return KeyRange.getKeyRange(lowerRange, lowerInclusive, upperRange, upperInclusive);
+            }
+            
+            /**
+             * Produces a the given decimal rounded to this rounding expression's scale. If the 
+             * decimal requires more scale precision to produce than this expression has, as in
+             * ROUND(?, 2) &gt; 2.0098974, it ensures that the decimal is rounded such that the
+             * given operator will still produce correct results.
+             * @param decimal  the decimal to round with this expression's scale
+             * @param op  the operator to preserve comparison with in the event of lost precision
+             * @return  the rounded decimal
+             */
+            private BigDecimal roundAndPreserveOperator(BigDecimal decimal, CompareFilter.CompareOp op) {
+                final BigDecimal rounded = roundToScale(decimal);
+                
+                // if we lost information, make sure that the rounding didn't break the operator
+                if(!hasEnoughPrecisionToProduce(decimal)) {
+                    switch(op) {
+                        case GREATER_OR_EQUAL:
+                            // e.g. 'ROUND(dec, 2) >= 2.013' would be converted to 
+                            // 'ROUND(dec, 2) >= 2.01' but should be 'ROUND(dec, 2) >= 2.02'
+                            if(decimal.compareTo(rounded) > 0) {
+                                return stepNextInScale(rounded);
+                            }
+                            break;
+                        case GREATER:
+                            // e.g. 'ROUND(dec, 2) > 2.017' would be converted to 
+                            // 'ROUND(dec, 2) > 2.02' but should be 'ROUND(dec, 2) > 2.01'
+                            if(decimal.compareTo(rounded) < 0) {
+                                return stepPrevInScale(rounded);
+                            }
+                            break;
+                        case LESS_OR_EQUAL:
+                            // e.g. 'ROUND(dec, 2) < 2.017' would be converted to 
+                            // 'ROUND(dec, 2) < 2.02' but should be 'ROUND(dec, 2) < 2.01'
+                            if(decimal.compareTo(rounded) < 0) {
+                                return stepPrevInScale(rounded);
+                            }
+                            break;
+                        case LESS:
+                            // e.g. 'ROUND(dec, 2) <= 2.013' would be converted to 
+                            // 'ROUND(dec, 2) <= 2.01' but should be 'ROUND(dec, 2) <= 2.02'
+                            if(decimal.compareTo(rounded) > 0) {
+                                return stepNextInScale(rounded);
+                            }
+                            break;
+                    }
+                }
+                
+                // otherwise, rounding has not affected the operator, so return normally
+                return rounded;
+            }
+        };
+    }
+    
+    /**
+     * Finds the Decimal KeyRange that will produce the given result when fed into this 
+     * rounding expression. For example, a ROUND expression with scale 2 will produce the
+     * result "2.05" with any decimal in the range [2.045, 2.0545). 
+     * The result must be pre-rounded to within this rounding expression's scale. 
+     * @param result  the result to find an input range for. Must be producable.
+     * @return  a KeyRange of DECIMAL keys that can be rounded by this expression to produce result
+     * @throws IllegalArgumentException  if the result has more scale than this expression can produce
+     */
+    protected KeyRange getInputRangeProducing(BigDecimal result) {
+        if(!hasEnoughPrecisionToProduce(result)) {
+            throw new IllegalArgumentException("Cannot produce input range for decimal " + result 
+                + ", not enough precision with scale " + getRoundingScale());
+        }
+        byte[] lowerRange = DECIMAL.toBytes(halfStepPrevInScale(result));
+        byte[] upperRange = DECIMAL.toBytes(halfStepNextInScale(result));
+        // inclusiveness changes depending on sign
+        // e.g. -0.5 rounds "up" to -1 even though it is the lower boundary
+        boolean lowerInclusive = result.signum() > 0;
+        boolean upperInclusive = result.signum() < 0;
+        return KeyRange.getKeyRange(lowerRange, lowerInclusive, upperRange, upperInclusive);
+    }
+    
+    /**
+     * Determines whether this rounding expression's scale has enough precision to produce the
+     * minimum precision for the input decimal. In other words, determines whether the given
+     * decimal can be rounded to this scale without losing ordering information.
+     * For example, an expression with a scale of 2 has enough precision to produce "2.3", "2.71"
+     * and "2.100000", but does not have enough precision to produce "2.001"
+     * @param result  the decimal to round
+     * @return  true if the given decimal can be precisely matched by this rounding expression
+     */
+    protected final boolean hasEnoughPrecisionToProduce(BigDecimal result) {
+        // use compareTo so that 2.0 and 2.00 are treated as "equal"
+        return roundToScale(result).compareTo(result) == 0;
+    }
+    
+    /**
+     * Returns the given decimal rounded to this rounding expression's scale.
+     * For example, with scale 2 the decimal "2.453" would be rounded to either 2.45 or 
+     * 2.46 depending on the rounding mode, while "2.38" and "2.7" would be unchanged.  
+     * @param decimal  the decimal to round
+     * @return  the rounded result decimal
+     */
+    protected final BigDecimal roundToScale(BigDecimal decimal) {
+        return decimal.setScale(getRoundingScale(), getRoundingMode());
+    }
+    
+    /**
+     * Produces a value half of a "step" back in this expression's rounding scale.
+     * For example with a scale of 2, "2.5" would be stepped back to "2.495".
+     */
+    protected final BigDecimal halfStepPrevInScale(BigDecimal decimal) {
+        BigDecimal step = BigDecimal.ONE.scaleByPowerOfTen(-getRoundingScale());
+        BigDecimal halfStep = step.divide(BigDecimal.valueOf(2));
+        return decimal.subtract(halfStep);
+    }
+    
+    /**
+     * Produces a value half of a "step" forward in this expression's rounding scale.
+     * For example with a scale of 2, "2.5" would be stepped forward to "2.505".
+     */
+    protected final BigDecimal halfStepNextInScale(BigDecimal decimal) {
+        BigDecimal step = BigDecimal.ONE.scaleByPowerOfTen(-getRoundingScale());
+        BigDecimal halfStep = step.divide(BigDecimal.valueOf(2));
+        return decimal.add(halfStep);
+    }
+
+    /**
+     * Produces a value one "step" back in this expression's rounding scale.
+     * For example with a scale of 2, "2.5" would be stepped back to "2.49".
+     */
+    protected final BigDecimal stepPrevInScale(BigDecimal decimal) {
+        BigDecimal step = BigDecimal.ONE.scaleByPowerOfTen(-getRoundingScale());
+        return decimal.subtract(step);
+    }
+
+    /**
+     * Produces a value one "step" forward in this expression's rounding scale.
+     * For example with a scale of 2, "2.5" would be stepped forward to "2.51".
+     */
+    protected final BigDecimal stepNextInScale(BigDecimal decimal) {
+        BigDecimal step = BigDecimal.ONE.scaleByPowerOfTen(-getRoundingScale());
+        return decimal.add(step);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c5853228/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java
index 55bcd7f..25520c4 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java
@@ -17,26 +17,36 @@
  */
 package org.apache.phoenix.expression;
 
+import java.math.BigDecimal;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-import java.math.BigDecimal;
 import java.sql.Date;
+import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
+import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp;
 
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.phoenix.compile.KeyPart;
 import org.apache.phoenix.expression.function.CeilDateExpression;
 import org.apache.phoenix.expression.function.CeilDecimalExpression;
 import org.apache.phoenix.expression.function.FloorDateExpression;
 import org.apache.phoenix.expression.function.FloorDecimalExpression;
 import org.apache.phoenix.expression.function.RoundDateExpression;
 import org.apache.phoenix.expression.function.RoundDecimalExpression;
+import org.apache.phoenix.expression.function.ScalarFunction;
 import org.apache.phoenix.expression.function.TimeUnit;
+import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr;
+import org.apache.phoenix.query.KeyRange;
 import org.apache.phoenix.schema.IllegalDataException;
 import org.apache.phoenix.schema.PDataType;
+import static org.apache.phoenix.schema.PDataType.DECIMAL;
 import org.apache.phoenix.util.DateUtil;
+import static org.junit.Assert.assertFalse;
 import org.junit.Test;
 
 /**
@@ -49,148 +59,520 @@ import org.junit.Test;
  */
 public class RoundFloorCeilExpressionsTest {
 
+    // Decimal Expression Tests
+    
     @Test
     public void testRoundDecimalExpression() throws Exception {
-        LiteralExpression bd = LiteralExpression.newConstant(1.23898, PDataType.DECIMAL);
-        Expression rde = RoundDecimalExpression.create(bd, 3);
+        LiteralExpression decimalLiteral = LiteralExpression.newConstant(1.23898, PDataType.DECIMAL);
+        Expression roundDecimalExpression = RoundDecimalExpression.create(decimalLiteral, 3);
+        
         ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof BigDecimal);
-        BigDecimal value = (BigDecimal)obj;
-        assertEquals(BigDecimal.valueOf(1.239), value);
+        roundDecimalExpression.evaluate(null, ptr);
+        Object result = roundDecimalExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof BigDecimal);
+        BigDecimal resultDecimal = (BigDecimal)result;
+        assertEquals(BigDecimal.valueOf(1.239), resultDecimal);
     }
     
     @Test
-    public void testCeilDecimalExpression() throws Exception {
-        LiteralExpression bd = LiteralExpression.newConstant(1.23898, PDataType.DECIMAL);
-        Expression rde = CeilDecimalExpression.create(bd, 3);
-        ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof BigDecimal);
-        BigDecimal value = (BigDecimal)obj;
-        assertEquals(BigDecimal.valueOf(1.239), value);
+    public void testRoundDecimalExpressionNoop() throws Exception {
+        LiteralExpression decimalLiteral = LiteralExpression.newConstant(5, PDataType.INTEGER);
+        Expression roundDecimalExpression = RoundDecimalExpression.create(decimalLiteral, 3);
+        
+        assertEquals(roundDecimalExpression, decimalLiteral);
     }
     
     @Test
-    public void testCeilDecimalExpressionNoop() throws Exception {
-        LiteralExpression bd = LiteralExpression.newConstant(5, PDataType.INTEGER);
-        Expression rde = CeilDecimalExpression.create(bd, 3);
-        assertEquals(rde, bd);
+    public void testFloorDecimalExpression() throws Exception {
+        LiteralExpression decimalLiteral = LiteralExpression.newConstant(1.23898, PDataType.DECIMAL);
+        Expression floorDecimalExpression = FloorDecimalExpression.create(decimalLiteral, 3);
+        
+        ImmutableBytesWritable ptr = new ImmutableBytesWritable();
+        floorDecimalExpression.evaluate(null, ptr);
+        Object result = floorDecimalExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof BigDecimal);
+        BigDecimal resultDecimal = (BigDecimal)result;
+        assertEquals(BigDecimal.valueOf(1.238), resultDecimal);
     }
     
     @Test
     public void testFloorDecimalExpressionNoop() throws Exception {
-        LiteralExpression bd = LiteralExpression.newConstant(5, PDataType.INTEGER);
-        Expression rde = FloorDecimalExpression.create(bd, 3);
-        assertEquals(rde, bd);
+        LiteralExpression decimalLiteral = LiteralExpression.newConstant(5, PDataType.INTEGER);
+        Expression floorDecimalExpression = FloorDecimalExpression.create(decimalLiteral, 3);
+        
+        assertEquals(floorDecimalExpression, decimalLiteral);
     }
     
     @Test
-    public void testRoundDecimalExpressionNoop() throws Exception {
-        LiteralExpression bd = LiteralExpression.newConstant(5, PDataType.INTEGER);
-        Expression rde = RoundDecimalExpression.create(bd, 3);
-        assertEquals(rde, bd);
+    public void testCeilDecimalExpression() throws Exception {
+        LiteralExpression decimalLiteral = LiteralExpression.newConstant(1.23898, PDataType.DECIMAL);
+        Expression ceilDecimalExpression = CeilDecimalExpression.create(decimalLiteral, 3);
+        
+        ImmutableBytesWritable ptr = new ImmutableBytesWritable();
+        ceilDecimalExpression.evaluate(null, ptr);
+        Object result = ceilDecimalExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof BigDecimal);
+        BigDecimal resultDecimal = (BigDecimal)result;
+        assertEquals(BigDecimal.valueOf(1.239), resultDecimal);
     }
     
     @Test
-    public void testFloorDecimalExpression() throws Exception {
-        LiteralExpression bd = LiteralExpression.newConstant(1.23898, PDataType.DECIMAL);
-        Expression rde = FloorDecimalExpression.create(bd, 3);
-        ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof BigDecimal);
-        BigDecimal value = (BigDecimal)obj;
-        assertEquals(BigDecimal.valueOf(1.238), value);
+    public void testCeilDecimalExpressionNoop() throws Exception {
+        LiteralExpression decimalLiteral = LiteralExpression.newConstant(5, PDataType.INTEGER);
+        Expression ceilDecimalExpression = CeilDecimalExpression.create(decimalLiteral, 3);
+        
+        assertEquals(ceilDecimalExpression, decimalLiteral);
     }
     
     @Test
     public void testRoundDecimalExpressionScaleParamValidation() throws Exception {
-        LiteralExpression bd = LiteralExpression.newConstant(1.23898, PDataType.DECIMAL);
+        LiteralExpression decimalLiteral = LiteralExpression.newConstant(1.23898, PDataType.DECIMAL);
         LiteralExpression scale = LiteralExpression.newConstant("3", PDataType.VARCHAR);
-        List<Expression> exprs = new ArrayList<Expression>(2);
-        exprs.add(bd);
-        exprs.add(scale);
+        
+        List<Expression> childExpressions = new ArrayList<Expression>(2);
+        childExpressions.add(decimalLiteral);
+        childExpressions.add(scale);
+        
         try {
-            RoundDecimalExpression.create(exprs);
+            RoundDecimalExpression.create(childExpressions);
             fail("Evaluation should have failed because only an INTEGER is allowed for second param in a RoundDecimalExpression");
         } catch(IllegalDataException e) {
 
         }
     }
     
+    // KeyRange explicit simple / sanity tests
+    
+    @Test
+    public void testRoundDecimalExpressionKeyRangeSimple() throws Exception {
+        ScalarFunction roundDecimalExpression = (ScalarFunction)RoundDecimalExpression.create(DUMMY_DECIMAL, 3);
+        
+        byte[] upperBound = DECIMAL.toBytes(new BigDecimal("1.2385"));
+        byte[] lowerBound = DECIMAL.toBytes(new BigDecimal("1.2375"));
+        KeyRange expectedKeyRange = KeyRange.getKeyRange(lowerBound, upperBound);
+        
+        KeyPart keyPart = roundDecimalExpression.newKeyPart(null);
+        assertEquals(expectedKeyRange, keyPart.getKeyRange(CompareOp.EQUAL, LiteralExpression.newConstant(new BigDecimal("1.238"), DECIMAL)));
+    }
+    
+    @Test
+    public void testFloorDecimalExpressionKeyRangeSimple() throws Exception {
+        ScalarFunction floorDecimalExpression = (ScalarFunction)FloorDecimalExpression.create(DUMMY_DECIMAL, 3);
+        
+        byte[] upperBound = DECIMAL.toBytes(new BigDecimal("1.239"));
+        byte[] lowerBound = DECIMAL.toBytes(new BigDecimal("1.238"));
+        KeyRange expectedKeyRange = KeyRange.getKeyRange(lowerBound, true, upperBound, false);
+        
+        KeyPart keyPart = floorDecimalExpression.newKeyPart(null);
+        assertEquals(expectedKeyRange, keyPart.getKeyRange(CompareOp.EQUAL, LiteralExpression.newConstant(new BigDecimal("1.238"), DECIMAL)));
+    }
+    
+    @Test
+    public void testCeilDecimalExpressionKeyRangeSimple() throws Exception {
+        ScalarFunction ceilDecimalExpression = (ScalarFunction)CeilDecimalExpression.create(DUMMY_DECIMAL, 3);
+        
+        byte[] upperBound = DECIMAL.toBytes(new BigDecimal("1.238"));
+        byte[] lowerBound = DECIMAL.toBytes(new BigDecimal("1.237"));
+        KeyRange expectedKeyRange = KeyRange.getKeyRange(lowerBound, false, upperBound, true);
+        
+        KeyPart keyPart = ceilDecimalExpression.newKeyPart(null);
+        assertEquals(expectedKeyRange, keyPart.getKeyRange(CompareOp.EQUAL, LiteralExpression.newConstant(new BigDecimal("1.238"), DECIMAL)));
+    }
+    
+    // KeyRange complex / generated tests
+    
+    @Test
+    public void testRoundDecimalExpressionKeyRangeCoverage() throws Exception {
+        for(int scale : SCALES) {
+            ScalarFunction roundDecimalExpression = (ScalarFunction) RoundDecimalExpression.create(DUMMY_DECIMAL, scale);
+            KeyPart keyPart = roundDecimalExpression.newKeyPart(null);
+            verifyKeyPart(RoundingType.ROUND, scale, keyPart);
+        }
+    }
+    
+    @Test
+    public void testFloorDecimalExpressionKeyRangeCoverage() throws Exception {
+        for(int scale : SCALES) {
+            ScalarFunction floorDecimalExpression = (ScalarFunction) FloorDecimalExpression.create(DUMMY_DECIMAL, scale);
+            KeyPart keyPart = floorDecimalExpression.newKeyPart(null);
+            verifyKeyPart(RoundingType.FLOOR, scale, keyPart);
+        }
+    }
+    
+    @Test
+    public void testCeilDecimalExpressionKeyRangeCoverage() throws Exception {
+        for(int scale : SCALES) {
+            ScalarFunction ceilDecimalExpression = (ScalarFunction) CeilDecimalExpression.create(DUMMY_DECIMAL, scale);
+            KeyPart keyPart = ceilDecimalExpression.newKeyPart(null);
+            verifyKeyPart(RoundingType.CEIL, scale, keyPart);
+        }
+    }
+    
+    /**
+     * Represents the three different types of rounding expression and produces
+     * expressions of their type when given a Decimal key and scale.
+     */
+    private static enum RoundingType {
+        ROUND("ROUND"), 
+        FLOOR("FLOOR"), 
+        CEIL("CEIL");
+        
+        public final String name;
+        
+        RoundingType(String name) {
+            this.name = name;
+        }
+        
+        /**
+         * Returns a rounding expression of this type that will round the given decimal key at the
+         * given scale. 
+         * @param key  the byte key for the Decimal to round
+         * @param scale  the scale to round the decimal to
+         * @return  the expression containing the above parameters
+         */
+        public Expression getExpression(byte[] key, int scale) {
+            try {
+                LiteralExpression decimalLiteral = LiteralExpression.newConstant(DECIMAL.toObject(key), DECIMAL);
+                switch(this) {
+                    case ROUND:
+                        return RoundDecimalExpression.create(decimalLiteral, scale);
+                    case FLOOR:
+                        return FloorDecimalExpression.create(decimalLiteral, scale);
+                    case CEIL:
+                        return CeilDecimalExpression.create(decimalLiteral, scale);
+                    default:
+                        throw new AssertionError("Unknown RoundingType");
+                }
+            }
+            catch (SQLException ex) {
+                throw new AssertionError("Should never happen when creating decimal literal", ex);
+            }
+        }
+    }
+    
+    /**
+     * Represents a possible relational operator used in rounding expression where clauses.
+     * Includes information not kept by CompareFilter.CompareOp, including a string symbol
+     * representation and a method for actually comparing comparables.
+     */
+    private static enum Relation {
+        EQUAL(CompareOp.EQUAL, "="),
+        GREATER(CompareOp.GREATER, ">"),
+        GREATER_OR_EQUAL(CompareOp.GREATER_OR_EQUAL, ">="),
+        LESS(CompareOp.LESS, "<"),
+        LESS_OR_EQUAL(CompareOp.LESS_OR_EQUAL, "<=");
+        
+        public final CompareOp compareOp;
+        public final String symbol;
+        
+        Relation(CompareOp compareOp, String symbol) {
+            this.compareOp = compareOp;
+            this.symbol = symbol;
+        }
+        
+        public <E extends Comparable<? super E>> boolean compare(E lhs, E rhs) {
+            int comparison = lhs.compareTo(rhs);
+            switch(this) {
+                case EQUAL:
+                    return comparison == 0;
+                case GREATER_OR_EQUAL:
+                    return comparison >= 0;
+                case GREATER:
+                    return comparison > 0;
+                case LESS_OR_EQUAL:
+                    return comparison <= 0;
+                case LESS:
+                    return comparison < 0;
+                default:
+                    throw new AssertionError("Unknown RelationType");
+            }
+        }
+    }
+    
+    /**
+     * Produces a string error message containing the given information, formatted like a where
+     * clause. <br>
+     * Example Output: <br>
+     * 'where ROUND(?, 2) &lt;= 2.55' (produced range: [2.545, 2.555) )
+     * @param exprType
+     * @param scale
+     * @param relation
+     * @param rhs
+     * @param range
+     * @return 
+     */
+    private static String getMessage(RoundingType exprType, int scale, Relation relation, BigDecimal rhs, KeyRange range) {
+        String where = exprType.name + "(?, " + scale + ") " + relation.symbol + " " + rhs;
+        return "'where " + where + "' (produced range: " + formatDecimalKeyRange(range) + " )";
+    }
+    
+    /**
+     * Interpreting the KeyRange as a range of decimal, produces a nicely formatted string
+     * representation. 
+     * @param range  the KeyRange to format
+     * @return  the string representation, e.g. [2.45, 2.55)
+     */
+    private static String formatDecimalKeyRange(KeyRange range) {
+        return (range.isLowerInclusive() ? "[" : "(") 
+            + (range.lowerUnbound() ? "*" : DECIMAL.toObject(range.getLowerRange())) 
+            + ", " 
+            + (range.upperUnbound() ? "*" : DECIMAL.toObject(range.getUpperRange())) 
+            + (range.isUpperInclusive() ? "]" : ")");
+    }
+    
+    // create methods need a dummy expression that is not coercible to to a long
+    // value doesn't matter because we only use those expressions to produce a keypart
+    private static final LiteralExpression DUMMY_DECIMAL = LiteralExpression.newConstant(new BigDecimal("2.5"));
+    
+    // this should be PDataType#MAX_PRECISION (38)
+    // but there are rounding errors in DECIMAL.toBytes() and DECIMAL.toObject()
+    // with precisions of 20 or greater. See https://issues.apache.org/jira/browse/PHOENIX-1206
+    private static final int MAX_RELIABLE_PRECISION = 18;
+    
+    // once PHOENIX-1206 is fixed, we should add more precise decimals to these tests
+    private static final List<BigDecimal> DECIMALS = Collections.unmodifiableList(
+        Arrays.asList(
+            new BigDecimal("-200300"), new BigDecimal("-8.44"), new BigDecimal("-2.00"), 
+            new BigDecimal("-0.6"), new BigDecimal("-0.00032"), 
+            BigDecimal.ZERO, BigDecimal.ONE, 
+            new BigDecimal("0.00000984"), new BigDecimal("0.74"), new BigDecimal("2.00"), 
+            new BigDecimal("7.09"), new BigDecimal("84900800")
+        ));
+    
+    private static final List<Integer> SCALES = Collections.unmodifiableList(Arrays.asList(0, 1, 2, 3, 8));
+    
+    /**
+     * Checks that a given KeyPart produces the right key ranges for each relational operator and
+     * a variety of right-hand-side decimals.
+     * @param exprType  the rounding expression type used to create this KeyPart
+     * @param scale  the scale used to create this KeyPart
+     * @param keyPart  the KeyPart to test
+     */
+    private void verifyKeyPart(RoundingType exprType, int scale, KeyPart keyPart) throws SQLException {
+        for(BigDecimal rhsDecimal : DECIMALS) {
+            LiteralExpression rhsExpression = LiteralExpression.newConstant(rhsDecimal, DECIMAL);
+            for(Relation relation : Relation.values()) {
+                KeyRange keyRange = keyPart.getKeyRange(relation.compareOp, rhsExpression);
+                verifyKeyRange(exprType, scale, relation, rhsDecimal, keyRange);
+            }
+        }
+    }
+    
+    /**
+     * Checks that a given KeyRange's boundaries match with the given rounding expression type,
+     * rounding scale, relational operator, and right hand side decimal.
+     * Does so by checking the decimal values immediately on either side of the KeyRange border and
+     * verifying that they either match or do not match the "where clause" formed by the 
+     * rounding type, scale, relation, and rhs decimal. If a relation should produce an unbounded
+     * upper or lower range, verifies that that end of the range is unbounded. Finally, if the
+     * range is empty, verifies that the rhs decimal required more precision than could be
+     * produced by the rounding expression.
+     * @param exprType  the rounding expression type used to create this KeyRange
+     * @param scale  the rounding scale used to create this KeyRange
+     * @param relation  the relational operator used to create this KeyRange
+     * @param rhs  the right hand side decimal used to create this KeyRange
+     * @param range  the KeyRange to test
+     */
+    private void verifyKeyRange(RoundingType exprType, int scale, Relation relation, BigDecimal rhs, KeyRange range) throws SQLException {
+        // dump of values for debugging
+        final String dump = getMessage(exprType, scale, relation, rhs, range);
+        
+        ImmutableBytesPtr rhsPtr = new ImmutableBytesPtr();
+        LiteralExpression.newConstant(rhs, DECIMAL).evaluate(null, rhsPtr);
+        
+        ImmutableBytesPtr lhsPtr = new ImmutableBytesPtr();
+        
+        // we should only get an empty range if we can verify that precision makes a match impossible
+        if(range == KeyRange.EMPTY_RANGE) {
+            assertTrue("should only get empty key range for unmatchable rhs precision (" + dump + ")", rhs.scale() > scale);
+            assertEquals("should only get empty key range for equals checks (" + dump + ")", Relation.EQUAL, relation);
+            return;
+        }
+        
+        // if it should have an upper bound
+        if(relation != Relation.GREATER && relation != Relation.GREATER_OR_EQUAL) {
+            // figure out what the upper bound is
+            byte[] highestHighIncluded;
+            byte[] lowestHighExcluded;
+            if(range.isUpperInclusive()) {
+                highestHighIncluded = range.getUpperRange();
+                lowestHighExcluded = nextDecimalKey(range.getUpperRange());
+            }
+            else {
+                highestHighIncluded = prevDecimalKey(range.getUpperRange());
+                lowestHighExcluded = range.getUpperRange();
+            }
+            
+            // check on either side of the boundary to validate that it is in fact the boundary
+            exprType.getExpression(highestHighIncluded, scale).evaluate(null, lhsPtr);
+            assertTrue("incorrectly excluding " + DECIMAL.toObject(highestHighIncluded) 
+                + " in upper bound for " + dump, relation.compare(lhsPtr, rhsPtr));
+            exprType.getExpression(lowestHighExcluded, scale).evaluate(null, lhsPtr);
+            assertFalse("incorrectly including " + DECIMAL.toObject(lowestHighExcluded) 
+                + " in upper bound for " + dump, relation.compare(lhsPtr, rhsPtr));
+        }
+        else {
+            // otherwise verify that it does not have an upper bound
+            assertTrue("should not have a upper bound for " + dump, range.upperUnbound());
+        }
+        
+        // if it should have a lower bound
+        if(relation != Relation.LESS && relation != Relation.LESS_OR_EQUAL) {
+            // figure out what the lower bound is
+            byte[] lowestLowIncluded;
+            byte[] highestLowExcluded;
+            if(range.isLowerInclusive()) {
+                lowestLowIncluded = range.getLowerRange();
+                highestLowExcluded = prevDecimalKey(range.getLowerRange());
+            }
+            else {
+                lowestLowIncluded = nextDecimalKey(range.getLowerRange());
+                highestLowExcluded = range.getLowerRange();
+            }
+            
+            // check on either side of the boundary to validate that it is in fact the boundary
+            exprType.getExpression(lowestLowIncluded, scale).evaluate(null, lhsPtr);
+            assertTrue("incorrectly excluding " + DECIMAL.toObject(lowestLowIncluded) 
+                + " in lower bound for " + dump, relation.compare(lhsPtr, rhsPtr));
+            exprType.getExpression(highestLowExcluded, scale).evaluate(null, lhsPtr);
+            assertFalse("incorrectly including " + DECIMAL.toObject(highestLowExcluded) 
+                + " in lower bound for " + dump, relation.compare(lhsPtr, rhsPtr));
+        }
+        else {
+            // otherwise verify that it does not have a lower bound
+            assertTrue("should not have a lower bound for " + dump, range.lowerUnbound());
+        }
+    }
+    
+    /**
+     * Produces the previous Decimal key relative to the given key. The new key will differ from
+     * the old key in as small a unit as possible while still maintaining accurate serialization.
+     * @param key  bytes for the old Decimal key
+     * @return  bytes for the new Decimal key, a single unit previous to the old one
+     */
+    private static byte[] prevDecimalKey(byte[] key) {
+        BigDecimal decimal = (BigDecimal) DECIMAL.toObject(key);
+        BigDecimal prev = decimal.subtract(getSmallestUnit(decimal));
+        return DECIMAL.toBytes(prev);
+    }
+    
+    /**
+     * Produces the next Decimal key relative to the given key. The new key will differ from the
+     * old key in as small a unit as possible while still maintaining accurate serialization.
+     * @param key  bytes for the old Decimal key
+     * @return  bytes for the new Decimal key, a single unit next from the old one
+     */
+    private static byte[] nextDecimalKey(byte[] key) {
+        BigDecimal decimal = (BigDecimal) DECIMAL.toObject(key);
+        BigDecimal next = decimal.add(getSmallestUnit(decimal));
+        return DECIMAL.toBytes(next);
+    }
+    
+    /**
+     * Produces the smallest unit of difference possible for the given decimal that will still
+     * be serialized accurately. For example, if the MAXIMUM_RELIABLE_PRECISION were 4, then 
+     * getSmallestUnit(2.3) would produce 0.001, as 2.301 could be serialized accurately but
+     * 2.3001 could not.
+     * @param decimal  the decimal to find the smallest unit in relation to
+     * @return  the smallest BigDecimal unit possible to add to decimal while still maintaining
+     *          accurate serialization
+     * @throws IllegalArgumentException if decimal requires more than the maximum reliable precision
+     */
+    private static BigDecimal getSmallestUnit(BigDecimal decimal) {
+        if (decimal.precision() > MAX_RELIABLE_PRECISION) {
+            throw new IllegalArgumentException("rounding errors mean that we cannot reliably test " + decimal);
+        }
+        int minScale = decimal.scale() + (MAX_RELIABLE_PRECISION - decimal.precision());
+        return BigDecimal.valueOf(1, minScale);
+    }
+    
+    // Date Expression Tests
+    
     @Test
     public void testRoundDateExpression() throws Exception {
-        LiteralExpression date = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
-        Expression rde = RoundDateExpression.create(date, TimeUnit.DAY);
+        LiteralExpression dateLiteral = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
+        Expression roundDateExpression = RoundDateExpression.create(dateLiteral, TimeUnit.DAY);
+        
         ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof Date);
-        Date value = (Date)obj;
-        assertEquals(DateUtil.parseDate("2012-01-02 00:00:00"), value);
+        roundDateExpression.evaluate(null, ptr);
+        Object result = roundDateExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof Date);
+        Date resultDate = (Date)result;
+        assertEquals(DateUtil.parseDate("2012-01-02 00:00:00"), resultDate);
     }
     
     @Test
     public void testRoundDateExpressionWithMultiplier() throws Exception {
-        Expression date = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
-        Expression rde = RoundDateExpression.create(date, TimeUnit.MINUTE, 10);
+        Expression dateLiteral = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
+        Expression roundDateExpression = RoundDateExpression.create(dateLiteral, TimeUnit.MINUTE, 10);
+        
         ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof Date);
-        Date value = (Date)obj;
-        assertEquals(DateUtil.parseDate("2012-01-01 14:30:00"), value);
+        roundDateExpression.evaluate(null, ptr);
+        Object result = roundDateExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof Date);
+        Date resultDate = (Date)result;
+        assertEquals(DateUtil.parseDate("2012-01-01 14:30:00"), resultDate);
     }
     
     @Test
-    public void testCeilDateExpression() throws Exception {
-        LiteralExpression date = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
-        Expression rde = CeilDateExpression.create(date, TimeUnit.DAY);
+    public void testFloorDateExpression() throws Exception {
+        LiteralExpression dateLiteral = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
+        Expression floorDateExpression = FloorDateExpression.create(dateLiteral, TimeUnit.DAY);
+        
         ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof Date);
-        Date value = (Date)obj;
-        assertEquals(DateUtil.parseDate("2012-01-02 00:00:00"), value);
+        floorDateExpression.evaluate(null, ptr);
+        Object result = floorDateExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof Date);
+        Date resultDate = (Date)result;
+        assertEquals(DateUtil.parseDate("2012-01-01 00:00:00"), resultDate);
     }
     
     @Test
-    public void testCeilDateExpressionWithMultiplier() throws Exception {
-        Expression date = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
-        Expression rde = CeilDateExpression.create(date, TimeUnit.SECOND, 10);
+    public void testFloorDateExpressionWithMultiplier() throws Exception {
+        Expression dateLiteral = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
+        Expression floorDateExpression = FloorDateExpression.create(dateLiteral, TimeUnit.SECOND, 10);
+        
         ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof Date);
-        Date value = (Date)obj;
-        assertEquals(DateUtil.parseDate("2012-01-01 14:25:30"), value);
+        floorDateExpression.evaluate(null, ptr);
+        Object result = floorDateExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof Date);
+        Date resultDate = (Date)result;
+        assertEquals(DateUtil.parseDate("2012-01-01 14:25:20"), resultDate);
     }
     
     @Test
-    public void testFloorDateExpression() throws Exception {
-        LiteralExpression date = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
-        Expression rde = FloorDateExpression.create(date, TimeUnit.DAY);
+    public void testCeilDateExpression() throws Exception {
+        LiteralExpression dateLiteral = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
+        Expression ceilDateExpression = CeilDateExpression.create(dateLiteral, TimeUnit.DAY);
+        
         ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof Date);
-        Date value = (Date)obj;
-        assertEquals(DateUtil.parseDate("2012-01-01 00:00:00"), value);
+        ceilDateExpression.evaluate(null, ptr);
+        Object result = ceilDateExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof Date);
+        Date resultDate = (Date)result;
+        assertEquals(DateUtil.parseDate("2012-01-02 00:00:00"), resultDate);
     }
     
     @Test
-    public void testFloorDateExpressionWithMultiplier() throws Exception {
-        Expression date = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
-        Expression rde = FloorDateExpression.create(date, TimeUnit.SECOND, 10);
+    public void testCeilDateExpressionWithMultiplier() throws Exception {
+        Expression dateLiteral = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
+        Expression ceilDateExpression = CeilDateExpression.create(dateLiteral, TimeUnit.SECOND, 10);
+        
         ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        rde.evaluate(null, ptr);
-        Object obj = rde.getDataType().toObject(ptr);
-        assertTrue(obj instanceof Date);
-        Date value = (Date)obj;
-        assertEquals(DateUtil.parseDate("2012-01-01 14:25:20"), value);
+        ceilDateExpression.evaluate(null, ptr);
+        Object result = ceilDateExpression.getDataType().toObject(ptr);
+        
+        assertTrue(result instanceof Date);
+        Date resultDate = (Date)result;
+        assertEquals(DateUtil.parseDate("2012-01-01 14:25:30"), resultDate);
     }
     
     /**
@@ -198,11 +580,13 @@ public class RoundFloorCeilExpressionsTest {
      */
     @Test
     public void testRoundDateExpressionValidation_1() throws Exception {
-        LiteralExpression date = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
-        List<Expression> exprs = new ArrayList<Expression>(1);
-        exprs.add(date);
+        LiteralExpression dateLiteral = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
+        
+        List<Expression> childExpressions = new ArrayList<Expression>(1);
+        childExpressions.add(dateLiteral);
+        
         try {
-            RoundDateExpression.create(exprs);
+            RoundDateExpression.create(childExpressions);
             fail("Instantiating a RoundDateExpression with only one argument should have failed.");
         } catch(IllegalArgumentException e) {
 
@@ -214,13 +598,15 @@ public class RoundFloorCeilExpressionsTest {
      */
     @Test
     public void testRoundDateExpressionValidation_2() throws Exception {
-        LiteralExpression date = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
-        LiteralExpression timeUnit = LiteralExpression.newConstant("millis", PDataType.VARCHAR);
-        List<Expression> exprs = new ArrayList<Expression>(1);
-        exprs.add(date);
-        exprs.add(timeUnit);
+        LiteralExpression dateLiteral = LiteralExpression.newConstant(DateUtil.parseDate("2012-01-01 14:25:28"), PDataType.DATE);
+        LiteralExpression timeUnitLiteral = LiteralExpression.newConstant("millis", PDataType.VARCHAR);
+        
+        List<Expression> childExpressions = new ArrayList<Expression>(1);
+        childExpressions.add(dateLiteral);
+        childExpressions.add(timeUnitLiteral);
+        
         try {
-            RoundDateExpression.create(exprs);
+            RoundDateExpression.create(childExpressions);
             fail("Only a valid time unit represented by TimeUnit enum is allowed and millis is invalid.");
         } catch(IllegalArgumentException e) {
 


Mime
View raw message