diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java index 95030b5b9053..d3251bac7bf5 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java +++ b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java @@ -55,6 +55,7 @@ import org.apache.calcite.util.DateString; import org.apache.calcite.util.NlsString; import org.apache.calcite.util.Sarg; +import org.apache.calcite.util.SqlConstantComparator; import org.apache.calcite.util.TimeString; import org.apache.calcite.util.TimeWithTimeZoneString; import org.apache.calcite.util.TimestampString; @@ -1996,6 +1997,8 @@ public RexNode makeBetween(RexNode arg, RexNode lower, RexNode upper) { final Comparable upperValue = toComparable(Comparable.class, upper); if (lowerValue != null && upperValue != null + && !SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(lowerValue) + && !SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(upperValue) && areAssignable(arg, Arrays.asList(lower, upper))) { final Sarg sarg = Sarg.of(RexUnknownAs.UNKNOWN, @@ -2028,6 +2031,9 @@ && areAssignable(arg, Arrays.asList(lower, upper))) { if (value == null) { return null; } + if (SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(value)) { + return null; + } rangeSet.add(Range.singleton(value)); } return Sarg.of(unknownAs, rangeSet); diff --git a/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java b/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java index 549efa5387f6..02508281bda2 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java +++ b/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java @@ -27,6 +27,7 @@ import org.apache.calcite.util.NlsString; import org.apache.calcite.util.RangeSets; import org.apache.calcite.util.Sarg; +import org.apache.calcite.util.SqlConstantComparator; import org.apache.calcite.util.TimeString; import org.apache.calcite.util.TimestampString; import org.apache.calcite.util.Util; @@ -481,8 +482,7 @@ private static Comparable compare(List values, IntPredicate p) { if (v1 instanceof Number) { v1 = number(v1); } - //noinspection unchecked - final int c = v0.compareTo(v1); + final int c = SqlConstantComparator.INSTANCE.compare(v0, v1); return p.test(c); } diff --git a/core/src/main/java/org/apache/calcite/rex/RexLiteral.java b/core/src/main/java/org/apache/calcite/rex/RexLiteral.java index 545e81995d0d..9fb6e55af0f6 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexLiteral.java +++ b/core/src/main/java/org/apache/calcite/rex/RexLiteral.java @@ -60,11 +60,6 @@ import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.text.SimpleDateFormat; -import java.time.Instant; -import java.time.LocalDateTime; -import java.time.ZoneId; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; import java.util.Calendar; import java.util.List; import java.util.Locale; @@ -1189,17 +1184,7 @@ public boolean isNull() { case TIMESTAMP_TZ: if (clazz == Long.class) { TimestampWithTimeZoneString tstz = (TimestampWithTimeZoneString) value; - long ms = tstz - .getLocalTimestampString() - .getMillisSinceEpoch(); - // Interpret the timestamp part as a UTC timestamp - LocalDateTime local = Instant.ofEpochMilli(ms).atZone(ZoneOffset.UTC).toLocalDateTime(); - // Adjust for the time zone - ZoneId id = tstz.getTimeZone().toZoneId(); - ZonedDateTime zoned = local.atZone(id); - ZonedDateTime utc = zoned.withZoneSameInstant(ZoneOffset.UTC); - ms = utc.toInstant().toEpochMilli(); - return clazz.cast(ms); + return clazz.cast(tstz.getMillisSinceEpoch()); } else if (clazz == Calendar.class) { TimestampWithTimeZoneString ts = (TimestampWithTimeZoneString) value; return clazz.cast(ts.getLocalTimestampString().toCalendar(ts.getTimeZone())); diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java index 4d7702295bff..8fa0a7836376 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -38,6 +38,7 @@ import org.apache.calcite.util.Pair; import org.apache.calcite.util.RangeSets; import org.apache.calcite.util.Sarg; +import org.apache.calcite.util.SqlConstantComparator; import org.apache.calcite.util.Util; import com.google.common.collect.ArrayListMultimap; @@ -747,7 +748,7 @@ private > RexNode simplifyComparison(RexCall e, ? rexBuilder.makeLiteral(false) : rexBuilder.makeNullLiteral(e.getType()); } - final int comparisonResult = v0.compareTo(v1); + final int comparisonResult = SqlConstantComparator.INSTANCE.compare(v0, v1); switch (e.getKind()) { case EQUALS: return rexBuilder.makeLiteral(comparisonResult == 0); @@ -1866,7 +1867,8 @@ private > RexNode simplifyAnd2ForUnknownAsFalse( if (comparison != null && comparison.kind != SqlKind.NOT_EQUALS) { // not supported yet final C v0 = comparison.literal.getValueAs(clazz); - if (v0 != null) { + if (v0 != null + && !SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(v0)) { final RexNode result = processRange(rexBuilder, terms, rangeTerms, predicate, comparison.ref, v0, comparison.kind); @@ -1961,12 +1963,14 @@ private > RexNode simplifyAnd2ForUnknownAsFalse( if (constant == null) { break; } - final RexNode result = - processRange(rexBuilder, terms, rangeTerms, - term, comparison.ref, constant, comparison.kind); - if (result != null) { - // Not satisfiable - return result; + if (!SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(constant)) { + final RexNode result = + processRange(rexBuilder, terms, rangeTerms, + term, comparison.ref, constant, comparison.kind); + if (result != null) { + // Not satisfiable + return result; + } } } break; @@ -2091,6 +2095,9 @@ private > RexNode simplifyUsingPredicates(RexNode e, if (v0 == null) { return e; } + if (SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(v0)) { + return e; + } final RangeSet rangeSet = rangeSet(comparison.kind, v0); final RangeSet rangeSet2 = residue(comparison.ref, rangeSet, predicates.pulledUpPredicates, @@ -2283,10 +2290,9 @@ private RexNode simplifyOrs(List terms, RexUnknownAs unknownAs) { final Comparable comparable1 = notEqualsComparison.literal.getValue(); final Comparable comparable2 = castNonNull(Comparison.of(prevNotEquals)).literal.getValue(); - //noinspection unchecked if (comparable1 != null && comparable2 != null - && comparable1.compareTo(comparable2) != 0) { + && SqlConstantComparator.INSTANCE.compare(comparable1, comparable2) != 0) { // X <> A OR X <> B => X IS NOT NULL OR NULL final RexNode isNotNull = rexBuilder.makeCall(RexUtil.getPos(term), SqlStdOperatorTable.IS_NOT_NULL, @@ -3073,6 +3079,16 @@ private static boolean constantsEquivalent(RexNode node1, RexNode node2) { if (Objects.equals(node1, node2)) { return true; } + if (node1 instanceof RexLiteral && node2 instanceof RexLiteral + && SqlTypeUtil.equalSansNullability(node1.getType(), node2.getType())) { + final Comparable value1 = ((RexLiteral) node1).getValueAs(Comparable.class); + final Comparable value2 = ((RexLiteral) node2).getValueAs(Comparable.class); + return value1 != null + && value2 != null + && SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(value1) + && SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(value2) + && SqlConstantComparator.INSTANCE.compare(value1, value2) == 0; + } if (!(node1 instanceof RexCall) || !(node2 instanceof RexCall)) { return false; } @@ -3437,6 +3453,9 @@ private boolean accept2b(SqlParserPos pos, RexNode e, SqlKind kind, } final Comparable value = requireNonNull(literal.getValueAs(Comparable.class), "value"); + if (SqlConstantComparator.INSTANCE.hasCustomSqlConstantComparison(value)) { + return false; + } switch (kind) { case LESS_THAN: b.addRange(Range.lessThan(value), literal.getType()); diff --git a/core/src/main/java/org/apache/calcite/util/ComparableSqlConstant.java b/core/src/main/java/org/apache/calcite/util/ComparableSqlConstant.java new file mode 100644 index 000000000000..f6e917d0f9bb --- /dev/null +++ b/core/src/main/java/org/apache/calcite/util/ComparableSqlConstant.java @@ -0,0 +1,28 @@ +/* + * 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.calcite.util; + +/** + * Constant expression representation whose SQL comparison semantics differ + * from its natural ordering. + * + * @param Type of constant to compare against + */ +public interface ComparableSqlConstant { + /** Compares this constant to another constant using SQL semantics. */ + int compareSqlConstantTo(T o); +} diff --git a/core/src/main/java/org/apache/calcite/util/SqlConstantComparator.java b/core/src/main/java/org/apache/calcite/util/SqlConstantComparator.java new file mode 100644 index 000000000000..27184e8b3200 --- /dev/null +++ b/core/src/main/java/org/apache/calcite/util/SqlConstantComparator.java @@ -0,0 +1,42 @@ +/* + * 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.calcite.util; + +import java.util.Comparator; + +/** Compares constants using SQL semantics. */ +public final class SqlConstantComparator implements Comparator { + /** Singleton instance. */ + public static final SqlConstantComparator INSTANCE = new SqlConstantComparator(); + + private SqlConstantComparator() { + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + @Override public int compare(Comparable v0, Comparable v1) { + if (hasCustomSqlConstantComparison(v0) + && v0.getClass() == v1.getClass()) { + return ((ComparableSqlConstant) v0).compareSqlConstantTo(v1); + } + return v0.compareTo(v1); + } + + /** Returns whether a constant should use custom SQL comparison. */ + public boolean hasCustomSqlConstantComparison(Comparable v) { + return v instanceof ComparableSqlConstant; + } +} diff --git a/core/src/main/java/org/apache/calcite/util/TimestampWithTimeZoneString.java b/core/src/main/java/org/apache/calcite/util/TimestampWithTimeZoneString.java index 52e9eb6d4f05..9ae83628b0bb 100644 --- a/core/src/main/java/org/apache/calcite/util/TimestampWithTimeZoneString.java +++ b/core/src/main/java/org/apache/calcite/util/TimestampWithTimeZoneString.java @@ -41,7 +41,8 @@ * and can support unlimited precision (milliseconds, nanoseconds). */ public class TimestampWithTimeZoneString - implements Comparable { + implements Comparable, + ComparableSqlConstant { final TimestampString localDateTime; final TimeZone timeZone; @@ -173,7 +174,22 @@ public TimestampWithTimeZoneString withTimeZone(TimeZone timeZone) { } @Override public int compareTo(TimestampWithTimeZoneString o) { - return this.pt.getCalendar().compareTo(o.pt.getCalendar()); + final int c = compareToInstant(o); + return c != 0 ? c : v.compareTo(o.v); + } + + /** Compares this timestamp to another timestamp by instant. */ + public int compareToInstant(TimestampWithTimeZoneString o) { + return Long.compare(getMillisSinceEpoch(), o.getMillisSinceEpoch()); + } + + @Override public int compareSqlConstantTo(TimestampWithTimeZoneString o) { + return compareToInstant(o); + } + + /** Returns the number of milliseconds since the epoch. */ + public long getMillisSinceEpoch() { + return pt.getCalendar().getTimeInMillis(); } public TimestampWithTimeZoneString round(int precision) { diff --git a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java index e23679eaf5f5..c955c424c945 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java @@ -964,6 +964,48 @@ private void checkDate(RexLiteral literal) { assertThat(inCall.getKind(), is(SqlKind.SEARCH)); } + /** Test case for [CALCITE-7553] + * TIMESTAMP_TZ IN-list construction should not use Sarg ordering. */ + @Test void testMakeInTimestampWithTimeZoneDoesNotCreateSarg() { + final RelDataTypeFactory typeFactory = + new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT); + final RexBuilder rexBuilder = new RexBuilder(typeFactory); + final RelDataType timestampTzType = + typeFactory.createSqlType(SqlTypeName.TIMESTAMP_TZ, 0); + final RexNode input = rexBuilder.makeInputRef(timestampTzType, 0); + final RexLiteral pst = + rexBuilder.makeTimestampTzLiteral( + new TimestampWithTimeZoneString("1969-07-21 02:56:15 GMT-08:00"), 0); + final RexLiteral gmt = + rexBuilder.makeTimestampTzLiteral( + new TimestampWithTimeZoneString("1969-07-21 10:56:15 GMT"), 0); + + final RexNode inCall = rexBuilder.makeIn(input, ImmutableList.of(pst, gmt)); + + assertThat(inCall.getKind(), is(SqlKind.OR)); + } + + /** Test case for [CALCITE-7553] + * TIMESTAMP_TZ BETWEEN construction should not use Sarg ordering. */ + @Test void testMakeBetweenTimestampWithTimeZoneDoesNotCreateSarg() { + final RelDataTypeFactory typeFactory = + new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT); + final RexBuilder rexBuilder = new RexBuilder(typeFactory); + final RelDataType timestampTzType = + typeFactory.createSqlType(SqlTypeName.TIMESTAMP_TZ, 0); + final RexNode input = rexBuilder.makeInputRef(timestampTzType, 0); + final RexLiteral gmt = + rexBuilder.makeTimestampTzLiteral( + new TimestampWithTimeZoneString("1969-07-21 10:56:15 GMT"), 0); + final RexLiteral pst = + rexBuilder.makeTimestampTzLiteral( + new TimestampWithTimeZoneString("1969-07-21 02:56:15 GMT-08:00"), 0); + + final RexNode betweenCall = rexBuilder.makeBetween(input, gmt, pst); + + assertThat(betweenCall.getKind(), is(SqlKind.AND)); + } + /** * Test case for * [CALCITE-6989] diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java index 2bdc7a1d56c4..a497847c58c8 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java @@ -3413,6 +3413,63 @@ private SqlOperator getNoDeterministicOperator() { assertThat(timestampLTZChar1.equals(timestampLTZChar4), is(true)); } + /** Test case for [CALCITE-7553] + * TIMESTAMP_TZ natural ordering must not collapse distinct values. */ + @Test void testTimestampWithTimeZoneNaturalOrderingKeepsDistinctValues() { + final TimestampWithTimeZoneString pst = + new TimestampWithTimeZoneString("1969-07-21 02:56:15 GMT-08:00"); + final TimestampWithTimeZoneString gmt = + new TimestampWithTimeZoneString("1969-07-21 10:56:15 GMT"); + + assertNotEquals(pst, gmt); + + final TreeMap values = new TreeMap<>(); + values.put(pst, "pst"); + values.put(gmt, "gmt"); + + assertThat(values.keySet(), hasSize(2)); + } + + /** Test case for [CALCITE-7553] + * SQL comparison of TIMESTAMP_TZ literals uses instant semantics. */ + @Test void testTimestampWithTimeZoneSqlComparisonUsesInstant() { + final RexLiteral pst = + rexBuilder.makeTimestampTzLiteral( + new TimestampWithTimeZoneString("1969-07-21 02:56:15 GMT-08:00"), 0); + final RexLiteral gmt = + rexBuilder.makeTimestampTzLiteral( + new TimestampWithTimeZoneString("1969-07-21 10:56:15 GMT"), 0); + + checkSimplify(eq(pst, gmt), "true"); + checkSimplify(ne(pst, gmt), "false"); + checkSimplify(le(pst, gmt), "true"); + checkSimplify(ge(pst, gmt), "true"); + checkSimplify(lt(pst, gmt), "false"); + checkSimplify(gt(pst, gmt), "false"); + + assertThat(eval(eq(pst, gmt)), is((Comparable) true)); + assertThat(eval(ne(pst, gmt)), is((Comparable) false)); + } + + /** Test case for [CALCITE-7553] + * TIMESTAMP_TZ equivalent constants do not create contradictory filters. */ + @Test void testTimestampWithTimeZoneEquivalentConstantsDoNotContradict() { + final RelDataType timestampTzType = + typeFactory.createSqlType(SqlTypeName.TIMESTAMP_TZ, 0); + final RexNode input = input(timestampTzType, 0); + final RexLiteral pst = + rexBuilder.makeTimestampTzLiteral( + new TimestampWithTimeZoneString("1969-07-21 02:56:15 GMT-08:00"), 0); + final RexLiteral gmt = + rexBuilder.makeTimestampTzLiteral( + new TimestampWithTimeZoneString("1969-07-21 10:56:15 GMT"), 0); + final RexNode condition = and(eq(input, pst), eq(input, gmt)); + final RexNode simplified = + simplify.withParanoid(false).simplifyUnknownAs(condition, RexUnknownAs.FALSE); + + assertThat(simplified.isAlwaysFalse(), is(false)); + } + /** Test case for [CALCITE-7526] * Incorrect TIMESTAMP WITH TIME ZONE produces wrong error message. */ @Test void testMalformedTimezone() {