From dc485bba55b87153b18dd733ddab3f37d38818d6 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Sat, 13 Jul 2019 13:15:14 -0700 Subject: [PATCH 01/21] Add projectStrict for Dates and Timestamps --- .../org/apache/iceberg/transforms/Dates.java | 7 +++++-- .../iceberg/transforms/ProjectionUtil.java | 17 +++++++++++++++++ .../apache/iceberg/transforms/Timestamps.java | 7 +++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/transforms/Dates.java b/api/src/main/java/org/apache/iceberg/transforms/Dates.java index a57d6d95eaa4..c7e8e9ea4e97 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/Dates.java +++ b/api/src/main/java/org/apache/iceberg/transforms/Dates.java @@ -73,8 +73,11 @@ public UnboundPredicate project(String fieldName, BoundPredicate projectStrict(String fieldName, BoundPredicate predicate) { - return null; + public UnboundPredicate projectStrict(String fieldName, BoundPredicate pred) { + if (pred.op() == NOT_NULL || pred.op() == IS_NULL) { + return Expressions.predicate(pred.op(), fieldName); + } + return ProjectionUtil.truncateStrictToInteger(fieldName, pred, this); } @Override diff --git a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java index 04da036e1637..4f1cf5c8270a 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java +++ b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java @@ -52,6 +52,23 @@ static UnboundPredicate truncateInteger( } } + static UnboundPredicate truncateStrictToInteger( + String name, BoundPredicate pred, Transform transform) { + T boundary = pred.literal().value(); + switch (pred.op()) { + case LT: + case LT_EQ: + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + case GT: + case GT_EQ: + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); + case NOT_EQ: + return predicate(Expression.Operation.NOT_EQ, name, transform.apply(boundary)); + default: + return null; + } + } + static UnboundPredicate truncateLong( String name, BoundPredicate pred, Transform transform) { long boundary = pred.literal().value(); diff --git a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java index f01ea050229c..c01e544c61f9 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java +++ b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java @@ -76,8 +76,11 @@ public UnboundPredicate project(String fieldName, BoundPredicate } @Override - public UnboundPredicate projectStrict(String fieldName, BoundPredicate predicate) { - return null; + public UnboundPredicate projectStrict(String fieldName, BoundPredicate pred) { + if (pred.op() == NOT_NULL || pred.op() == IS_NULL) { + return Expressions.predicate(pred.op(), fieldName); + } + return ProjectionUtil.truncateStrictToInteger(fieldName, pred, this); } @Override From f533ecd9453f4c4b8719aa7ee8f5a50532815c62 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Sat, 13 Jul 2019 15:41:09 -0700 Subject: [PATCH 02/21] Add tests for Timestamps --- .../transforms/TestTimestampsProjection.java | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java new file mode 100644 index 000000000000..b452e32c677e --- /dev/null +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -0,0 +1,162 @@ +package org.apache.iceberg.transforms; + +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.Literal; +import org.apache.iceberg.expressions.Projections; +import org.apache.iceberg.expressions.UnboundPredicate; +import org.apache.iceberg.types.Types; +import org.junit.Assert; +import org.junit.Test; + +import static org.apache.iceberg.TestHelpers.assertAndUnwrapUnbound; +import static org.apache.iceberg.expressions.Expressions.*; +import static org.apache.iceberg.types.Types.NestedField.optional; + +public class TestTimestampsProjection { + private static final Types.TimestampType TYPE = Types.TimestampType.withoutZone(); + private static final Schema SCHEMA = new Schema( + optional(1, "timestamp", TYPE)); + + public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { + + Expression projection = Projections.strict(spec).project(filter); + UnboundPredicate predicate = assertAndUnwrapUnbound(projection); + + Assert.assertEquals(predicate.op(), expectedOp); + + Literal literal = predicate.literal(); + Timestamps transform = (Timestamps) spec.getFieldsBySourceId(1).get(0).transform(); + String output = transform.toHumanString((int) literal.value()); + Assert.assertEquals(output, expectedLiteral); + + } + + public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { + Expression projection = Projections.inclusive(spec).project(filter); + UnboundPredicate predicate = assertAndUnwrapUnbound(projection); + + Assert.assertEquals(predicate.op(), expectedOp); + + Literal literal = predicate.literal(); + Timestamps transform = (Timestamps) spec.getFieldsBySourceId(1).get(0).transform(); + String output = transform.toHumanString((int) literal.value()); + Assert.assertEquals(output, expectedLiteral); + + } + + @Test + public void testMonthStrict() { + Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); + + } + + @Test + public void testMonthInclusive() { + Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); + + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12"); + + } + + @Test + public void testDayStrict() { + Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); + + } + + @Test + public void testDayInclusive() { + Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); + + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01"); + + } + + @Test + public void testYearStrict() { + Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); + + } + + @Test + public void testYearInclusive() { + Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); + + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017"); + assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); + assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017"); + + } + + @Test + public void testHourStrict() { + Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); + + } + + @Test + public void testHourInclusive() { + Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); + + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01-10"); + + } + + +} From 834303737d17a8e3f6ceb89675d31c39c942de54 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Sat, 13 Jul 2019 16:47:31 -0700 Subject: [PATCH 03/21] Adding more tests with lower and upper bounds --- .../transforms/TestTimestampsProjection.java | 112 +++++++++++++++++- 1 file changed, 109 insertions(+), 3 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index b452e32c677e..6c93ee6fe377 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -60,10 +60,37 @@ public void testMonthStrict() { } + @Test + public void testMonthStrictLowerBound() { + Long date = (long) Literal.of("2017-12-01T00:00:00.00000").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); + // bound should include the same month for lower bound + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); + + } + + @Test + public void testMonthStrictUpperBound() { + Long date = (long) Literal.of("2017-12-31T23:59:59.99999").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); + // bound should include the same month for upper bound + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); + + } + @Test public void testMonthInclusive() { Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); - PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); @@ -77,12 +104,39 @@ public void testMonthInclusive() { @Test public void testDayStrict() { Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); + } + + @Test + public void testDayStrictLowerBound() { + Long date = (long) Literal.of("2017-12-01T00:00:00.00000").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); + // bound should include the same day for lower bound + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); + + } + + @Test + public void testDayStrictUpperBound() { + Long date = (long) Literal.of("2017-12-01T23:59:59.99999").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); + // bound should include the same day for lower bound + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); @@ -91,7 +145,6 @@ public void testDayStrict() { @Test public void testDayInclusive() { Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); - PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); @@ -105,12 +158,39 @@ public void testDayInclusive() { @Test public void testYearStrict() { Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); + } + + @Test + public void testYearStrictLowerBound() { + Long date = (long) Literal.of("2017-01-01T00:00:00.00000").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2016"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); + // bound should include the same year for lower bound + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); + + } + + @Test + public void testYearStrictUpperBound() { + Long date = (long) Literal.of("2017-12-31T23:59:59.99999").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); + // bound should include the same year for upper bound + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); @@ -133,12 +213,39 @@ public void testYearInclusive() { @Test public void testHourStrict() { Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); + + } + + @Test + public void testHourStrictLowerBound() { + Long date = (long) Literal.of("2017-12-01T10:00:00.00000").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + // bound should include the same hour for lower bound + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); + + } + + @Test + public void testHourStrictUpperBound() { + Long date = (long) Literal.of("2017-12-01T10:59:59.99999").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); + + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); + // bound should include the same hour for upper bound + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); @@ -147,7 +254,6 @@ public void testHourStrict() { @Test public void testHourInclusive() { Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); - PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); From c3ad7e3b7c7f3fb70ccb79206e6862cc0f0e1e43 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Sat, 13 Jul 2019 17:19:49 -0700 Subject: [PATCH 04/21] Fixing tests for TimestampsProjections --- .../org/apache/iceberg/transforms/Dates.java | 2 +- .../iceberg/transforms/ProjectionUtil.java | 49 +++++++++++++++++-- .../apache/iceberg/transforms/Timestamps.java | 2 +- .../transforms/TestTimestampsProjection.java | 12 ++--- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/transforms/Dates.java b/api/src/main/java/org/apache/iceberg/transforms/Dates.java index c7e8e9ea4e97..e1210764928d 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/Dates.java +++ b/api/src/main/java/org/apache/iceberg/transforms/Dates.java @@ -77,7 +77,7 @@ public UnboundPredicate projectStrict(String fieldName, BoundPredicate< if (pred.op() == NOT_NULL || pred.op() == IS_NULL) { return Expressions.predicate(pred.op(), fieldName); } - return ProjectionUtil.truncateStrictToInteger(fieldName, pred, this); + return ProjectionUtil.truncateIntegerStrictToInteger(fieldName, pred, this); } @Override diff --git a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java index 4f1cf5c8270a..8eceda45bcfe 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java +++ b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java @@ -52,16 +52,57 @@ static UnboundPredicate truncateInteger( } } - static UnboundPredicate truncateStrictToInteger( - String name, BoundPredicate pred, Transform transform) { - T boundary = pred.literal().value(); + static UnboundPredicate truncateIntegerStrictToInteger( + String name, BoundPredicate pred, Transform transform) { + Integer boundary = pred.literal().value(); switch (pred.op()) { case LT: - case LT_EQ: return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + case LT_EQ: + // Checking if the timestamp is at the date boundary + if (transform.apply(boundary + 1).equals(transform.apply(boundary))) { + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + } else { + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary)); + } case GT: + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); case GT_EQ: + // Checking if the timestamp is at the date boundary + if (transform.apply(boundary - 1).equals(transform.apply(boundary))) { + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); + } else { + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary)); + } + case NOT_EQ: + return predicate(Expression.Operation.NOT_EQ, name, transform.apply(boundary)); + default: + return null; + } + } + + static UnboundPredicate truncateLongStrictToInteger( + String name, BoundPredicate pred, Transform transform) { + Long boundary = pred.literal().value(); + switch (pred.op()) { + case LT: + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + case LT_EQ: + // Checking if the timestamp is at the date boundary + if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + } else { + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary)); + } + case GT: return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); + case GT_EQ: + // Checking if the timestamp is at the date boundary + if (transform.apply(boundary - 1L).equals(transform.apply(boundary))) { + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); + } else { + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary)); + } case NOT_EQ: return predicate(Expression.Operation.NOT_EQ, name, transform.apply(boundary)); default: diff --git a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java index c01e544c61f9..3fe6de4a5b04 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java +++ b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java @@ -80,7 +80,7 @@ public UnboundPredicate projectStrict(String fieldName, BoundPredicate< if (pred.op() == NOT_NULL || pred.op() == IS_NULL) { return Expressions.predicate(pred.op(), fieldName); } - return ProjectionUtil.truncateStrictToInteger(fieldName, pred, this); + return ProjectionUtil.truncateLongStrictToInteger(fieldName, pred, this); } @Override diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index 6c93ee6fe377..5000865c8f0e 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -76,7 +76,7 @@ public void testMonthStrictLowerBound() { @Test public void testMonthStrictUpperBound() { - Long date = (long) Literal.of("2017-12-31T23:59:59.99999").to(TYPE).value(); + Long date = (long) Literal.of("2017-12-31T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); @@ -130,11 +130,11 @@ public void testDayStrictLowerBound() { @Test public void testDayStrictUpperBound() { - Long date = (long) Literal.of("2017-12-01T23:59:59.99999").to(TYPE).value(); + Long date = (long) Literal.of("2017-12-01T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); - // bound should include the same day for lower bound + // bound should include the same day for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); @@ -184,7 +184,7 @@ public void testYearStrictLowerBound() { @Test public void testYearStrictUpperBound() { - Long date = (long) Literal.of("2017-12-31T23:59:59.99999").to(TYPE).value(); + Long date = (long) Literal.of("2017-12-31T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); @@ -232,14 +232,14 @@ public void testHourStrictLowerBound() { assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); // bound should include the same hour for lower bound - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); } @Test public void testHourStrictUpperBound() { - Long date = (long) Literal.of("2017-12-01T10:59:59.99999").to(TYPE).value(); + Long date = (long) Literal.of("2017-12-01T10:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); From fae0b36f40dfdfc39cce78ac77b0843815997693 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Sat, 13 Jul 2019 17:40:37 -0700 Subject: [PATCH 05/21] Adding tests for Dates Projection --- .../transforms/TestDatesProjection.java | 200 ++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java new file mode 100644 index 000000000000..bfd9356e20e5 --- /dev/null +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -0,0 +1,200 @@ +package org.apache.iceberg.transforms; + +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.Literal; +import org.apache.iceberg.expressions.Projections; +import org.apache.iceberg.expressions.UnboundPredicate; +import org.apache.iceberg.types.Types; +import org.junit.Assert; +import org.junit.Test; + +import static org.apache.iceberg.TestHelpers.assertAndUnwrapUnbound; +import static org.apache.iceberg.expressions.Expressions.*; +import static org.apache.iceberg.types.Types.NestedField.optional; + +public class TestDatesProjection { + private static final Types.DateType TYPE = Types.DateType.get(); + private static final Schema SCHEMA = new Schema( + optional(1, "date", TYPE)); + + public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { + + Expression projection = Projections.strict(spec).project(filter); + UnboundPredicate predicate = assertAndUnwrapUnbound(projection); + + Assert.assertEquals(predicate.op(), expectedOp); + + Literal literal = predicate.literal(); + Dates transform = (Dates) spec.getFieldsBySourceId(1).get(0).transform(); + String output = transform.toHumanString((int) literal.value()); + Assert.assertEquals(output, expectedLiteral); + + } + + public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { + Expression projection = Projections.inclusive(spec).project(filter); + UnboundPredicate predicate = assertAndUnwrapUnbound(projection); + + Assert.assertEquals(predicate.op(), expectedOp); + + Literal literal = predicate.literal(); + Dates transform = (Dates) spec.getFieldsBySourceId(1).get(0).transform(); + String output = transform.toHumanString((int) literal.value()); + Assert.assertEquals(output, expectedLiteral); + + } + + @Test + public void testMonthStrictLowerBound() { + Integer date = (Integer) Literal.of("2017-01-01").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); + + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016-12"); + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2016-12"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-02"); + // bound should include the same month for lower bound + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01"); + assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01"); + + } + + @Test + public void testMonthStrictUpperBound() { + Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); + + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-11"); + // bound should include the same month for upper bound + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-12"); + + } + + @Test + public void testMonthInclusiveLowerBound() { + Integer date = (Integer) Literal.of("2017-12-01").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); + + assertProjectionInclusive(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-11"); + assertProjectionInclusive(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-12"); + + } + + @Test + public void testMonthInclusiveUpperBound() { + Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); + + assertProjectionInclusive(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionInclusive(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-12"); + + } + + @Test + public void testDayStrictLowerBound() { + Integer date = (Integer) Literal.of("2017-01-01").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("date").build(); + + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016-12-31"); + // should be the same date for <= + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-01-01"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-01-02"); + // should be the same date for >= + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01-01"); + assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01-01"); + + } + + @Test + public void testDayStrictUpperBound() { + Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("date").build(); + + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-12-30"); + // should be the same date for <= + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12-31"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01-01"); + // should be the same date for >= + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12-31"); + assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-12-31"); + + } + + @Test + public void testDayInclusive() { + Integer date = (Integer) Literal.of("2017-12-01").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("date").build(); + + assertProjectionInclusive(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-11-30"); + assertProjectionInclusive(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-12-02"); + assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-12-01"); + + } + + @Test + public void testYearStrictLowerBound() { + Integer date = (Integer) Literal.of("2017-01-01").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); + + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); + // bound should include the same year for lower bound + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); + assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); + + } + + @Test + public void testYearStrictUpperBound() { + Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); + + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); + // bound should include the same year for upper bound + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); + + } + + @Test + public void testYearInclusiveLowerBound() { + Integer date = (Integer) Literal.of("2017-01-01").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); + + assertProjectionInclusive(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionInclusive(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017"); + assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); + assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017"); + + } + + @Test + public void testYearInclusiveUpperBound() { + Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); + + assertProjectionInclusive(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionInclusive(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); + assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017"); + + } + +} From c3620e55e95eed8e473c0a6aef49f0ba88082ed5 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Sat, 13 Jul 2019 17:48:43 -0700 Subject: [PATCH 06/21] Refactor tests --- .../transforms/TestDatesProjection.java | 4 +- .../transforms/TestTimestampsProjection.java | 108 +++++++++--------- 2 files changed, 55 insertions(+), 57 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java index bfd9356e20e5..685889db20ea 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -29,7 +29,7 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte Literal literal = predicate.literal(); Dates transform = (Dates) spec.getFieldsBySourceId(1).get(0).transform(); String output = transform.toHumanString((int) literal.value()); - Assert.assertEquals(output, expectedLiteral); + Assert.assertEquals(expectedLiteral, output); } @@ -42,7 +42,7 @@ public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate fi Literal literal = predicate.literal(); Dates transform = (Dates) spec.getFieldsBySourceId(1).get(0).transform(); String output = transform.toHumanString((int) literal.value()); - Assert.assertEquals(output, expectedLiteral); + Assert.assertEquals(expectedLiteral, output); } diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index 5000865c8f0e..8cfe54e65680 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -29,7 +29,7 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte Literal literal = predicate.literal(); Timestamps transform = (Timestamps) spec.getFieldsBySourceId(1).get(0).transform(); String output = transform.toHumanString((int) literal.value()); - Assert.assertEquals(output, expectedLiteral); + Assert.assertEquals(expectedLiteral, output); } @@ -42,21 +42,7 @@ public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate fi Literal literal = predicate.literal(); Timestamps transform = (Timestamps) spec.getFieldsBySourceId(1).get(0).transform(); String output = transform.toHumanString((int) literal.value()); - Assert.assertEquals(output, expectedLiteral); - - } - - @Test - public void testMonthStrict() { - Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); - - PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); - - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); - assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); + Assert.assertEquals(expectedLiteral, output); } @@ -76,20 +62,20 @@ public void testMonthStrictLowerBound() { @Test public void testMonthStrictUpperBound() { - Long date = (long) Literal.of("2017-12-31T23:59:59.999999").to(TYPE).value(); + Long date = (long) Literal.of("2017-12-01T00:00:00.00000").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - // bound should include the same month for upper bound - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); + // bound should include the same month for upper bound + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); } @Test - public void testMonthInclusive() { + public void testMonthInclusiveLowerBound() { Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); @@ -102,15 +88,15 @@ public void testMonthInclusive() { } @Test - public void testDayStrict() { - Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); - PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); + public void testMonthInclusiveUpperBound() { + Long date = (long) Literal.of("2017-12-01T00:00:00.00000").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); - assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); + assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12"); } @@ -143,11 +129,11 @@ public void testDayStrictUpperBound() { } @Test - public void testDayInclusive() { - Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + public void testDayInclusiveLowerBound() { + Long date = (long) Literal.of("2017-12-01T00:00:00.00000").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); - assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); @@ -156,15 +142,15 @@ public void testDayInclusive() { } @Test - public void testYearStrict() { - Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); - PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); + public void testDayInclusiveUpperBound() { + Long date = (long) Literal.of("2017-12-01T23:59:59.999999").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2016"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018"); - assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); + assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); + assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01"); } @@ -197,12 +183,11 @@ public void testYearStrictUpperBound() { } @Test - public void testYearInclusive() { - Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); - + public void testYearInclusiveLowerBound() { + Long date = (long) Literal.of("2017-01-01T00:00:00.00000").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); - assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017"); assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); @@ -211,15 +196,15 @@ public void testYearInclusive() { } @Test - public void testHourStrict() { - Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); - PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); + public void testYearInclusiveUpperBound() { + Long date = (long) Literal.of("2017-12-31T23:59:59.999999").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); - assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017"); + assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); + assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017"); } @@ -252,11 +237,11 @@ public void testHourStrictUpperBound() { } @Test - public void testHourInclusive() { - Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + public void testHourInclusiveLowerBound() { + Long date = (long) Literal.of("2017-12-01T10:00:00.00000").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); - assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); @@ -264,5 +249,18 @@ public void testHourInclusive() { } + @Test + public void testHourInclusiveUpperBound() { + Long date = (long) Literal.of("2017-12-01T10:59:59.999999").to(TYPE).value(); + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); + + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); + assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01-10"); + + } + } From 304321f6571138c5e3f96fb65726da5b92f28389 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Mon, 15 Jul 2019 14:26:18 -0700 Subject: [PATCH 07/21] Formatting --- .../transforms/TestDatesProjection.java | 35 ++++++++++++++++--- .../transforms/TestTimestampsProjection.java | 35 ++++++++++++++++--- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java index 685889db20ea..5f5f77a0706e 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -1,3 +1,22 @@ +/* + * 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.iceberg.transforms; import org.apache.iceberg.PartitionSpec; @@ -11,15 +30,20 @@ import org.junit.Test; import static org.apache.iceberg.TestHelpers.assertAndUnwrapUnbound; -import static org.apache.iceberg.expressions.Expressions.*; +import static org.apache.iceberg.expressions.Expressions.equal; +import static org.apache.iceberg.expressions.Expressions.greaterThan; +import static org.apache.iceberg.expressions.Expressions.greaterThanOrEqual; +import static org.apache.iceberg.expressions.Expressions.lessThan; +import static org.apache.iceberg.expressions.Expressions.lessThanOrEqual; +import static org.apache.iceberg.expressions.Expressions.notEqual; import static org.apache.iceberg.types.Types.NestedField.optional; public class TestDatesProjection { private static final Types.DateType TYPE = Types.DateType.get(); - private static final Schema SCHEMA = new Schema( - optional(1, "date", TYPE)); + private static final Schema SCHEMA = new Schema(optional(1, "date", TYPE)); - public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { + public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filter, + Expression.Operation expectedOp, String expectedLiteral) { Expression projection = Projections.strict(spec).project(filter); UnboundPredicate predicate = assertAndUnwrapUnbound(projection); @@ -33,7 +57,8 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte } - public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { + public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, + Expression.Operation expectedOp, String expectedLiteral) { Expression projection = Projections.inclusive(spec).project(filter); UnboundPredicate predicate = assertAndUnwrapUnbound(projection); diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index 8cfe54e65680..8d6e562646b1 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -1,3 +1,22 @@ +/* + * 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.iceberg.transforms; import org.apache.iceberg.PartitionSpec; @@ -11,15 +30,20 @@ import org.junit.Test; import static org.apache.iceberg.TestHelpers.assertAndUnwrapUnbound; -import static org.apache.iceberg.expressions.Expressions.*; +import static org.apache.iceberg.expressions.Expressions.equal; +import static org.apache.iceberg.expressions.Expressions.greaterThan; +import static org.apache.iceberg.expressions.Expressions.greaterThanOrEqual; +import static org.apache.iceberg.expressions.Expressions.lessThan; +import static org.apache.iceberg.expressions.Expressions.lessThanOrEqual; +import static org.apache.iceberg.expressions.Expressions.notEqual; import static org.apache.iceberg.types.Types.NestedField.optional; public class TestTimestampsProjection { private static final Types.TimestampType TYPE = Types.TimestampType.withoutZone(); - private static final Schema SCHEMA = new Schema( - optional(1, "timestamp", TYPE)); + private static final Schema SCHEMA = new Schema(optional(1, "timestamp", TYPE)); - public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { + public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filter, + Expression.Operation expectedOp, String expectedLiteral) { Expression projection = Projections.strict(spec).project(filter); UnboundPredicate predicate = assertAndUnwrapUnbound(projection); @@ -33,7 +57,8 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte } - public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { + public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, + Expression.Operation expectedOp, String expectedLiteral) { Expression projection = Projections.inclusive(spec).project(filter); UnboundPredicate predicate = assertAndUnwrapUnbound(projection); From b90edff9ec18b0939adbbc51b549f91d0630185a Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Mon, 15 Jul 2019 16:47:44 -0700 Subject: [PATCH 08/21] Removing duplicate test --- .../iceberg/transforms/TestDatesProjection.java | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java index 5f5f77a0706e..16ca37ca305a 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -126,7 +126,7 @@ public void testMonthInclusiveUpperBound() { } @Test - public void testDayStrictLowerBound() { + public void testDayStrict() { Integer date = (Integer) Literal.of("2017-01-01").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("date").build(); @@ -140,21 +140,6 @@ public void testDayStrictLowerBound() { } - @Test - public void testDayStrictUpperBound() { - Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); - PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("date").build(); - - assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-12-30"); - // should be the same date for <= - assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12-31"); - assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01-01"); - // should be the same date for >= - assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12-31"); - assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-12-31"); - - } - @Test public void testDayInclusive() { Integer date = (Integer) Literal.of("2017-12-01").to(TYPE).value(); From 3a39123d699b904e24954bef490dc084866e478e Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Mon, 15 Jul 2019 18:35:18 -0700 Subject: [PATCH 09/21] Using generics instead of Integer for the truncateIntegerStrict and truncateLongStrict methods --- .../org/apache/iceberg/transforms/Dates.java | 2 +- .../iceberg/transforms/ProjectionUtil.java | 56 +++++++++++++------ .../apache/iceberg/transforms/Timestamps.java | 2 +- .../transforms/TestDatesProjection.java | 30 +++++----- .../transforms/TestTimestampsProjection.java | 34 +++++------ 5 files changed, 72 insertions(+), 52 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/transforms/Dates.java b/api/src/main/java/org/apache/iceberg/transforms/Dates.java index e1210764928d..94714d2e7069 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/Dates.java +++ b/api/src/main/java/org/apache/iceberg/transforms/Dates.java @@ -77,7 +77,7 @@ public UnboundPredicate projectStrict(String fieldName, BoundPredicate< if (pred.op() == NOT_NULL || pred.op() == IS_NULL) { return Expressions.predicate(pred.op(), fieldName); } - return ProjectionUtil.truncateIntegerStrictToInteger(fieldName, pred, this); + return ProjectionUtil.truncateIntegerStrict(fieldName, pred, this); } @Override diff --git a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java index 8eceda45bcfe..112a09441fcf 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java +++ b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java @@ -52,25 +52,35 @@ static UnboundPredicate truncateInteger( } } - static UnboundPredicate truncateIntegerStrictToInteger( - String name, BoundPredicate pred, Transform transform) { - Integer boundary = pred.literal().value(); + static UnboundPredicate truncateIntegerStrict( + String name, BoundPredicate pred, Transform transform) { + int boundary = pred.literal().value(); switch (pred.op()) { case LT: - return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + // Checking if the literal is at the lower partition boundary + if (transform.apply(boundary - 1).equals(transform.apply(boundary))) { + return predicate(Expression.Operation.LT, name, transform.apply(boundary - 1)); + } else { + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary - 1)); + } case LT_EQ: - // Checking if the timestamp is at the date boundary + // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1).equals(transform.apply(boundary))) { - return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + return predicate(Expression.Operation.LT, name, transform.apply(boundary)); } else { return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary)); } case GT: - return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); + // Checking if the literal is at the upper partition boundary + if (transform.apply(boundary + 1).equals(transform.apply(boundary))) { + return predicate(Expression.Operation.GT, name, transform.apply(boundary + 1)); + } else { + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary + 1)); + } case GT_EQ: - // Checking if the timestamp is at the date boundary + // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1).equals(transform.apply(boundary))) { - return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); + return predicate(Expression.Operation.GT, name, transform.apply(boundary)); } else { return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary)); } @@ -81,25 +91,35 @@ static UnboundPredicate truncateIntegerStrictToInteger( } } - static UnboundPredicate truncateLongStrictToInteger( - String name, BoundPredicate pred, Transform transform) { - Long boundary = pred.literal().value(); + static UnboundPredicate truncateLongStrict( + String name, BoundPredicate pred, Transform transform) { + long boundary = pred.literal().value(); switch (pred.op()) { case LT: - return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + // Checking if the literal is at the lower partition boundary + if (transform.apply(boundary - 1L).equals(transform.apply(boundary))) { + return predicate(Expression.Operation.LT, name, transform.apply(boundary - 1L)); + } else { + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary - 1L)); + } case LT_EQ: - // Checking if the timestamp is at the date boundary + // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { - return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); + return predicate(Expression.Operation.LT, name, transform.apply(boundary)); } else { return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary)); } case GT: - return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); + // Checking if the literal is at the upper partition boundary + if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { + return predicate(Expression.Operation.GT, name, transform.apply(boundary + 1L)); + } else { + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary + 1L)); + } case GT_EQ: - // Checking if the timestamp is at the date boundary + // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1L).equals(transform.apply(boundary))) { - return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); + return predicate(Expression.Operation.GT, name, transform.apply(boundary)); } else { return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary)); } diff --git a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java index 3fe6de4a5b04..7259defd85f3 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java +++ b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java @@ -80,7 +80,7 @@ public UnboundPredicate projectStrict(String fieldName, BoundPredicate< if (pred.op() == NOT_NULL || pred.op() == IS_NULL) { return Expressions.predicate(pred.op(), fieldName); } - return ProjectionUtil.truncateLongStrictToInteger(fieldName, pred, this); + return ProjectionUtil.truncateLongStrict(fieldName, pred, this); } @Override diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java index 16ca37ca305a..dbdbd8e31d7b 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -48,7 +48,7 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte Expression projection = Projections.strict(spec).project(filter); UnboundPredicate predicate = assertAndUnwrapUnbound(projection); - Assert.assertEquals(predicate.op(), expectedOp); + Assert.assertEquals(expectedOp, predicate.op()); Literal literal = predicate.literal(); Dates transform = (Dates) spec.getFieldsBySourceId(1).get(0).transform(); @@ -77,8 +77,8 @@ public void testMonthStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016-12"); - assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2016-12"); - assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-02"); + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT, "2017-01"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT, "2017-01"); // bound should include the same month for lower bound assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01"); @@ -90,11 +90,11 @@ public void testMonthStrictUpperBound() { Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); - assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-11"); + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT, "2017-12"); // bound should include the same month for upper bound assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12"); assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); - assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT, "2017-12"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-12"); } @@ -142,14 +142,14 @@ public void testDayStrict() { @Test public void testDayInclusive() { - Integer date = (Integer) Literal.of("2017-12-01").to(TYPE).value(); + Integer date = (Integer) Literal.of("2017-01-01").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("date").build(); - assertProjectionInclusive(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-11-30"); - assertProjectionInclusive(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12-01"); - assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-12-02"); - assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12-01"); - assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-12-01"); + assertProjectionInclusive(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016-12-31"); + assertProjectionInclusive(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-01-01"); + assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-01-02"); + assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01-01"); + assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-01-01"); } @@ -159,8 +159,8 @@ public void testYearStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); - assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2016"); - assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT, "2017"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT, "2017"); // bound should include the same year for lower bound assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); @@ -172,11 +172,11 @@ public void testYearStrictUpperBound() { Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); - assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT, "2017"); // bound should include the same year for upper bound assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017"); assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); - assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT, "2017"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); } diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index 8d6e562646b1..02fcf9035d7d 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -48,7 +48,7 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte Expression projection = Projections.strict(spec).project(filter); UnboundPredicate predicate = assertAndUnwrapUnbound(projection); - Assert.assertEquals(predicate.op(), expectedOp); + Assert.assertEquals(expectedOp, predicate.op()); Literal literal = predicate.literal(); Timestamps transform = (Timestamps) spec.getFieldsBySourceId(1).get(0).transform(); @@ -77,8 +77,8 @@ public void testMonthStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017-12"); // bound should include the same month for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); @@ -91,8 +91,8 @@ public void testMonthStrictUpperBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017-12"); // bound should include the same month for upper bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); @@ -131,8 +131,8 @@ public void testDayStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12-01"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017-12-01"); // bound should include the same day for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); @@ -144,11 +144,11 @@ public void testDayStrictUpperBound() { Long date = (long) Literal.of("2017-12-01T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT, "2017-12-01"); // bound should include the same day for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); } @@ -185,8 +185,8 @@ public void testYearStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2016"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017"); // bound should include the same year for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); @@ -198,11 +198,11 @@ public void testYearStrictUpperBound() { Long date = (long) Literal.of("2017-12-31T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT, "2017"); // bound should include the same year for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); } @@ -239,8 +239,8 @@ public void testHourStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12-01-10"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017-12-01-10"); // bound should include the same hour for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); @@ -252,11 +252,11 @@ public void testHourStrictUpperBound() { Long date = (long) Literal.of("2017-12-01T10:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT, "2017-12-01-10"); // bound should include the same hour for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12-01-10"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); } From 712ce2266d20fc70995bb051ad2f3d0969ca252c Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Mon, 15 Jul 2019 18:49:45 -0700 Subject: [PATCH 10/21] Correcting upper bound test values --- .../transforms/TestTimestampsProjection.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index 02fcf9035d7d..d8d8261e9474 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -87,24 +87,24 @@ public void testMonthStrictLowerBound() { @Test public void testMonthStrictUpperBound() { - Long date = (long) Literal.of("2017-12-01T00:00:00.00000").to(TYPE).value(); + Long date = (long) Literal.of("2017-12-31T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017-12"); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT, "2017-12"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); // bound should include the same month for upper bound - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); } @Test public void testMonthInclusiveLowerBound() { - Long date = (long) Literal.of("2017-12-01T10:12:55.038194").to(TYPE).value(); + Long date = (long) Literal.of("2017-12-01T00:00:00.00000").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); - assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); @@ -114,10 +114,10 @@ public void testMonthInclusiveLowerBound() { @Test public void testMonthInclusiveUpperBound() { - Long date = (long) Literal.of("2017-12-01T00:00:00.00000").to(TYPE).value(); + Long date = (long) Literal.of("2017-12-01T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); - assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); + assertProjectionInclusive(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); assertProjectionInclusive(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); From f9763258173edbd3c9eb605e1043da486e16d0c0 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Wed, 17 Jul 2019 18:22:52 -0700 Subject: [PATCH 11/21] Fix Residual Evaluator --- .../expressions/ResidualEvaluator.java | 82 ++++++++++++++++--- 1 file changed, 71 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java b/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java index 0fae40d3decb..e257b1c00720 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java +++ b/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java @@ -153,48 +153,50 @@ public Expression or(Expression leftResult, Expression rightResult) { @Override public Expression isNull(BoundReference ref) { - return (ref.get(struct) == null) ? alwaysTrue() : alwaysFalse(); + // return alwaysTrue or alwaysFalse because the strict projection always gives correct answers for isNull + return (ref.get(struct) == null) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression notNull(BoundReference ref) { - return (ref.get(struct) != null) ? alwaysTrue() : alwaysFalse(); + // return alwaysTrue or alwaysFalse because the strict projection always gives correct answers for notNull + return (ref.get(struct) != null) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression lt(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) < 0) ? alwaysTrue() : alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) < 0) ? Expressions.alwaysTrue() : null; } @Override public Expression ltEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) <= 0) ? alwaysTrue() : alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) <= 0) ? Expressions.alwaysTrue() : null; } @Override public Expression gt(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) > 0) ? alwaysTrue() : alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) > 0) ? Expressions.alwaysTrue() : null; } @Override public Expression gtEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) >= 0) ? alwaysTrue() : alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) >= 0) ? Expressions.alwaysTrue() : null; } @Override public Expression eq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) == 0) ? alwaysTrue() : alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) == 0) ? Expressions.alwaysTrue() : null; } @Override public Expression notEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) != 0) ? alwaysTrue() : alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) != 0) ? Expressions.alwaysTrue() : null; } @Override @@ -220,15 +222,46 @@ public Expression predicate(BoundPredicate pred) { } Expression result = Expressions.alwaysFalse(); - for (UnboundPredicate strictProjection : strictProjections) { + for (PartitionField part : parts) { + UnboundPredicate strictProjection = ((Transform) part.transform()).projectStrict(part.name(), pred); + if (strictProjection == null) { continue; } Expression bound = strictProjection.bind(spec.partitionType(), caseSensitive); + if (bound instanceof BoundPredicate) { - // evaluate the bound predicate, which will return alwaysTrue or alwaysFalse - result = Expressions.or(result, super.predicate((BoundPredicate) bound)); + // evaluate the bound predicate, which will return alwaysTrue, alwaysFalse, or null + Expression strictResult = super.predicate((BoundPredicate) bound); + + if (strictResult != null) { + result = Expressions.or(result, strictResult); + } else { + + // strict result is null i.e. inconclusive. Getting inclusive projection + UnboundPredicate inclusiveProjection = ((Transform) part.transform()).project(part.name(), pred); + Expression boundInclusive = inclusiveProjection.bind(spec.partitionType(), caseSensitive); + + if (boundInclusive instanceof BoundPredicate) { + Expression inclusiveResult = predicateInclusive((BoundPredicate) boundInclusive); + + if (inclusiveResult != null) { + // strict projection is null, but inclusive projection is not + // AND-ing the expression because it's inclusive + result = Expressions.and(result, inclusiveResult); + } else { + // if both strict and inclusive projections are null, then the strict projection did + // not guarantee a result for all values, nor the inclusive projection so the + // predicate must be included in the residual + result = Expressions.or(result, pred); + } + } else { + // update the result expression with the non-predicate residual (e.g. alwaysTrue) + // AND-ing the expression because it's inclusive + result = Expressions.and(result, boundInclusive); + } + } } else { // update the result expression with the non-predicate residual (e.g. alwaysTrue) result = Expressions.or(result, bound); @@ -253,5 +286,32 @@ public Expression predicate(UnboundPredicate pred) { // if binding didn't result in a Predicate, return the expression return bound; } + + // Overriding some behaviour for inclusive projections + public Expression predicateInclusive(BoundPredicate pred) { + Comparator cmp = null; + switch (pred.op()) { + case LT: + cmp = pred.literal().comparator(); + return (cmp.compare(pred.ref().get(struct), pred.literal().value()) < 0) ? null : Expressions.alwaysFalse(); + case LT_EQ: + cmp = pred.literal().comparator(); + return (cmp.compare(pred.ref().get(struct), pred.literal().value()) <= 0) ? null : Expressions.alwaysFalse(); + case GT: + cmp = pred.literal().comparator(); + return (cmp.compare(pred.ref().get(struct), pred.literal().value()) > 0) ? null : Expressions.alwaysFalse(); + case GT_EQ: + cmp = pred.literal().comparator(); + return (cmp.compare(pred.ref().get(struct), pred.literal().value()) >= 0) ? null : Expressions.alwaysFalse(); + case EQ: + cmp = pred.literal().comparator(); + return (cmp.compare(pred.ref().get(struct), pred.literal().value()) == 0) ? null : Expressions.alwaysFalse(); + case NOT_EQ: + cmp = pred.literal().comparator(); + return (cmp.compare(pred.ref().get(struct), pred.literal().value()) != 0) ? null : Expressions.alwaysFalse(); + default: + return super.predicate(pred); + } + } } } From a82faafc7ef7e0766842d56ed0a4ebb6910c34ed Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Thu, 18 Jul 2019 14:05:38 -0700 Subject: [PATCH 12/21] Refactoring logic --- .../expressions/ResidualEvaluator.java | 79 ++++++++----------- 1 file changed, 31 insertions(+), 48 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java b/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java index e257b1c00720..737c31ac0d2a 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java +++ b/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java @@ -19,12 +19,9 @@ package org.apache.iceberg.expressions; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import java.io.Serializable; import java.util.Comparator; import java.util.List; -import java.util.Objects; import org.apache.iceberg.PartitionField; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.StructLike; @@ -213,62 +210,48 @@ public Expression predicate(BoundPredicate pred) { return pred; // not associated inclusive a partition field, can't be evaluated } - List> strictProjections = Lists.transform(parts, - part -> ((Transform) part.transform()).projectStrict(part.name(), pred)); - - if (Iterables.all(strictProjections, Objects::isNull)) { - // if there are no strict projections, the predicate must be in the residual - return pred; - } - - Expression result = Expressions.alwaysFalse(); for (PartitionField part : parts) { + + // checking the strict projection UnboundPredicate strictProjection = ((Transform) part.transform()).projectStrict(part.name(), pred); + Expression strictResult = null; - if (strictProjection == null) { - continue; + if (strictProjection != null) { + Expression bound = strictProjection.bind(spec.partitionType(), caseSensitive); + if (bound instanceof BoundPredicate) { + strictResult = super.predicate((BoundPredicate) bound); + } else { + strictResult = bound; + } } - Expression bound = strictProjection.bind(spec.partitionType(), caseSensitive); - - if (bound instanceof BoundPredicate) { - // evaluate the bound predicate, which will return alwaysTrue, alwaysFalse, or null - Expression strictResult = super.predicate((BoundPredicate) bound); + if (strictResult != null && strictResult.equals(Expressions.alwaysTrue())) { + // If strict is true, returning true + return Expressions.alwaysTrue(); + } - if (strictResult != null) { - result = Expressions.or(result, strictResult); + // checking the inclusive projection + UnboundPredicate inclusiveProjection = ((Transform) part.transform()).project(part.name(), pred); + Expression inclusiveResult = null; + if (inclusiveProjection != null) { + Expression boundInclusive = inclusiveProjection.bind(spec.partitionType(), caseSensitive); + if (boundInclusive instanceof BoundPredicate) { + // using predicate method specific to inclusive + inclusiveResult = predicateInclusive((BoundPredicate) boundInclusive); } else { - - // strict result is null i.e. inconclusive. Getting inclusive projection - UnboundPredicate inclusiveProjection = ((Transform) part.transform()).project(part.name(), pred); - Expression boundInclusive = inclusiveProjection.bind(spec.partitionType(), caseSensitive); - - if (boundInclusive instanceof BoundPredicate) { - Expression inclusiveResult = predicateInclusive((BoundPredicate) boundInclusive); - - if (inclusiveResult != null) { - // strict projection is null, but inclusive projection is not - // AND-ing the expression because it's inclusive - result = Expressions.and(result, inclusiveResult); - } else { - // if both strict and inclusive projections are null, then the strict projection did - // not guarantee a result for all values, nor the inclusive projection so the - // predicate must be included in the residual - result = Expressions.or(result, pred); - } - } else { - // update the result expression with the non-predicate residual (e.g. alwaysTrue) - // AND-ing the expression because it's inclusive - result = Expressions.and(result, boundInclusive); - } + inclusiveResult = boundInclusive; } - } else { - // update the result expression with the non-predicate residual (e.g. alwaysTrue) - result = Expressions.or(result, bound); } + + if (inclusiveResult != null && inclusiveResult.equals(Expressions.alwaysFalse())) { + // If inclusive is false, returning false + return Expressions.alwaysFalse(); + } + } - return result; + // neither strict not inclusive predicate was conclusive, returning the original pred + return pred; } @Override From c8155f8f1cba723d8fdc40c04d15f2ee21287b24 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Wed, 24 Jul 2019 11:56:39 -0700 Subject: [PATCH 13/21] Addressing review comments Refactored and removed predicateInclusive Updated and added more comments --- .../expressions/ResidualEvaluator.java | 62 ++++++------------- 1 file changed, 20 insertions(+), 42 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java b/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java index 737c31ac0d2a..9835c094f6b9 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java +++ b/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java @@ -150,59 +150,62 @@ public Expression or(Expression leftResult, Expression rightResult) { @Override public Expression isNull(BoundReference ref) { - // return alwaysTrue or alwaysFalse because the strict projection always gives correct answers for isNull return (ref.get(struct) == null) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression notNull(BoundReference ref) { - // return alwaysTrue or alwaysFalse because the strict projection always gives correct answers for notNull return (ref.get(struct) != null) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression lt(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) < 0) ? Expressions.alwaysTrue() : null; + return (cmp.compare(ref.get(struct), lit.value()) < 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression ltEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) <= 0) ? Expressions.alwaysTrue() : null; + return (cmp.compare(ref.get(struct), lit.value()) <= 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression gt(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) > 0) ? Expressions.alwaysTrue() : null; + return (cmp.compare(ref.get(struct), lit.value()) > 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression gtEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) >= 0) ? Expressions.alwaysTrue() : null; + return (cmp.compare(ref.get(struct), lit.value()) >= 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression eq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) == 0) ? Expressions.alwaysTrue() : null; + return (cmp.compare(ref.get(struct), lit.value()) == 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override public Expression notEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) != 0) ? Expressions.alwaysTrue() : null; + return (cmp.compare(ref.get(struct), lit.value()) != 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); } @Override @SuppressWarnings("unchecked") public Expression predicate(BoundPredicate pred) { - // Get the strict projection of this predicate in partition data, then use it to determine - // whether to return the original predicate. The strict projection returns true iff the - // original predicate would have returned true, so the predicate can be eliminated if the - // strict projection evaluates to true. + /** + * Get the strict projection and inclusive projection of this predicate in partition data, + * then use them to determine whether to return the original predicate. The strict projection + * returns true iff the original predicate would have returned true, so the predicate can be + * eliminated if the strict projection evaluates to true. Similarly the inclusive projection + * returns false iff the original predicate would have returned false, so the predicate can + * also be eliminated if the inclusive projection evaluates to false. + */ + // // If there is no strict projection or if it evaluates to false, then return the predicate. List parts = spec.getFieldsBySourceId(pred.ref().fieldId()); @@ -221,11 +224,12 @@ public Expression predicate(BoundPredicate pred) { if (bound instanceof BoundPredicate) { strictResult = super.predicate((BoundPredicate) bound); } else { + // if the result is not a predicate, then it must be a constant like alwaysTrue or alwaysFalse strictResult = bound; } } - if (strictResult != null && strictResult.equals(Expressions.alwaysTrue())) { + if (strictResult != null && strictResult.op() == Expression.Operation.TRUE) { // If strict is true, returning true return Expressions.alwaysTrue(); } @@ -237,13 +241,14 @@ public Expression predicate(BoundPredicate pred) { Expression boundInclusive = inclusiveProjection.bind(spec.partitionType(), caseSensitive); if (boundInclusive instanceof BoundPredicate) { // using predicate method specific to inclusive - inclusiveResult = predicateInclusive((BoundPredicate) boundInclusive); + inclusiveResult = super.predicate((BoundPredicate) boundInclusive); } else { + // if the result is not a predicate, then it must be a constant like alwaysTrue or alwaysFalse inclusiveResult = boundInclusive; } } - if (inclusiveResult != null && inclusiveResult.equals(Expressions.alwaysFalse())) { + if (inclusiveResult != null && inclusiveResult.op() == Expression.Operation.FALSE) { // If inclusive is false, returning false return Expressions.alwaysFalse(); } @@ -269,32 +274,5 @@ public Expression predicate(UnboundPredicate pred) { // if binding didn't result in a Predicate, return the expression return bound; } - - // Overriding some behaviour for inclusive projections - public Expression predicateInclusive(BoundPredicate pred) { - Comparator cmp = null; - switch (pred.op()) { - case LT: - cmp = pred.literal().comparator(); - return (cmp.compare(pred.ref().get(struct), pred.literal().value()) < 0) ? null : Expressions.alwaysFalse(); - case LT_EQ: - cmp = pred.literal().comparator(); - return (cmp.compare(pred.ref().get(struct), pred.literal().value()) <= 0) ? null : Expressions.alwaysFalse(); - case GT: - cmp = pred.literal().comparator(); - return (cmp.compare(pred.ref().get(struct), pred.literal().value()) > 0) ? null : Expressions.alwaysFalse(); - case GT_EQ: - cmp = pred.literal().comparator(); - return (cmp.compare(pred.ref().get(struct), pred.literal().value()) >= 0) ? null : Expressions.alwaysFalse(); - case EQ: - cmp = pred.literal().comparator(); - return (cmp.compare(pred.ref().get(struct), pred.literal().value()) == 0) ? null : Expressions.alwaysFalse(); - case NOT_EQ: - cmp = pred.literal().comparator(); - return (cmp.compare(pred.ref().get(struct), pred.literal().value()) != 0) ? null : Expressions.alwaysFalse(); - default: - return super.predicate(pred); - } - } } } From 07ae9998822a8f50d856b63605ee10669f100fc9 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Wed, 24 Jul 2019 13:16:32 -0700 Subject: [PATCH 14/21] Removing unnecessary changes to ResidualEvaluator --- .../iceberg/expressions/ResidualEvaluator.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java b/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java index 9835c094f6b9..dd3a0b8da8a0 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java +++ b/api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java @@ -150,48 +150,48 @@ public Expression or(Expression leftResult, Expression rightResult) { @Override public Expression isNull(BoundReference ref) { - return (ref.get(struct) == null) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); + return (ref.get(struct) == null) ? alwaysTrue() : alwaysFalse(); } @Override public Expression notNull(BoundReference ref) { - return (ref.get(struct) != null) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); + return (ref.get(struct) != null) ? alwaysTrue() : alwaysFalse(); } @Override public Expression lt(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) < 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) < 0) ? alwaysTrue() : alwaysFalse(); } @Override public Expression ltEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) <= 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) <= 0) ? alwaysTrue() : alwaysFalse(); } @Override public Expression gt(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) > 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) > 0) ? alwaysTrue() : alwaysFalse(); } @Override public Expression gtEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) >= 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) >= 0) ? alwaysTrue() : alwaysFalse(); } @Override public Expression eq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) == 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) == 0) ? alwaysTrue() : alwaysFalse(); } @Override public Expression notEq(BoundReference ref, Literal lit) { Comparator cmp = lit.comparator(); - return (cmp.compare(ref.get(struct), lit.value()) != 0) ? Expressions.alwaysTrue() : Expressions.alwaysFalse(); + return (cmp.compare(ref.get(struct), lit.value()) != 0) ? alwaysTrue() : alwaysFalse(); } @Override From 33df0d3721b88e2b4aac62b4ddb21da0f7c7014d Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Wed, 24 Jul 2019 13:21:05 -0700 Subject: [PATCH 15/21] Adding explicit case for EQ --- .../java/org/apache/iceberg/transforms/ProjectionUtil.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java index 112a09441fcf..5df55e512294 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java +++ b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java @@ -86,6 +86,9 @@ static UnboundPredicate truncateIntegerStrict( } case NOT_EQ: return predicate(Expression.Operation.NOT_EQ, name, transform.apply(boundary)); + case EQ: + // there is no predicate that guarantees equality because adjacent ints transform to the same value + return null; default: return null; } @@ -125,6 +128,9 @@ static UnboundPredicate truncateLongStrict( } case NOT_EQ: return predicate(Expression.Operation.NOT_EQ, name, transform.apply(boundary)); + case EQ: + // there is no predicate that guarantees equality because adjacent longs transform to the same value + return null; default: return null; } From ad1d7a784cc1d536f9c6d023fce432be254f0b9c Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Thu, 25 Jul 2019 14:50:15 -0700 Subject: [PATCH 16/21] Add examples --- .../iceberg/transforms/ProjectionUtil.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java index 5df55e512294..81781c5bb3a0 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java +++ b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java @@ -59,29 +59,45 @@ static UnboundPredicate truncateIntegerStrict( case LT: // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1).equals(transform.apply(boundary))) { + // Literal is not at lower boundary, for eg: 2019-07-02T02:12:34.0000 + // the predicate can be < 2019-07-02 return predicate(Expression.Operation.LT, name, transform.apply(boundary - 1)); } else { + // Literal was at the lower boundary, for eg: 2019-07-02T00:00:00.0000 + // the predicate can be <= 2019-07-01 return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary - 1)); } case LT_EQ: // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1).equals(transform.apply(boundary))) { + // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 + // the predicate can be < 2019-07-02 return predicate(Expression.Operation.LT, name, transform.apply(boundary)); } else { + // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 + // the predicate can be <= 2019-07-02 return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary)); } case GT: // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1).equals(transform.apply(boundary))) { + // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 + // the predicate can be > 2019-07-02 return predicate(Expression.Operation.GT, name, transform.apply(boundary + 1)); } else { + // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 + // the predicate can be >= 2019-07-03 return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary + 1)); } case GT_EQ: // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1).equals(transform.apply(boundary))) { + // Literal is not at lower boundary, for eg: 2019-07-02T02:12:34.0000 + // the predicate can be > 2019-07-02 return predicate(Expression.Operation.GT, name, transform.apply(boundary)); } else { + // Literal was at the lower boundary, for eg: 2019-07-02T00:00:00.0000 + // the predicate can be >= 2019-07-02 return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary)); } case NOT_EQ: @@ -101,29 +117,45 @@ static UnboundPredicate truncateLongStrict( case LT: // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1L).equals(transform.apply(boundary))) { + // Literal is not at lower boundary, for eg: 2019-07-02T02:12:34.0000 + // the predicate can be < 2019-07-02 return predicate(Expression.Operation.LT, name, transform.apply(boundary - 1L)); } else { + // Literal was at the lower boundary, for eg: 2019-07-02T00:00:00.0000 + // the predicate can be <= 2019-07-01 return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary - 1L)); } case LT_EQ: // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { + // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 + // the predicate can be < 2019-07-02 return predicate(Expression.Operation.LT, name, transform.apply(boundary)); } else { + // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 + // the predicate can be <= 2019-07-02 return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary)); } case GT: // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { + // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 + // the predicate can be > 2019-07-02 return predicate(Expression.Operation.GT, name, transform.apply(boundary + 1L)); } else { + // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 + // the predicate can be >= 2019-07-03 return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary + 1L)); } case GT_EQ: // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1L).equals(transform.apply(boundary))) { + // Literal is not at lower boundary, for eg: 2019-07-02T02:12:34.0000 + // the predicate can be > 2019-07-02 return predicate(Expression.Operation.GT, name, transform.apply(boundary)); } else { + // Literal was at the lower boundary, for eg: 2019-07-02T00:00:00.0000 + // the predicate can be >= 2019-07-02 return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary)); } case NOT_EQ: From 40e14715fd56c31943e3f2b5179b9cb705e08585 Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Thu, 25 Jul 2019 15:58:46 -0700 Subject: [PATCH 17/21] Simplifying the LT and GT predicates Simplifying the LT and GT predicates and updating tests to reflect that --- .../iceberg/transforms/ProjectionUtil.java | 56 ++++--------------- .../transforms/TestDatesProjection.java | 8 +-- .../transforms/TestTimestampsProjection.java | 16 +++--- 3 files changed, 24 insertions(+), 56 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java index 81781c5bb3a0..18cc86b38211 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java +++ b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java @@ -52,21 +52,13 @@ static UnboundPredicate truncateInteger( } } - static UnboundPredicate truncateIntegerStrict( - String name, BoundPredicate pred, Transform transform) { + static UnboundPredicate truncateIntegerStrict( + String name, BoundPredicate pred, Transform transform) { int boundary = pred.literal().value(); switch (pred.op()) { case LT: - // Checking if the literal is at the lower partition boundary - if (transform.apply(boundary - 1).equals(transform.apply(boundary))) { - // Literal is not at lower boundary, for eg: 2019-07-02T02:12:34.0000 - // the predicate can be < 2019-07-02 - return predicate(Expression.Operation.LT, name, transform.apply(boundary - 1)); - } else { - // Literal was at the lower boundary, for eg: 2019-07-02T00:00:00.0000 - // the predicate can be <= 2019-07-01 - return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary - 1)); - } + // predicate would be <= the previous partition + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); case LT_EQ: // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1).equals(transform.apply(boundary))) { @@ -79,16 +71,8 @@ static UnboundPredicate truncateIntegerStrict( return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary)); } case GT: - // Checking if the literal is at the upper partition boundary - if (transform.apply(boundary + 1).equals(transform.apply(boundary))) { - // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 - // the predicate can be > 2019-07-02 - return predicate(Expression.Operation.GT, name, transform.apply(boundary + 1)); - } else { - // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 - // the predicate can be >= 2019-07-03 - return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary + 1)); - } + // predicate would be >= the next partition + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); case GT_EQ: // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1).equals(transform.apply(boundary))) { @@ -110,21 +94,13 @@ static UnboundPredicate truncateIntegerStrict( } } - static UnboundPredicate truncateLongStrict( - String name, BoundPredicate pred, Transform transform) { + static UnboundPredicate truncateLongStrict( + String name, BoundPredicate pred, Transform transform) { long boundary = pred.literal().value(); switch (pred.op()) { case LT: - // Checking if the literal is at the lower partition boundary - if (transform.apply(boundary - 1L).equals(transform.apply(boundary))) { - // Literal is not at lower boundary, for eg: 2019-07-02T02:12:34.0000 - // the predicate can be < 2019-07-02 - return predicate(Expression.Operation.LT, name, transform.apply(boundary - 1L)); - } else { - // Literal was at the lower boundary, for eg: 2019-07-02T00:00:00.0000 - // the predicate can be <= 2019-07-01 - return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary - 1L)); - } + // predicate would be <= the previous partition + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); case LT_EQ: // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { @@ -137,16 +113,8 @@ static UnboundPredicate truncateLongStrict( return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary)); } case GT: - // Checking if the literal is at the upper partition boundary - if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { - // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 - // the predicate can be > 2019-07-02 - return predicate(Expression.Operation.GT, name, transform.apply(boundary + 1L)); - } else { - // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 - // the predicate can be >= 2019-07-03 - return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary + 1L)); - } + // predicate would be >= the next partition + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); case GT_EQ: // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1L).equals(transform.apply(boundary))) { diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java index dbdbd8e31d7b..21e8341e0345 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -78,7 +78,7 @@ public void testMonthStrictLowerBound() { assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016-12"); assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT, "2017-01"); - assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT, "2017-01"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-02"); // bound should include the same month for lower bound assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01"); @@ -90,7 +90,7 @@ public void testMonthStrictUpperBound() { Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); - assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT, "2017-12"); + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-11"); // bound should include the same month for upper bound assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12"); assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); @@ -160,7 +160,7 @@ public void testYearStrictLowerBound() { assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT, "2017"); - assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT, "2017"); + assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); // bound should include the same year for lower bound assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); @@ -172,7 +172,7 @@ public void testYearStrictUpperBound() { Integer date = (Integer) Literal.of("2017-12-31").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); - assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT, "2017"); + assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); // bound should include the same year for upper bound assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017"); assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index d8d8261e9474..0f07e5596225 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -78,7 +78,7 @@ public void testMonthStrictLowerBound() { assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017-12"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); // bound should include the same month for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); @@ -90,7 +90,7 @@ public void testMonthStrictUpperBound() { Long date = (long) Literal.of("2017-12-31T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT, "2017-12"); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); // bound should include the same month for upper bound @@ -132,7 +132,7 @@ public void testDayStrictLowerBound() { assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12-01"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017-12-01"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); // bound should include the same day for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); @@ -144,7 +144,7 @@ public void testDayStrictUpperBound() { Long date = (long) Literal.of("2017-12-01T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT, "2017-12-01"); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); // bound should include the same day for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); @@ -186,7 +186,7 @@ public void testYearStrictLowerBound() { assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); // bound should include the same year for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); @@ -198,7 +198,7 @@ public void testYearStrictUpperBound() { Long date = (long) Literal.of("2017-12-31T23:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT, "2017"); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); // bound should include the same year for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); @@ -240,7 +240,7 @@ public void testHourStrictLowerBound() { assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12-01-10"); - assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT, "2017-12-01-10"); + assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); // bound should include the same hour for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); @@ -252,7 +252,7 @@ public void testHourStrictUpperBound() { Long date = (long) Literal.of("2017-12-01T10:59:59.999999").to(TYPE).value(); PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); - assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT, "2017-12-01-10"); + assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); // bound should include the same hour for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); From ba3262e4651582b8de7787c69ffa1cea49085afc Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Fri, 26 Jul 2019 12:14:14 -0700 Subject: [PATCH 18/21] Getting rid of white space --- .../transforms/TestDatesProjection.java | 13 ------------ .../transforms/TestTimestampsProjection.java | 20 ------------------- 2 files changed, 33 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java index 21e8341e0345..afeee2bd2d6c 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -54,7 +54,6 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte Dates transform = (Dates) spec.getFieldsBySourceId(1).get(0).transform(); String output = transform.toHumanString((int) literal.value()); Assert.assertEquals(expectedLiteral, output); - } public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, @@ -68,7 +67,6 @@ public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate fi Dates transform = (Dates) spec.getFieldsBySourceId(1).get(0).transform(); String output = transform.toHumanString((int) literal.value()); Assert.assertEquals(expectedLiteral, output); - } @Test @@ -82,7 +80,6 @@ public void testMonthStrictLowerBound() { // bound should include the same month for lower bound assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01"); - } @Test @@ -96,7 +93,6 @@ public void testMonthStrictUpperBound() { assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT, "2017-12"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-12"); - } @Test @@ -109,7 +105,6 @@ public void testMonthInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-12"); - } @Test @@ -122,7 +117,6 @@ public void testMonthInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-12"); - } @Test @@ -137,7 +131,6 @@ public void testDayStrict() { // should be the same date for >= assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01-01"); - } @Test @@ -150,7 +143,6 @@ public void testDayInclusive() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-01-02"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01-01"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-01-01"); - } @Test @@ -164,7 +156,6 @@ public void testYearStrictLowerBound() { // bound should include the same year for lower bound assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); - } @Test @@ -178,7 +169,6 @@ public void testYearStrictUpperBound() { assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT, "2017"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); - } @Test @@ -191,7 +181,6 @@ public void testYearInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017"); - } @Test @@ -204,7 +193,5 @@ public void testYearInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017"); - } - } diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index 0f07e5596225..221c33789d32 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -54,7 +54,6 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte Timestamps transform = (Timestamps) spec.getFieldsBySourceId(1).get(0).transform(); String output = transform.toHumanString((int) literal.value()); Assert.assertEquals(expectedLiteral, output); - } public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, @@ -68,7 +67,6 @@ public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate fi Timestamps transform = (Timestamps) spec.getFieldsBySourceId(1).get(0).transform(); String output = transform.toHumanString((int) literal.value()); Assert.assertEquals(expectedLiteral, output); - } @Test @@ -82,7 +80,6 @@ public void testMonthStrictLowerBound() { // bound should include the same month for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); - } @Test @@ -96,7 +93,6 @@ public void testMonthStrictUpperBound() { // bound should include the same month for upper bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); - } @Test @@ -109,7 +105,6 @@ public void testMonthInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12"); - } @Test @@ -122,7 +117,6 @@ public void testMonthInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12"); - } @Test @@ -136,7 +130,6 @@ public void testDayStrictLowerBound() { // bound should include the same day for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); - } @Test @@ -150,7 +143,6 @@ public void testDayStrictUpperBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); - } @Test @@ -163,7 +155,6 @@ public void testDayInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01"); - } @Test @@ -176,7 +167,6 @@ public void testDayInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01"); - } @Test @@ -190,7 +180,6 @@ public void testYearStrictLowerBound() { // bound should include the same year for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); - } @Test @@ -204,7 +193,6 @@ public void testYearStrictUpperBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); - } @Test @@ -217,7 +205,6 @@ public void testYearInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017"); - } @Test @@ -230,7 +217,6 @@ public void testYearInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017"); - } @Test @@ -244,7 +230,6 @@ public void testHourStrictLowerBound() { // bound should include the same hour for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); - } @Test @@ -258,7 +243,6 @@ public void testHourStrictUpperBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12-01-10"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); - } @Test @@ -271,7 +255,6 @@ public void testHourInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01-10"); - } @Test @@ -284,8 +267,5 @@ public void testHourInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01-10"); - } - - } From 0722e2265b5629f8f827b9d2c742de75728d43ee Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Fri, 26 Jul 2019 12:42:09 -0700 Subject: [PATCH 19/21] Making all the predicates either LT_EQ or GT_EQ --- .../iceberg/transforms/ProjectionUtil.java | 16 ++++++------- .../transforms/TestDatesProjection.java | 12 ++++------ .../transforms/TestTimestampsProjection.java | 24 +++++++------------ 3 files changed, 20 insertions(+), 32 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java index 18cc86b38211..5ba16dd63d1e 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java +++ b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java @@ -63,8 +63,8 @@ static UnboundPredicate truncateIntegerStrict( // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1).equals(transform.apply(boundary))) { // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 - // the predicate can be < 2019-07-02 - return predicate(Expression.Operation.LT, name, transform.apply(boundary)); + // the predicate can be < 2019-07-01 + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); } else { // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 // the predicate can be <= 2019-07-02 @@ -77,8 +77,8 @@ static UnboundPredicate truncateIntegerStrict( // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1).equals(transform.apply(boundary))) { // Literal is not at lower boundary, for eg: 2019-07-02T02:12:34.0000 - // the predicate can be > 2019-07-02 - return predicate(Expression.Operation.GT, name, transform.apply(boundary)); + // the predicate can be >= 2019-07-03 + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); } else { // Literal was at the lower boundary, for eg: 2019-07-02T00:00:00.0000 // the predicate can be >= 2019-07-02 @@ -105,8 +105,8 @@ static UnboundPredicate truncateLongStrict( // Checking if the literal is at the upper partition boundary if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 - // the predicate can be < 2019-07-02 - return predicate(Expression.Operation.LT, name, transform.apply(boundary)); + // the predicate can be <= 2019-07-01 + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) -1); } else { // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 // the predicate can be <= 2019-07-02 @@ -119,8 +119,8 @@ static UnboundPredicate truncateLongStrict( // Checking if the literal is at the lower partition boundary if (transform.apply(boundary - 1L).equals(transform.apply(boundary))) { // Literal is not at lower boundary, for eg: 2019-07-02T02:12:34.0000 - // the predicate can be > 2019-07-02 - return predicate(Expression.Operation.GT, name, transform.apply(boundary)); + // the predicate can be >= 2019-07-03 + return predicate(Expression.Operation.GT_EQ, name, transform.apply(boundary) + 1); } else { // Literal was at the lower boundary, for eg: 2019-07-02T00:00:00.0000 // the predicate can be >= 2019-07-02 diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java index afeee2bd2d6c..3fb65e323bb7 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -75,9 +75,8 @@ public void testMonthStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016-12"); - assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT, "2017-01"); + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2016-12"); assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-02"); - // bound should include the same month for lower bound assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01"); } @@ -88,10 +87,9 @@ public void testMonthStrictUpperBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("date").build(); assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2017-11"); - // bound should include the same month for upper bound assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017-12"); assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); - assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT, "2017-12"); + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-12"); } @@ -151,9 +149,8 @@ public void testYearStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); - assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT, "2017"); + assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2016"); assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); - // bound should include the same year for lower bound assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); } @@ -164,10 +161,9 @@ public void testYearStrictUpperBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("date").build(); assertProjectionStrict(spec, lessThan("date", date), Expression.Operation.LT_EQ, "2016"); - // bound should include the same year for upper bound assertProjectionStrict(spec, lessThanOrEqual("date", date), Expression.Operation.LT_EQ, "2017"); assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); - assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT, "2017"); + assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); } diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index 221c33789d32..f472cf87cd51 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -75,9 +75,8 @@ public void testMonthStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).month("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); - // bound should include the same month for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); } @@ -90,8 +89,7 @@ public void testMonthStrictUpperBound() { assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11"); assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); - // bound should include the same month for upper bound - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); } @@ -125,9 +123,8 @@ public void testDayStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12-01"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); - // bound should include the same day for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); } @@ -138,10 +135,9 @@ public void testDayStrictUpperBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).day("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-11-30"); - // bound should include the same day for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12-01"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); } @@ -175,9 +171,8 @@ public void testYearStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2016"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); - // bound should include the same year for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); } @@ -188,10 +183,9 @@ public void testYearStrictUpperBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).year("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2016"); - // bound should include the same year for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); } @@ -225,9 +219,8 @@ public void testHourStrictLowerBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); - assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT, "2017-12-01-10"); + assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); - // bound should include the same hour for lower bound assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); } @@ -238,10 +231,9 @@ public void testHourStrictUpperBound() { PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).hour("timestamp").build(); assertProjectionStrict(spec, lessThan("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-09"); - // bound should include the same hour for upper bound assertProjectionStrict(spec, lessThanOrEqual("timestamp", date), Expression.Operation.LT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); - assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT, "2017-12-01-10"); + assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); } From 30f35947c964b63821689df8eb48e1628191b88e Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Fri, 26 Jul 2019 13:25:32 -0700 Subject: [PATCH 20/21] Adding equal and notEqual tests for all --- .../iceberg/expressions/Projections.java | 14 +++++---- .../transforms/TestDatesProjection.java | 24 +++++++++++++++ .../transforms/TestTimestampsProjection.java | 30 +++++++++++++++++++ 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/expressions/Projections.java b/api/src/main/java/org/apache/iceberg/expressions/Projections.java index 50d0693d4590..f800b350858e 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/Projections.java +++ b/api/src/main/java/org/apache/iceberg/expressions/Projections.java @@ -221,9 +221,10 @@ public Expression predicate(BoundPredicate pred) { // similarly, if partitioning by day(ts) and hour(ts), the more restrictive // projection should be used. ts = 2019-01-01T01:00:00 produces day=2019-01-01 and // hour=2019-01-01-01. the value will be in 2019-01-01-01 and not in 2019-01-01-02. - result = Expressions.and( - result, - ((Transform) part.transform()).project(part.name(), pred)); + UnboundPredicate inclusiveProjection = ((Transform) part.transform()).project(part.name(), pred); + if (inclusiveProjection != null) { + result = Expressions.and(result, inclusiveProjection); + } } return result; @@ -251,9 +252,10 @@ public Expression predicate(BoundPredicate pred) { // any timestamp where either projection predicate is true must match the original // predicate. For example, ts = 2019-01-01T03:00:00 matches the hour projection but not // the day, but does match the original predicate. - result = Expressions.or( - result, - ((Transform) part.transform()).projectStrict(part.name(), pred)); + UnboundPredicate strictProjection = ((Transform) part.transform()).projectStrict(part.name(), pred); + if (strictProjection != null) { + result = Expressions.or(result, strictProjection); + } } return result; diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java index 3fb65e323bb7..7b4ee7844b5b 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java @@ -56,6 +56,20 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte Assert.assertEquals(expectedLiteral, output); } + public void assertProjectionStrictValue(PartitionSpec spec, UnboundPredicate filter, + Expression.Operation expectedOp) { + + Expression projection = Projections.strict(spec).project(filter); + Assert.assertEquals(projection.op(), expectedOp); + } + + public void assertProjectionInclusiveValue(PartitionSpec spec, UnboundPredicate filter, + Expression.Operation expectedOp) { + + Expression projection = Projections.inclusive(spec).project(filter); + Assert.assertEquals(projection.op(), expectedOp); + } + public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { Expression projection = Projections.inclusive(spec).project(filter); @@ -79,6 +93,7 @@ public void testMonthStrictLowerBound() { assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-02"); assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01"); + assertProjectionStrictValue(spec, equal("date", date), Expression.Operation.FALSE); } @Test @@ -91,6 +106,7 @@ public void testMonthStrictUpperBound() { assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-12"); + assertProjectionStrictValue(spec, equal("date", date), Expression.Operation.FALSE); } @Test @@ -103,6 +119,7 @@ public void testMonthInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-12"); + assertProjectionInclusiveValue(spec, notEqual("date", date), Expression.Operation.TRUE); } @Test @@ -115,6 +132,7 @@ public void testMonthInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-12"); + assertProjectionInclusiveValue(spec, notEqual("date", date), Expression.Operation.TRUE); } @Test @@ -129,6 +147,7 @@ public void testDayStrict() { // should be the same date for >= assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01-01"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017-01-01"); + assertProjectionStrictValue(spec, equal("date", date), Expression.Operation.FALSE); } @Test @@ -141,6 +160,7 @@ public void testDayInclusive() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017-01-02"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017-01-01"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017-01-01"); + assertProjectionInclusiveValue(spec, notEqual("date", date), Expression.Operation.TRUE); } @Test @@ -153,6 +173,7 @@ public void testYearStrictLowerBound() { assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); + assertProjectionStrictValue(spec, equal("date", date), Expression.Operation.FALSE); } @Test @@ -165,6 +186,7 @@ public void testYearStrictUpperBound() { assertProjectionStrict(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, notEqual("date", date), Expression.Operation.NOT_EQ, "2017"); + assertProjectionStrictValue(spec, equal("date", date), Expression.Operation.FALSE); } @Test @@ -177,6 +199,7 @@ public void testYearInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017"); + assertProjectionInclusiveValue(spec, notEqual("date", date), Expression.Operation.TRUE); } @Test @@ -189,5 +212,6 @@ public void testYearInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("date", date), Expression.Operation.GT_EQ, "2018"); assertProjectionInclusive(spec, greaterThanOrEqual("date", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, equal("date", date), Expression.Operation.EQ, "2017"); + assertProjectionInclusiveValue(spec, notEqual("date", date), Expression.Operation.TRUE); } } diff --git a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java index f472cf87cd51..5d3c5b09085f 100644 --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestampsProjection.java @@ -56,6 +56,20 @@ public void assertProjectionStrict(PartitionSpec spec, UnboundPredicate filte Assert.assertEquals(expectedLiteral, output); } + public void assertProjectionStrictValue(PartitionSpec spec, UnboundPredicate filter, + Expression.Operation expectedOp) { + + Expression projection = Projections.strict(spec).project(filter); + Assert.assertEquals(projection.op(), expectedOp); + } + + public void assertProjectionInclusiveValue(PartitionSpec spec, UnboundPredicate filter, + Expression.Operation expectedOp) { + + Expression projection = Projections.inclusive(spec).project(filter); + Assert.assertEquals(projection.op(), expectedOp); + } + public void assertProjectionInclusive(PartitionSpec spec, UnboundPredicate filter, Expression.Operation expectedOp, String expectedLiteral) { Expression projection = Projections.inclusive(spec).project(filter); @@ -79,6 +93,7 @@ public void testMonthStrictLowerBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); + assertProjectionStrictValue(spec, equal("timestamp", date), Expression.Operation.FALSE); } @Test @@ -91,6 +106,7 @@ public void testMonthStrictUpperBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12"); + assertProjectionStrictValue(spec, equal("timestamp", date), Expression.Operation.FALSE); } @Test @@ -103,6 +119,7 @@ public void testMonthInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12"); + assertProjectionInclusiveValue(spec, notEqual("timestamp", date), Expression.Operation.TRUE); } @Test @@ -115,6 +132,7 @@ public void testMonthInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12"); + assertProjectionInclusiveValue(spec, notEqual("timestamp", date), Expression.Operation.TRUE); } @Test @@ -127,6 +145,7 @@ public void testDayStrictLowerBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); + assertProjectionStrictValue(spec, equal("timestamp", date), Expression.Operation.FALSE); } @Test @@ -139,6 +158,7 @@ public void testDayStrictUpperBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01"); + assertProjectionStrictValue(spec, equal("timestamp", date), Expression.Operation.FALSE); } @Test @@ -151,6 +171,7 @@ public void testDayInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01"); + assertProjectionInclusiveValue(spec, notEqual("timestamp", date), Expression.Operation.TRUE); } @Test @@ -163,6 +184,7 @@ public void testDayInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-02"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01"); + assertProjectionInclusiveValue(spec, notEqual("timestamp", date), Expression.Operation.TRUE); } @Test @@ -175,6 +197,7 @@ public void testYearStrictLowerBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); + assertProjectionStrictValue(spec, equal("timestamp", date), Expression.Operation.FALSE); } @Test @@ -187,6 +210,7 @@ public void testYearStrictUpperBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017"); + assertProjectionStrictValue(spec, equal("timestamp", date), Expression.Operation.FALSE); } @Test @@ -199,6 +223,7 @@ public void testYearInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017"); + assertProjectionInclusiveValue(spec, notEqual("timestamp", date), Expression.Operation.TRUE); } @Test @@ -211,6 +236,7 @@ public void testYearInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2018"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017"); + assertProjectionInclusiveValue(spec, notEqual("timestamp", date), Expression.Operation.TRUE); } @Test @@ -223,6 +249,7 @@ public void testHourStrictLowerBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); + assertProjectionStrictValue(spec, equal("timestamp", date), Expression.Operation.FALSE); } @Test @@ -235,6 +262,7 @@ public void testHourStrictUpperBound() { assertProjectionStrict(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionStrict(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionStrict(spec, notEqual("timestamp", date), Expression.Operation.NOT_EQ, "2017-12-01-10"); + assertProjectionStrictValue(spec, equal("timestamp", date), Expression.Operation.FALSE); } @Test @@ -247,6 +275,7 @@ public void testHourInclusiveLowerBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01-10"); + assertProjectionInclusiveValue(spec, notEqual("timestamp", date), Expression.Operation.TRUE); } @Test @@ -259,5 +288,6 @@ public void testHourInclusiveUpperBound() { assertProjectionInclusive(spec, greaterThan("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-11"); assertProjectionInclusive(spec, greaterThanOrEqual("timestamp", date), Expression.Operation.GT_EQ, "2017-12-01-10"); assertProjectionInclusive(spec, equal("timestamp", date), Expression.Operation.EQ, "2017-12-01-10"); + assertProjectionInclusiveValue(spec, notEqual("timestamp", date), Expression.Operation.TRUE); } } From dc15fbb23b99b1ef83bf199b124091d260ea799a Mon Sep 17 00:00:00 2001 From: Mouli Mukherjee Date: Fri, 26 Jul 2019 13:47:17 -0700 Subject: [PATCH 21/21] Fixing style error --- .../main/java/org/apache/iceberg/transforms/ProjectionUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java index 5ba16dd63d1e..ef1e0c3591ed 100644 --- a/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java +++ b/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java @@ -106,7 +106,7 @@ static UnboundPredicate truncateLongStrict( if (transform.apply(boundary + 1L).equals(transform.apply(boundary))) { // Literal is not at upper boundary, for eg: 2019-07-02T02:12:34.0000 // the predicate can be <= 2019-07-01 - return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) -1); + return predicate(Expression.Operation.LT_EQ, name, transform.apply(boundary) - 1); } else { // Literal is not at upper boundary, for eg: 2019-07-02T23:59:59.99999 // the predicate can be <= 2019-07-02