You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jc...@apache.org on 2018/05/29 22:41:52 UTC
calcite git commit: [CALCITE-2332] Wrong simplification of
FLOOR(CEIL(x)) to FLOOR(x)
Repository: calcite
Updated Branches:
refs/heads/master 27a190ff3 -> 76fd6c41d
[CALCITE-2332] Wrong simplification of FLOOR(CEIL(x)) to FLOOR(x)
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/76fd6c41
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/76fd6c41
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/76fd6c41
Branch: refs/heads/master
Commit: 76fd6c41d18176258fc9205e4847c3490e1746ba
Parents: 27a190f
Author: Jesus Camacho Rodriguez <jc...@apache.org>
Authored: Tue May 29 15:23:42 2018 -0700
Committer: Jesus Camacho Rodriguez <jc...@apache.org>
Committed: Tue May 29 15:23:42 2018 -0700
----------------------------------------------------------------------
.../org/apache/calcite/rex/RexSimplify.java | 18 +++++++--------
.../calcite/test/RexImplicationCheckerTest.java | 23 ++++++++------------
2 files changed, 17 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/76fd6c41/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
index d168402..9b0df88 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -174,9 +174,8 @@ public class RexSimplify {
return simplifyCoalesce((RexCall) e);
case CAST:
return simplifyCast((RexCall) e);
- case CEIL:
case FLOOR:
- return simplifyCeilFloor((RexCall) e);
+ return simplifyFloor((RexCall) e);
case IS_NULL:
case IS_NOT_NULL:
case IS_TRUE:
@@ -1160,32 +1159,31 @@ public class RexSimplify {
}
}
- /** Tries to simplify CEIL/FLOOR function on top of CEIL/FLOOR.
+ /** Tries to simplify FLOOR function on top of FLOOR.
*
* <p>Examples:
* <ul>
*
- * <li>{@code ceil(floor($0, flag(hour)), flag(day))} returns {@code ceil($0, flag(day))}
+ * <li>{@code floor(floor($0, flag(hour)), flag(day))} returns {@code floor($0, flag(day))}
*
* <li>{@code floor(floor($0, flag(second)), flag(day))} returns {@code floor($0, flag(day))}
*
- * <li>{@code floor(ceil($0, flag(day)), flag(second))} does not change
+ * <li>{@code floor(floor($0, flag(day)), flag(second))} does not change
*
* </ul>
*/
- private RexNode simplifyCeilFloor(RexCall e) {
+ private RexNode simplifyFloor(RexCall e) {
if (e.getOperands().size() != 2) {
- // Bail out since we only simplify ceil/floor <date>
+ // Bail out since we only simplify floor <date>
return e;
}
final RexNode operand = simplify_(e.getOperands().get(0));
switch (operand.getKind()) {
- case CEIL:
case FLOOR:
- // CEIL/FLOOR on top of CEIL/FLOOR
+ // FLOOR on top of FLOOR
final RexCall child = (RexCall) operand;
if (child.getOperands().size() != 2) {
- // Bail out since we only simplify ceil/floor <date>
+ // Bail out since we only simplify floor <date>
return e;
}
final RexLiteral parentFlag = (RexLiteral) e.operands.get(1);
http://git-wip-us.apache.org/repos/asf/calcite/blob/76fd6c41/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java b/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java
index 24b78c2..e09487b 100644
--- a/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java
@@ -50,7 +50,6 @@ import org.apache.calcite.util.Util;
import com.google.common.collect.ImmutableList;
-import org.junit.Ignore;
import org.junit.Test;
import java.math.BigDecimal;
@@ -377,28 +376,24 @@ public class RexImplicationCheckerTest {
is("2014"));
}
- /** Test case for simplifier of ceil and floor. */
- // Disabled: we wrongly simplify FLOOR(CEIL(2010-10-10, YEAR), YEAR)
- // to FLOOR(2010-10-10, YEAR)
- @Ignore("[CALCITE-2332]")
+ /** Test case for simplifier of floor. */
@Test public void testSimplifyFloor() {
+ // We can add more time units here once they are supported in
+ // RexInterpreter, e.g., TimeUnitRange.HOUR, TimeUnitRange.MINUTE,
+ // TimeUnitRange.SECOND.
final ImmutableList<TimeUnitRange> timeUnitRanges =
- ImmutableList.of(TimeUnitRange.WEEK,
- TimeUnitRange.YEAR, TimeUnitRange.QUARTER, TimeUnitRange.MONTH, TimeUnitRange.DAY,
- TimeUnitRange.HOUR, TimeUnitRange.MINUTE, TimeUnitRange.SECOND,
- TimeUnitRange.MILLISECOND, TimeUnitRange.MICROSECOND);
+ ImmutableList.of(TimeUnitRange.YEAR, TimeUnitRange.MONTH);
final Fixture f = new Fixture();
final RexUtil.ExprSimplifier defaultSimplifier =
new RexUtil.ExprSimplifier(f.simplify, true);
final RexNode literalTs =
f.timestampLiteral(new TimestampString("2010-10-10 00:00:00"));
- // Exclude WEEK as it is only used for the negative tests
- for (int i = 1; i < timeUnitRanges.size(); i++) {
+ for (int i = 0; i < timeUnitRanges.size(); i++) {
final RexNode innerFloorCall = f.rexBuilder.makeCall(
- SqlStdOperatorTable.CEIL, literalTs,
+ SqlStdOperatorTable.FLOOR, literalTs,
f.rexBuilder.makeFlag(timeUnitRanges.get(i)));
- for (int j = 1; j <= i; j++) {
+ for (int j = 0; j <= i; j++) {
final RexNode outerFloorCall = f.rexBuilder.makeCall(
SqlStdOperatorTable.FLOOR, innerFloorCall,
f.rexBuilder.makeFlag(timeUnitRanges.get(j)));
@@ -416,7 +411,7 @@ public class RexImplicationCheckerTest {
f.rexBuilder.makeFlag(timeUnitRanges.get(i)));
for (int j = timeUnitRanges.size() - 1; j > i; j--) {
final RexNode outerFloorCall = f.rexBuilder.makeCall(
- SqlStdOperatorTable.CEIL, innerFloorCall,
+ SqlStdOperatorTable.FLOOR, innerFloorCall,
f.rexBuilder.makeFlag(timeUnitRanges.get(j)));
final RexCall simplifiedExpr = (RexCall) defaultSimplifier.apply(outerFloorCall);
assertThat(simplifiedExpr.toString(), is(outerFloorCall.toString()));