From 17d4401f58c78bd25db801e1a3a2bc25a1fb77d6 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 7 Nov 2022 20:11:35 +0100 Subject: [PATCH 1/7] Handle when max cannot be coerced in UnwrapCastInComparison The source type's max value is not guaranteed to be coercible into the target type. Back out rather than failing. --- .../rule/UnwrapCastInComparison.java | 93 ++++++++++--------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/UnwrapCastInComparison.java b/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/UnwrapCastInComparison.java index bd72b003c6b6..234d9a0a1712 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/UnwrapCastInComparison.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/UnwrapCastInComparison.java @@ -238,53 +238,60 @@ private Expression unwrapCast(ComparisonExpression expression) Optional sourceRange = sourceType.getRange(); if (sourceRange.isPresent()) { Object max = sourceRange.get().getMax(); - Object maxInTargetType = coerce(max, sourceToTarget); - - // NaN values of `right` are excluded at this point. Otherwise, NaN would be recognized as - // greater than source type upper bound, and incorrect expression might be derived. - int upperBoundComparison = compare(targetType, right, maxInTargetType); - if (upperBoundComparison > 0) { - // larger than maximum representable value - return switch (operator) { - case EQUAL, GREATER_THAN, GREATER_THAN_OR_EQUAL -> falseIfNotNull(cast.getExpression()); - case NOT_EQUAL, LESS_THAN, LESS_THAN_OR_EQUAL -> trueIfNotNull(cast.getExpression()); - case IS_DISTINCT_FROM -> TRUE_LITERAL; - }; + Object maxInTargetType = null; + try { + maxInTargetType = coerce(max, sourceToTarget); } - - if (upperBoundComparison == 0) { - // equal to max representable value - return switch (operator) { - case GREATER_THAN -> falseIfNotNull(cast.getExpression()); - case GREATER_THAN_OR_EQUAL -> new ComparisonExpression(EQUAL, cast.getExpression(), literalEncoder.toExpression(session, max, sourceType)); - case LESS_THAN_OR_EQUAL -> trueIfNotNull(cast.getExpression()); - case LESS_THAN -> new ComparisonExpression(NOT_EQUAL, cast.getExpression(), literalEncoder.toExpression(session, max, sourceType)); - case EQUAL, NOT_EQUAL, IS_DISTINCT_FROM -> new ComparisonExpression(operator, cast.getExpression(), literalEncoder.toExpression(session, max, sourceType)); - }; + catch (RuntimeException e) { + // Coercion may fail e.g. for out of range values, it's not guaranteed to be "saturated" } + if (maxInTargetType != null) { + // NaN values of `right` are excluded at this point. Otherwise, NaN would be recognized as + // greater than source type upper bound, and incorrect expression might be derived. + int upperBoundComparison = compare(targetType, right, maxInTargetType); + if (upperBoundComparison > 0) { + // larger than maximum representable value + return switch (operator) { + case EQUAL, GREATER_THAN, GREATER_THAN_OR_EQUAL -> falseIfNotNull(cast.getExpression()); + case NOT_EQUAL, LESS_THAN, LESS_THAN_OR_EQUAL -> trueIfNotNull(cast.getExpression()); + case IS_DISTINCT_FROM -> TRUE_LITERAL; + }; + } - Object min = sourceRange.get().getMin(); - Object minInTargetType = coerce(min, sourceToTarget); - - int lowerBoundComparison = compare(targetType, right, minInTargetType); - if (lowerBoundComparison < 0) { - // smaller than minimum representable value - return switch (operator) { - case NOT_EQUAL, GREATER_THAN, GREATER_THAN_OR_EQUAL -> trueIfNotNull(cast.getExpression()); - case EQUAL, LESS_THAN, LESS_THAN_OR_EQUAL -> falseIfNotNull(cast.getExpression()); - case IS_DISTINCT_FROM -> TRUE_LITERAL; - }; - } + if (upperBoundComparison == 0) { + // equal to max representable value + return switch (operator) { + case GREATER_THAN -> falseIfNotNull(cast.getExpression()); + case GREATER_THAN_OR_EQUAL -> new ComparisonExpression(EQUAL, cast.getExpression(), literalEncoder.toExpression(session, max, sourceType)); + case LESS_THAN_OR_EQUAL -> trueIfNotNull(cast.getExpression()); + case LESS_THAN -> new ComparisonExpression(NOT_EQUAL, cast.getExpression(), literalEncoder.toExpression(session, max, sourceType)); + case EQUAL, NOT_EQUAL, IS_DISTINCT_FROM -> new ComparisonExpression(operator, cast.getExpression(), literalEncoder.toExpression(session, max, sourceType)); + }; + } - if (lowerBoundComparison == 0) { - // equal to min representable value - return switch (operator) { - case LESS_THAN -> falseIfNotNull(cast.getExpression()); - case LESS_THAN_OR_EQUAL -> new ComparisonExpression(EQUAL, cast.getExpression(), literalEncoder.toExpression(session, min, sourceType)); - case GREATER_THAN_OR_EQUAL -> trueIfNotNull(cast.getExpression()); - case GREATER_THAN -> new ComparisonExpression(NOT_EQUAL, cast.getExpression(), literalEncoder.toExpression(session, min, sourceType)); - case EQUAL, NOT_EQUAL, IS_DISTINCT_FROM -> new ComparisonExpression(operator, cast.getExpression(), literalEncoder.toExpression(session, min, sourceType)); - }; + Object min = sourceRange.get().getMin(); + Object minInTargetType = coerce(min, sourceToTarget); + + int lowerBoundComparison = compare(targetType, right, minInTargetType); + if (lowerBoundComparison < 0) { + // smaller than minimum representable value + return switch (operator) { + case NOT_EQUAL, GREATER_THAN, GREATER_THAN_OR_EQUAL -> trueIfNotNull(cast.getExpression()); + case EQUAL, LESS_THAN, LESS_THAN_OR_EQUAL -> falseIfNotNull(cast.getExpression()); + case IS_DISTINCT_FROM -> TRUE_LITERAL; + }; + } + + if (lowerBoundComparison == 0) { + // equal to min representable value + return switch (operator) { + case LESS_THAN -> falseIfNotNull(cast.getExpression()); + case LESS_THAN_OR_EQUAL -> new ComparisonExpression(EQUAL, cast.getExpression(), literalEncoder.toExpression(session, min, sourceType)); + case GREATER_THAN_OR_EQUAL -> trueIfNotNull(cast.getExpression()); + case GREATER_THAN -> new ComparisonExpression(NOT_EQUAL, cast.getExpression(), literalEncoder.toExpression(session, min, sourceType)); + case EQUAL, NOT_EQUAL, IS_DISTINCT_FROM -> new ComparisonExpression(operator, cast.getExpression(), literalEncoder.toExpression(session, min, sourceType)); + }; + } } } From dd35872b9591551f8045ab10c87a0fd495de8ea9 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 18 Oct 2022 10:05:16 +0200 Subject: [PATCH 2/7] Implement Type.getRange for date type --- .../src/test/java/io/trino/type/TestDateType.java | 10 ++++++++++ .../src/main/java/io/trino/spi/type/DateType.java | 8 ++++++++ .../main/java/io/trino/plugin/iceberg/TrinoTypes.java | 10 ++++------ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/core/trino-main/src/test/java/io/trino/type/TestDateType.java b/core/trino-main/src/test/java/io/trino/type/TestDateType.java index d1c344cba340..016d5321ca5d 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestDateType.java +++ b/core/trino-main/src/test/java/io/trino/type/TestDateType.java @@ -16,8 +16,10 @@ import io.trino.spi.block.Block; import io.trino.spi.block.BlockBuilder; import io.trino.spi.type.SqlDate; +import io.trino.spi.type.Type.Range; import static io.trino.spi.type.DateType.DATE; +import static org.testng.Assert.assertEquals; public class TestDateType extends AbstractTestType @@ -49,4 +51,12 @@ protected Object getGreaterValue(Object value) { return ((Long) value) + 1; } + + @Override + public void testRange() + { + Range range = type.getRange().orElseThrow(); + assertEquals(range.getMin(), (long) Integer.MIN_VALUE); + assertEquals(range.getMax(), (long) Integer.MAX_VALUE); + } } diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/DateType.java b/core/trino-spi/src/main/java/io/trino/spi/type/DateType.java index e9b78bf62a2f..583e76a1b5c9 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/DateType.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/DateType.java @@ -16,6 +16,8 @@ import io.trino.spi.block.Block; import io.trino.spi.connector.ConnectorSession; +import java.util.Optional; + // // A date is stored as days from 1970-01-01. // @@ -45,6 +47,12 @@ public Object getObjectValue(ConnectorSession session, Block block, int position return new SqlDate(days); } + @Override + public Optional getRange() + { + return Optional.of(new Range((long) Integer.MIN_VALUE, (long) Integer.MAX_VALUE)); + } + @Override @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") public boolean equals(Object other) diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java index 9263e5f63b34..19f12ba3a939 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java @@ -57,9 +57,8 @@ public static Optional getPreviousValue(Type type, Object value) } if (type == DATE) { - // TODO update the code here when type implements getRange - verify(type.getRange().isEmpty(), "Type %s unexpectedly returned a range", type); - return getAdjacentValue(Integer.MIN_VALUE, Integer.MAX_VALUE, (long) value, Direction.PREV); + Range typeRange = type.getRange().orElseThrow(); + return getAdjacentValue((long) typeRange.getMin(), (long) typeRange.getMax(), (long) value, Direction.PREV); } if (type instanceof TimestampType) { @@ -113,9 +112,8 @@ public static Optional getNextValue(Type type, Object value) } if (type == DATE) { - // TODO update the code here when type implements getRange - verify(type.getRange().isEmpty(), "Type %s unexpectedly returned a range", type); - return getAdjacentValue(Integer.MIN_VALUE, Integer.MAX_VALUE, (long) value, Direction.NEXT); + Range typeRange = type.getRange().orElseThrow(); + return getAdjacentValue((long) typeRange.getMin(), (long) typeRange.getMax(), (long) value, Direction.NEXT); } if (type instanceof TimestampType) { From b8d81442638510bd41531005d05617171665d3a9 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 18 Oct 2022 10:05:16 +0200 Subject: [PATCH 3/7] Implement Type.getRange for timestamp type --- .../io/trino/type/TestLongTimestampType.java | 34 ++++++++++++++++++ .../io/trino/type/TestShortTimestampType.java | 35 +++++++++++++++++++ .../io/trino/spi/type/LongTimestampType.java | 16 +++++++++ .../io/trino/spi/type/ShortTimestampType.java | 19 ++++++++++ .../io/trino/plugin/iceberg/TrinoTypes.java | 10 +++--- 5 files changed, 108 insertions(+), 6 deletions(-) diff --git a/core/trino-main/src/test/java/io/trino/type/TestLongTimestampType.java b/core/trino-main/src/test/java/io/trino/type/TestLongTimestampType.java index 08723115d959..e419e65993cb 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestLongTimestampType.java +++ b/core/trino-main/src/test/java/io/trino/type/TestLongTimestampType.java @@ -17,8 +17,13 @@ import io.trino.spi.block.BlockBuilder; import io.trino.spi.type.LongTimestamp; import io.trino.spi.type.SqlTimestamp; +import io.trino.spi.type.Type.Range; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; import static io.trino.spi.type.TimestampType.TIMESTAMP_NANOS; +import static io.trino.spi.type.TimestampType.createTimestampType; +import static org.testng.Assert.assertEquals; public class TestLongTimestampType extends AbstractTestType @@ -51,4 +56,33 @@ protected Object getGreaterValue(Object value) LongTimestamp timestamp = (LongTimestamp) value; return new LongTimestamp(timestamp.getEpochMicros() + 1, 0); } + + @Override + public void testRange() + { + Range range = type.getRange().orElseThrow(); + assertEquals(range.getMin(), new LongTimestamp(Long.MIN_VALUE, 0)); + assertEquals(range.getMax(), new LongTimestamp(Long.MAX_VALUE, 999_000)); + } + + @Test(dataProvider = "testRangeEveryPrecisionDataProvider") + public void testRangeEveryPrecision(int precision, LongTimestamp expectedMax) + { + Range range = createTimestampType(precision).getRange().orElseThrow(); + assertEquals(range.getMin(), new LongTimestamp(Long.MIN_VALUE, 0)); + assertEquals(range.getMax(), expectedMax); + } + + @DataProvider + public static Object[][] testRangeEveryPrecisionDataProvider() + { + return new Object[][] { + {7, new LongTimestamp(Long.MAX_VALUE, 900_000)}, + {8, new LongTimestamp(Long.MAX_VALUE, 990_000)}, + {9, new LongTimestamp(Long.MAX_VALUE, 999_000)}, + {10, new LongTimestamp(Long.MAX_VALUE, 999_900)}, + {11, new LongTimestamp(Long.MAX_VALUE, 999_990)}, + {12, new LongTimestamp(Long.MAX_VALUE, 999_999)}, + }; + } } diff --git a/core/trino-main/src/test/java/io/trino/type/TestShortTimestampType.java b/core/trino-main/src/test/java/io/trino/type/TestShortTimestampType.java index 02ef97cfd3d6..b4bb8e656e96 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestShortTimestampType.java +++ b/core/trino-main/src/test/java/io/trino/type/TestShortTimestampType.java @@ -16,8 +16,13 @@ import io.trino.spi.block.Block; import io.trino.spi.block.BlockBuilder; import io.trino.spi.type.SqlTimestamp; +import io.trino.spi.type.Type.Range; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; import static io.trino.spi.type.TimestampType.TIMESTAMP_MILLIS; +import static io.trino.spi.type.TimestampType.createTimestampType; +import static org.testng.Assert.assertEquals; public class TestShortTimestampType extends AbstractTestType @@ -49,4 +54,34 @@ protected Object getGreaterValue(Object value) { return ((Long) value) + 1_000; } + + @Override + public void testRange() + { + Range range = type.getRange().orElseThrow(); + assertEquals(range.getMin(), Long.MIN_VALUE + 808); + assertEquals(range.getMax(), Long.MAX_VALUE - 807); + } + + @Test(dataProvider = "testRangeEveryPrecisionDataProvider") + public void testRangeEveryPrecision(int precision, long expectedMin, long expectedMax) + { + Range range = createTimestampType(precision).getRange().orElseThrow(); + assertEquals(range.getMin(), expectedMin); + assertEquals(range.getMax(), expectedMax); + } + + @DataProvider + public static Object[][] testRangeEveryPrecisionDataProvider() + { + return new Object[][] { + {0, Long.MIN_VALUE + 775808, Long.MAX_VALUE - 775807}, + {1, Long.MIN_VALUE + 75808, Long.MAX_VALUE - 75807}, + {2, Long.MIN_VALUE + 5808, Long.MAX_VALUE - 5807}, + {3, Long.MIN_VALUE + 808, Long.MAX_VALUE - 807}, + {4, Long.MIN_VALUE + 8, Long.MAX_VALUE - 7}, + {5, Long.MIN_VALUE + 8, Long.MAX_VALUE - 7}, + {6, Long.MIN_VALUE, Long.MAX_VALUE}, + }; + } } diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/LongTimestampType.java b/core/trino-spi/src/main/java/io/trino/spi/type/LongTimestampType.java index 656d38309bd3..d3ed36374ad5 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/LongTimestampType.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/LongTimestampType.java @@ -24,13 +24,18 @@ import io.trino.spi.function.BlockPosition; import io.trino.spi.function.ScalarOperator; +import java.util.Optional; + import static io.airlift.slice.SizeOf.SIZE_OF_LONG; import static io.trino.spi.function.OperatorType.COMPARISON_UNORDERED_LAST; import static io.trino.spi.function.OperatorType.EQUAL; import static io.trino.spi.function.OperatorType.LESS_THAN; import static io.trino.spi.function.OperatorType.LESS_THAN_OR_EQUAL; import static io.trino.spi.function.OperatorType.XX_HASH_64; +import static io.trino.spi.type.Timestamps.PICOSECONDS_PER_MICROSECOND; +import static io.trino.spi.type.Timestamps.rescale; import static io.trino.spi.type.TypeOperatorDeclaration.extractOperatorDeclaration; +import static java.lang.Math.toIntExact; import static java.lang.String.format; import static java.lang.invoke.MethodHandles.lookup; @@ -43,6 +48,7 @@ class LongTimestampType extends TimestampType { private static final TypeOperatorDeclaration TYPE_OPERATOR_DECLARATION = extractOperatorDeclaration(LongTimestampType.class, lookup(), LongTimestamp.class); + private final Range range; public LongTimestampType(int precision) { @@ -51,6 +57,10 @@ public LongTimestampType(int precision) if (precision < MAX_SHORT_PRECISION + 1 || precision > MAX_PRECISION) { throw new IllegalArgumentException(format("Precision must be in the range [%s, %s]", MAX_SHORT_PRECISION + 1, MAX_PRECISION)); } + + // ShortTimestampType instances are created eagerly and shared so it's OK to precompute some things. + int picosOfMicroMax = toIntExact(PICOSECONDS_PER_MICROSECOND - rescale(1, 0, 12 - getPrecision())); + range = new Range(new LongTimestamp(Long.MIN_VALUE, 0), new LongTimestamp(Long.MAX_VALUE, picosOfMicroMax)); } @Override @@ -148,6 +158,12 @@ private static int getFraction(Block block, int position) return block.getInt(position, SIZE_OF_LONG); } + @Override + public Optional getRange() + { + return Optional.of(range); + } + @ScalarOperator(EQUAL) private static boolean equalOperator(LongTimestamp left, LongTimestamp right) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/ShortTimestampType.java b/core/trino-spi/src/main/java/io/trino/spi/type/ShortTimestampType.java index 66baab12ec89..a0adf94dd762 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/ShortTimestampType.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/ShortTimestampType.java @@ -22,12 +22,15 @@ import io.trino.spi.connector.ConnectorSession; import io.trino.spi.function.ScalarOperator; +import java.util.Optional; + import static io.trino.spi.function.OperatorType.COMPARISON_UNORDERED_LAST; import static io.trino.spi.function.OperatorType.EQUAL; import static io.trino.spi.function.OperatorType.HASH_CODE; import static io.trino.spi.function.OperatorType.LESS_THAN; import static io.trino.spi.function.OperatorType.LESS_THAN_OR_EQUAL; import static io.trino.spi.function.OperatorType.XX_HASH_64; +import static io.trino.spi.type.Timestamps.rescale; import static io.trino.spi.type.TypeOperatorDeclaration.extractOperatorDeclaration; import static java.lang.String.format; import static java.lang.invoke.MethodHandles.lookup; @@ -42,6 +45,7 @@ class ShortTimestampType extends TimestampType { private static final TypeOperatorDeclaration TYPE_OPERATOR_DECLARATION = extractOperatorDeclaration(ShortTimestampType.class, lookup(), long.class); + private final Range range; public ShortTimestampType(int precision) { @@ -50,6 +54,15 @@ public ShortTimestampType(int precision) if (precision < 0 || precision > MAX_SHORT_PRECISION) { throw new IllegalArgumentException(format("Precision must be in the range [0, %s]", MAX_SHORT_PRECISION)); } + + // ShortTimestampType instances are created eagerly and shared so it's OK to precompute some things. + if (getPrecision() == MAX_SHORT_PRECISION) { + range = new Range(Long.MIN_VALUE, Long.MAX_VALUE); + } + else { + long max = rescale(rescale(Long.MAX_VALUE, MAX_SHORT_PRECISION, getPrecision()), getPrecision(), MAX_SHORT_PRECISION); + range = new Range(-max, max); + } } @Override @@ -125,6 +138,12 @@ public Object getObjectValue(ConnectorSession session, Block block, int position return SqlTimestamp.newInstance(getPrecision(), epochMicros, 0); } + @Override + public Optional getRange() + { + return Optional.of(range); + } + @ScalarOperator(EQUAL) private static boolean equalOperator(long left, long right) { diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java index 19f12ba3a939..37d0aa90971d 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java @@ -64,9 +64,8 @@ public static Optional getPreviousValue(Type type, Object value) if (type instanceof TimestampType) { // Iceberg supports timestamp only with microsecond precision checkArgument(((TimestampType) type).getPrecision() == 6, "Unexpected type: %s", type); - // TODO update the code here when type implements getRange - verify(type.getRange().isEmpty(), "Type %s unexpectedly returned a range", type); - return getAdjacentValue(Long.MIN_VALUE, Long.MAX_VALUE, (long) value, Direction.PREV); + Range typeRange = type.getRange().orElseThrow(); + return getAdjacentValue((long) typeRange.getMin(), (long) typeRange.getMax(), (long) value, Direction.PREV); } if (type instanceof TimestampWithTimeZoneType) { @@ -119,9 +118,8 @@ public static Optional getNextValue(Type type, Object value) if (type instanceof TimestampType) { // Iceberg supports timestamp only with microsecond precision checkArgument(((TimestampType) type).getPrecision() == 6, "Unexpected type: %s", type); - // TODO update the code here when type implements getRange - verify(type.getRange().isEmpty(), "Type %s unexpectedly returned a range", type); - return getAdjacentValue(Long.MIN_VALUE, Long.MAX_VALUE, (long) value, Direction.NEXT); + Range typeRange = type.getRange().orElseThrow(); + return getAdjacentValue((long) typeRange.getMin(), (long) typeRange.getMax(), (long) value, Direction.NEXT); } if (type instanceof TimestampWithTimeZoneType) { From 2546fdf6e5cff1b365a78cf38af5030a2ce357b3 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Wed, 9 Nov 2022 14:52:59 +0100 Subject: [PATCH 4/7] Remove redundant null-friendliness Avoid Apache Commons' `StringUtils`. --- .../java/io/trino/plugin/hive/fs/CachingDirectoryLister.java | 3 +-- .../plugin/hive/fs/TransactionScopeCachingDirectoryLister.java | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/CachingDirectoryLister.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/CachingDirectoryLister.java index 19c9d95c6808..bddd0487fd25 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/CachingDirectoryLister.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/CachingDirectoryLister.java @@ -42,7 +42,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static io.trino.collect.cache.CacheUtils.uncheckedCacheGet; import static java.util.Objects.requireNonNull; -import static org.apache.commons.lang3.StringUtils.isNotEmpty; public class CachingDirectoryLister implements DirectoryLister @@ -238,7 +237,7 @@ private boolean isCacheEnabledFor(SchemaTableName schemaTableName) private static boolean isLocationPresent(Storage storage) { // Some Hive table types (e.g.: views) do not have a storage location - return storage.getOptionalLocation().isPresent() && isNotEmpty(storage.getLocation()); + return storage.getOptionalLocation().isPresent() && !storage.getLocation().isEmpty(); } /** diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/TransactionScopeCachingDirectoryLister.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/TransactionScopeCachingDirectoryLister.java index 6b2c29aff8c4..e22f29823951 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/TransactionScopeCachingDirectoryLister.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/TransactionScopeCachingDirectoryLister.java @@ -40,7 +40,6 @@ import static com.google.common.base.Throwables.throwIfUnchecked; import static java.util.Collections.synchronizedList; import static java.util.Objects.requireNonNull; -import static org.apache.commons.lang3.StringUtils.isNotEmpty; /** * Caches directory content (including listings that were started concurrently). @@ -185,7 +184,7 @@ boolean isCached(DirectoryListingCacheKey cacheKey) private static boolean isLocationPresent(Storage storage) { // Some Hive table types (e.g.: views) do not have a storage location - return storage.getOptionalLocation().isPresent() && isNotEmpty(storage.getLocation()); + return storage.getOptionalLocation().isPresent() && !storage.getLocation().isEmpty(); } private static class FetchingValueHolder From 3513750bb97f263211959a008274cf69ab68fddc Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Wed, 9 Nov 2022 14:54:14 +0100 Subject: [PATCH 5/7] Use expression switch This emphasizes that all option are covered. --- .../io/trino/plugin/iceberg/TrinoTypes.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java index 37d0aa90971d..27bf8472a80c 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoTypes.java @@ -147,20 +147,20 @@ public static Optional getNextValue(Type type, Object value) private static Optional getAdjacentValue(long min, long max, long value, Direction direction) { - switch (direction) { - case PREV: + return switch (direction) { + case PREV -> { if (value == min) { - return Optional.empty(); + yield Optional.empty(); } - return Optional.of(value - 1); - - case NEXT: + yield Optional.of(value - 1); + } + case NEXT -> { if (value == max) { - return Optional.empty(); + yield Optional.empty(); } - return Optional.of(value + 1); - } - throw new UnsupportedOperationException("Unsupported direction: " + direction); + yield Optional.of(value + 1); + } + }; } private enum Direction From 6a9ef3fbe620b2677d82ec6cc9a1117d3143d107 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Wed, 9 Nov 2022 14:55:26 +0100 Subject: [PATCH 6/7] Remove redundant warning suppression --- .../accumulo/conf/AccumuloTableProperties.java | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/conf/AccumuloTableProperties.java b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/conf/AccumuloTableProperties.java index 92a28f559f45..8544e73ba45c 100644 --- a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/conf/AccumuloTableProperties.java +++ b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/conf/AccumuloTableProperties.java @@ -134,7 +134,6 @@ public static Optional>> getColumnMapping( { requireNonNull(tableProperties); - @SuppressWarnings("unchecked") String strMapping = (String) tableProperties.get(COLUMN_MAPPING); if (strMapping == null) { return Optional.empty(); @@ -156,7 +155,6 @@ public static Optional> getIndexColumns(Map tablePr { requireNonNull(tableProperties); - @SuppressWarnings("unchecked") String indexColumns = (String) tableProperties.get(INDEX_COLUMNS); if (indexColumns == null) { return Optional.empty(); @@ -177,7 +175,6 @@ public static Optional>> getLocalityGroups(Map getRowId(Map tableProperties) { requireNonNull(tableProperties); - @SuppressWarnings("unchecked") String rowId = (String) tableProperties.get(ROW_ID); return Optional.ofNullable(rowId); } @@ -219,7 +215,6 @@ public static Optional getScanAuthorizations(Map tablePr { requireNonNull(tableProperties); - @SuppressWarnings("unchecked") String scanAuths = (String) tableProperties.get(SCAN_AUTHS); return Optional.ofNullable(scanAuths); } @@ -234,17 +229,13 @@ public static String getSerializerClass(Map tableProperties) { requireNonNull(tableProperties); - @SuppressWarnings("unchecked") - String serializerClass = (String) tableProperties.get(SERIALIZER); - return serializerClass; + return (String) tableProperties.get(SERIALIZER); } public static boolean isExternal(Map tableProperties) { requireNonNull(tableProperties); - @SuppressWarnings("unchecked") - Boolean serializerClass = (Boolean) tableProperties.get(EXTERNAL); - return serializerClass; + return (Boolean) tableProperties.get(EXTERNAL); } } From f95946c099b1500622e9724e65b270c8b5fda2c5 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 14 Nov 2022 10:46:15 +0100 Subject: [PATCH 7/7] empty