From e7a7f168f488e7cde946ec21509ddee8320e4a0e Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Sun, 13 Jul 2025 11:24:33 +0200 Subject: [PATCH 1/2] style: avoid using star imports Signed-off-by: Niels Pardon --- .../extension/ExtensionCollector.java | 4 +- .../substrait/extension/SimpleExtension.java | 7 +- .../main/java/io/substrait/type/YamlRead.java | 5 +- .../ExtendedExpressionRoundTripTest.java | 6 +- .../java/io/substrait/relation/SetTest.java | 7 +- .../type/proto/GenericRoundtripTest.java | 2 +- .../isthmus/cli/RegisterAtRuntime.java | 26 ++++++- .../expression/FieldSelectionConverter.java | 4 +- .../isthmus/expression/LiteralConverter.java | 77 +++++++++++-------- .../io/substrait/isthmus/CalciteCallTest.java | 18 +++-- .../substrait/isthmus/CalciteLiteralTest.java | 69 +++++++++-------- .../AggregateFunctionConverterTest.java | 3 +- 12 files changed, 148 insertions(+), 80 deletions(-) diff --git a/core/src/main/java/io/substrait/extension/ExtensionCollector.java b/core/src/main/java/io/substrait/extension/ExtensionCollector.java index 5cfb03589..86c2a9f1a 100644 --- a/core/src/main/java/io/substrait/extension/ExtensionCollector.java +++ b/core/src/main/java/io/substrait/extension/ExtensionCollector.java @@ -5,7 +5,9 @@ import io.substrait.proto.Plan; import io.substrait.proto.SimpleExtensionDeclaration; import io.substrait.proto.SimpleExtensionURI; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; /** diff --git a/core/src/main/java/io/substrait/extension/SimpleExtension.java b/core/src/main/java/io/substrait/extension/SimpleExtension.java index 56cf2f2ba..898809fa6 100644 --- a/core/src/main/java/io/substrait/extension/SimpleExtension.java +++ b/core/src/main/java/io/substrait/extension/SimpleExtension.java @@ -21,7 +21,12 @@ import io.substrait.util.Util; import java.io.IOException; import java.io.InputStream; -import java.util.*; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.OptionalInt; +import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.IntStream; diff --git a/core/src/main/java/io/substrait/type/YamlRead.java b/core/src/main/java/io/substrait/type/YamlRead.java index 89f8a9163..ac19891bf 100644 --- a/core/src/main/java/io/substrait/type/YamlRead.java +++ b/core/src/main/java/io/substrait/type/YamlRead.java @@ -6,7 +6,10 @@ import io.substrait.extension.DefaultExtensionCatalog; import io.substrait.extension.SimpleExtension; import java.io.File; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; diff --git a/core/src/test/java/io/substrait/extendedexpression/ExtendedExpressionRoundTripTest.java b/core/src/test/java/io/substrait/extendedexpression/ExtendedExpressionRoundTripTest.java index b417b15a6..50ce1572b 100644 --- a/core/src/test/java/io/substrait/extendedexpression/ExtendedExpressionRoundTripTest.java +++ b/core/src/test/java/io/substrait/extendedexpression/ExtendedExpressionRoundTripTest.java @@ -2,7 +2,11 @@ import io.substrait.TestBase; import io.substrait.dsl.SubstraitBuilder; -import io.substrait.expression.*; +import io.substrait.expression.AggregateFunctionInvocation; +import io.substrait.expression.Expression; +import io.substrait.expression.ExpressionCreator; +import io.substrait.expression.FieldReference; +import io.substrait.expression.ImmutableFieldReference; import io.substrait.extension.DefaultExtensionCatalog; import io.substrait.relation.Aggregate; import io.substrait.type.NamedStruct; diff --git a/core/src/test/java/io/substrait/relation/SetTest.java b/core/src/test/java/io/substrait/relation/SetTest.java index 4f27c1ecc..8c857fb3a 100644 --- a/core/src/test/java/io/substrait/relation/SetTest.java +++ b/core/src/test/java/io/substrait/relation/SetTest.java @@ -1,12 +1,15 @@ package io.substrait.relation; -import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import io.substrait.type.NamedStruct; import io.substrait.type.Type; import io.substrait.type.TypeCreator; -import java.util.*; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; class SetTest { diff --git a/core/src/test/java/io/substrait/type/proto/GenericRoundtripTest.java b/core/src/test/java/io/substrait/type/proto/GenericRoundtripTest.java index 1471154d8..ffa5ab9ae 100644 --- a/core/src/test/java/io/substrait/type/proto/GenericRoundtripTest.java +++ b/core/src/test/java/io/substrait/type/proto/GenericRoundtripTest.java @@ -19,8 +19,8 @@ import java.math.BigDecimal; import java.time.Instant; import java.time.LocalDateTime; -import java.util.*; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Random; import java.util.UUID; diff --git a/isthmus-cli/src/main/java/io/substrait/isthmus/cli/RegisterAtRuntime.java b/isthmus-cli/src/main/java/io/substrait/isthmus/cli/RegisterAtRuntime.java index 416574208..dd8e8ed04 100644 --- a/isthmus-cli/src/main/java/io/substrait/isthmus/cli/RegisterAtRuntime.java +++ b/isthmus-cli/src/main/java/io/substrait/isthmus/cli/RegisterAtRuntime.java @@ -7,7 +7,31 @@ import io.substrait.extension.SimpleExtension; import java.lang.annotation.Annotation; import java.util.Arrays; -import org.apache.calcite.rel.metadata.*; +import org.apache.calcite.rel.metadata.BuiltInMetadata; +import org.apache.calcite.rel.metadata.Metadata; +import org.apache.calcite.rel.metadata.MetadataHandler; +import org.apache.calcite.rel.metadata.RelMdAllPredicates; +import org.apache.calcite.rel.metadata.RelMdCollation; +import org.apache.calcite.rel.metadata.RelMdColumnOrigins; +import org.apache.calcite.rel.metadata.RelMdColumnUniqueness; +import org.apache.calcite.rel.metadata.RelMdDistinctRowCount; +import org.apache.calcite.rel.metadata.RelMdDistribution; +import org.apache.calcite.rel.metadata.RelMdExplainVisibility; +import org.apache.calcite.rel.metadata.RelMdExpressionLineage; +import org.apache.calcite.rel.metadata.RelMdLowerBoundCost; +import org.apache.calcite.rel.metadata.RelMdMaxRowCount; +import org.apache.calcite.rel.metadata.RelMdMemory; +import org.apache.calcite.rel.metadata.RelMdMinRowCount; +import org.apache.calcite.rel.metadata.RelMdNodeTypes; +import org.apache.calcite.rel.metadata.RelMdParallelism; +import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows; +import org.apache.calcite.rel.metadata.RelMdPopulationSize; +import org.apache.calcite.rel.metadata.RelMdPredicates; +import org.apache.calcite.rel.metadata.RelMdRowCount; +import org.apache.calcite.rel.metadata.RelMdSelectivity; +import org.apache.calcite.rel.metadata.RelMdSize; +import org.apache.calcite.rel.metadata.RelMdTableReferences; +import org.apache.calcite.rel.metadata.RelMdUniqueKeys; import org.apache.calcite.runtime.CalciteContextException; import org.apache.calcite.runtime.Resources; import org.apache.calcite.sql.fun.SqlStdOperatorTable; diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/FieldSelectionConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/FieldSelectionConverter.java index ab6233f54..e7474110d 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/FieldSelectionConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/FieldSelectionConverter.java @@ -7,7 +7,9 @@ import io.substrait.isthmus.TypeConverter; import java.util.Optional; import java.util.function.Function; -import org.apache.calcite.rex.*; +import org.apache.calcite.rex.RexCall; +import org.apache.calcite.rex.RexLiteral; +import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.SqlKind; /** Converts field selections from Calcite representation. */ diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java index 47f63908d..cf654b36e 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java @@ -1,9 +1,5 @@ package io.substrait.isthmus.expression; -import static io.substrait.expression.ExpressionCreator.*; -import static java.time.temporal.ChronoField.*; -import static java.util.concurrent.TimeUnit.NANOSECONDS; - import com.google.protobuf.ByteString; import io.substrait.expression.Expression; import io.substrait.expression.ExpressionCreator; @@ -11,16 +7,27 @@ import io.substrait.type.Type; import java.math.BigDecimal; import java.math.RoundingMode; -import java.time.*; +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.OffsetDateTime; +import java.time.ZoneId; +import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatterBuilder; +import java.time.temporal.ChronoField; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rex.RexLiteral; -import org.apache.calcite.util.*; +import org.apache.calcite.util.DateString; +import org.apache.calcite.util.NlsString; +import org.apache.calcite.util.TimeString; +import org.apache.calcite.util.TimestampString; public class LiteralConverter { // TODO: Handle conversion of user-defined type literals @@ -35,13 +42,13 @@ public LiteralConverter(TypeConverter typeConverter) { static final DateTimeFormatter CALCITE_LOCAL_DATE_FORMATTER = DateTimeFormatter.ISO_LOCAL_DATE; static final DateTimeFormatter CALCITE_LOCAL_TIME_FORMATTER = new DateTimeFormatterBuilder() - .appendValue(HOUR_OF_DAY, 2) + .appendValue(ChronoField.HOUR_OF_DAY, 2) .appendLiteral(':') - .appendValue(MINUTE_OF_HOUR, 2) + .appendValue(ChronoField.MINUTE_OF_HOUR, 2) .appendLiteral(':') - .appendValue(SECOND_OF_MINUTE, 2) + .appendValue(ChronoField.SECOND_OF_MINUTE, 2) .optionalStart() - .appendFraction(NANO_OF_SECOND, 0, 9, true) + .appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true) .toFormatter(); private static final DateTimeFormatter CALCITE_LOCAL_DATETIME_FORMATTER = new DateTimeFormatterBuilder() @@ -86,51 +93,55 @@ public Expression.Literal convert(RexLiteral literal) { final boolean n = type.nullable(); if (literal.isNull()) { - return typedNull(type); + return ExpressionCreator.typedNull(type); } return switch (literal.getType().getSqlTypeName()) { - case TINYINT -> i8(n, i(literal).intValue()); - case SMALLINT -> i16(n, i(literal).intValue()); - case INTEGER -> i32(n, i(literal).intValue()); - case BIGINT -> i64(n, i(literal).longValue()); - case BOOLEAN -> bool(n, literal.getValueAs(Boolean.class)); + case TINYINT -> ExpressionCreator.i8(n, i(literal).intValue()); + case SMALLINT -> ExpressionCreator.i16(n, i(literal).intValue()); + case INTEGER -> ExpressionCreator.i32(n, i(literal).intValue()); + case BIGINT -> ExpressionCreator.i64(n, i(literal).longValue()); + case BOOLEAN -> ExpressionCreator.bool(n, literal.getValueAs(Boolean.class)); case CHAR -> { var val = literal.getValue(); if (val instanceof NlsString nls) { - yield fixedChar(n, nls.getValue()); + yield ExpressionCreator.fixedChar(n, nls.getValue()); } throw new UnsupportedOperationException("Unable to handle char type: " + val); } - case FLOAT, DOUBLE -> fp64(n, literal.getValueAs(Double.class)); - case REAL -> fp32(n, literal.getValueAs(Float.class)); + case FLOAT, DOUBLE -> ExpressionCreator.fp64(n, literal.getValueAs(Double.class)); + case REAL -> ExpressionCreator.fp32(n, literal.getValueAs(Float.class)); case DECIMAL -> { BigDecimal bd = bd(literal); - yield decimal(n, bd, literal.getType().getPrecision(), literal.getType().getScale()); + yield ExpressionCreator.decimal( + n, bd, literal.getType().getPrecision(), literal.getType().getScale()); } case VARCHAR -> { if (literal.getType().getPrecision() == RelDataType.PRECISION_NOT_SPECIFIED) { - yield string(n, s(literal)); + yield ExpressionCreator.string(n, s(literal)); } - yield varChar(n, s(literal), literal.getType().getPrecision()); + yield ExpressionCreator.varChar(n, s(literal), literal.getType().getPrecision()); } - case BINARY -> fixedBinary( + case BINARY -> ExpressionCreator.fixedBinary( n, ByteString.copyFrom( padRightIfNeeded( literal.getValueAs(org.apache.calcite.avatica.util.ByteString.class), literal.getType().getPrecision()))); - case VARBINARY -> binary(n, ByteString.copyFrom(literal.getValueAs(byte[].class))); + case VARBINARY -> ExpressionCreator.binary( + n, ByteString.copyFrom(literal.getValueAs(byte[].class))); case SYMBOL -> { Object value = literal.getValue(); // case TimeUnitRange tur -> string(n, tur.name()); if (value instanceof NlsString s) { - yield string(n, s.getValue()); + yield ExpressionCreator.string(n, s.getValue()); } else if (value instanceof Enum v) { Optional r = - EnumConverter.canConvert(v) ? Optional.of(string(n, v.name())) : Optional.empty(); + EnumConverter.canConvert(v) + ? Optional.of(ExpressionCreator.string(n, v.name())) + : Optional.empty(); yield r.orElseThrow( () -> new UnsupportedOperationException("Unable to handle symbol: " + value)); } else { @@ -145,19 +156,19 @@ public Expression.Literal convert(RexLiteral literal) { case TIME -> { TimeString time = literal.getValueAs(TimeString.class); LocalTime localTime = LocalTime.parse(time.toString(), CALCITE_LOCAL_TIME_FORMATTER); - yield time(n, NANOSECONDS.toMicros(localTime.toNanoOfDay())); + yield ExpressionCreator.time(n, TimeUnit.NANOSECONDS.toMicros(localTime.toNanoOfDay())); } case TIMESTAMP, TIMESTAMP_WITH_LOCAL_TIME_ZONE -> { TimestampString timestamp = literal.getValueAs(TimestampString.class); LocalDateTime ldt = LocalDateTime.parse(timestamp.toString(), CALCITE_LOCAL_DATETIME_FORMATTER); - yield timestamp(n, ldt); + yield ExpressionCreator.timestamp(n, ldt); } case INTERVAL_YEAR, INTERVAL_YEAR_MONTH, INTERVAL_MONTH -> { long intervalLength = Objects.requireNonNull(literal.getValueAs(Long.class)); var years = intervalLength / 12; var months = intervalLength - years * 12; - yield intervalYear(n, (int) years, (int) months); + yield ExpressionCreator.intervalYear(n, (int) years, (int) months); } case INTERVAL_DAY, INTERVAL_DAY_HOUR, @@ -177,17 +188,19 @@ public Expression.Literal convert(RexLiteral literal) { var seconds = interval.minusDays(days).toSeconds(); var micros = interval.toMillisPart() * 1000; - yield intervalDay(n, (int) days, (int) seconds, micros, 6); + yield ExpressionCreator.intervalDay(n, (int) days, (int) seconds, micros, 6); } case ROW -> { List literals = (List) literal.getValue(); - yield struct(n, literals.stream().map(this::convert).collect(Collectors.toList())); + yield ExpressionCreator.struct( + n, literals.stream().map(this::convert).collect(Collectors.toList())); } case ARRAY -> { List literals = (List) literal.getValue(); - yield list(n, literals.stream().map(this::convert).collect(Collectors.toList())); + yield ExpressionCreator.list( + n, literals.stream().map(this::convert).collect(Collectors.toList())); } default -> throw new UnsupportedOperationException( diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java index 870093289..94a40bfe6 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java @@ -1,6 +1,5 @@ package io.substrait.isthmus; -import static org.apache.calcite.sql.fun.SqlStdOperatorTable.*; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -49,7 +48,8 @@ public void extract() { public void coerceNumericOp() { test( "add:i64_i64", - rex.makeCall(PLUS, c(20, SqlTypeName.INTEGER), c(4, SqlTypeName.BIGINT)), + rex.makeCall( + SqlStdOperatorTable.PLUS, c(20, SqlTypeName.INTEGER), c(4, SqlTypeName.BIGINT)), func -> { // check that there is a cast for the incorrect argument type. assertEquals( @@ -66,7 +66,7 @@ public void coerceNumericOp() { public void directMatchPlus() { test( "add:i64_i64", - rex.makeCall(PLUS, c(4, SqlTypeName.BIGINT), c(4, SqlTypeName.BIGINT)), + rex.makeCall(SqlStdOperatorTable.PLUS, c(4, SqlTypeName.BIGINT), c(4, SqlTypeName.BIGINT)), func -> { // ensure both literals are included directly. @@ -78,17 +78,23 @@ public void directMatchPlus() { @Test public void directMatchAnd() { - test("and:bool", rex.makeCall(AND, c(true, SqlTypeName.BOOLEAN), c(true, SqlTypeName.BOOLEAN))); + test( + "and:bool", + rex.makeCall( + SqlStdOperatorTable.AND, c(true, SqlTypeName.BOOLEAN), c(true, SqlTypeName.BOOLEAN))); } @Test public void directMatchOr() { - test("or:bool", rex.makeCall(OR, c(false, SqlTypeName.BOOLEAN), c(true, SqlTypeName.BOOLEAN))); + test( + "or:bool", + rex.makeCall( + SqlStdOperatorTable.OR, c(false, SqlTypeName.BOOLEAN), c(true, SqlTypeName.BOOLEAN))); } @Test public void not() { - test("not:bool", rex.makeCall(NOT, c(false, SqlTypeName.BOOLEAN))); + test("not:bool", rex.makeCall(SqlStdOperatorTable.NOT, c(false, SqlTypeName.BOOLEAN))); } private void test(String expectedName, RexNode call) { diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java index 6692d5e88..959469d3c 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java @@ -1,13 +1,12 @@ package io.substrait.isthmus; -import static io.substrait.expression.ExpressionCreator.*; -import static io.substrait.isthmus.SqlConverterBase.EXTENSION_COLLECTION; import static io.substrait.isthmus.SqlToSubstrait.EXTENSION_COLLECTION; import static io.substrait.isthmus.SubstraitTypeSystem.YEAR_MONTH_INTERVAL; import static org.junit.jupiter.api.Assertions.assertEquals; import com.google.common.collect.ImmutableMap; import io.substrait.expression.Expression; +import io.substrait.expression.ExpressionCreator; import io.substrait.isthmus.SubstraitRelNodeConverter.Context; import io.substrait.isthmus.expression.ExpressionRexConverter; import io.substrait.isthmus.expression.RexExpressionConverter; @@ -46,62 +45,62 @@ public class CalciteLiteralTest extends CalciteObjs { @Test void nullLiteral() { bitest( - typedNull(TypeCreator.NULLABLE.varChar(10)), + ExpressionCreator.typedNull(TypeCreator.NULLABLE.varChar(10)), rex.makeNullLiteral(tN(SqlTypeName.VARCHAR, 10))); } @Test void tI8() { - bitest(i8(false, 4), c(4, SqlTypeName.TINYINT)); + bitest(ExpressionCreator.i8(false, 4), c(4, SqlTypeName.TINYINT)); } @Test void tI16() { - bitest(i16(false, 4), c(4, SqlTypeName.SMALLINT)); + bitest(ExpressionCreator.i16(false, 4), c(4, SqlTypeName.SMALLINT)); } @Test void tI32() { - bitest(i32(false, 4), c(4, SqlTypeName.INTEGER)); + bitest(ExpressionCreator.i32(false, 4), c(4, SqlTypeName.INTEGER)); } @Test void tI64() { - bitest(i64(false, 1234L), c(1234L, SqlTypeName.BIGINT)); + bitest(ExpressionCreator.i64(false, 1234L), c(1234L, SqlTypeName.BIGINT)); } @Test void tFP32() { - bitest(fp32(false, 4.44F), c(4.44F, SqlTypeName.REAL)); + bitest(ExpressionCreator.fp32(false, 4.44F), c(4.44F, SqlTypeName.REAL)); } @Test void tFP64() { - bitest(fp64(false, 4.45F), c(4.45F, SqlTypeName.DOUBLE)); + bitest(ExpressionCreator.fp64(false, 4.45F), c(4.45F, SqlTypeName.DOUBLE)); } @Test void tFloatFP64() { - test(fp64(false, 4.45F), c(4.45F, SqlTypeName.FLOAT)); + test(ExpressionCreator.fp64(false, 4.45F), c(4.45F, SqlTypeName.FLOAT)); } @Test void tStr() { - bitest(string(false, "my test"), c("my test", SqlTypeName.VARCHAR)); + bitest(ExpressionCreator.string(false, "my test"), c("my test", SqlTypeName.VARCHAR)); } @Test void tBinary() { var val = "my test".getBytes(StandardCharsets.UTF_8); bitest( - binary(false, val), + ExpressionCreator.binary(false, val), c(new org.apache.calcite.avatica.util.ByteString(val), SqlTypeName.VARBINARY)); } @Test void tTime() { bitest( - time(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000), + ExpressionCreator.time(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000), rex.makeTimeLiteral(new TimeString(14, 22, 47), 6)); } @@ -117,7 +116,7 @@ void tTimeWithMicroSecond() { new TimeString("14:22:47.123456")); bitest( - time(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000 + 123456), + ExpressionCreator.time(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000 + 123456), rex.makeTimeLiteral(new TimeString("14:22:47.123456"), 6)); } @@ -131,13 +130,13 @@ void tTimeWithNanoSecond() { @Test void tDate() { bitest( - date(false, (int) LocalDate.of(2002, 2, 14).toEpochDay()), + ExpressionCreator.date(false, (int) LocalDate.of(2002, 2, 14).toEpochDay()), rex.makeDateLiteral(new DateString(2002, 2, 14))); } @Test void tTimestamp() { - var ts = timestamp(false, 2002, 2, 14, 16, 20, 47, 123); + var ts = ExpressionCreator.timestamp(false, 2002, 2, 14, 16, 20, 47, 123); var nano = (int) TimeUnit.MICROSECONDS.toNanos(123); var tsx = new TimestampString(2002, 2, 14, 16, 20, 47).withNanos(nano); bitest(ts, rex.makeTimestampLiteral(tsx, 6)); @@ -145,7 +144,7 @@ void tTimestamp() { @Test void tTimestampWithMilliMacroSeconds() { - var ts = timestamp(false, 2002, 2, 14, 16, 20, 47, 123456); + var ts = ExpressionCreator.timestamp(false, 2002, 2, 14, 16, 20, 47, 123456); var nano = (int) TimeUnit.MICROSECONDS.toNanos(123456); var tsx = new TimestampString(2002, 2, 14, 16, 20, 47).withNanos(nano); bitest(ts, rex.makeTimestampLiteral(tsx, 6)); @@ -163,7 +162,7 @@ void tTimestampTZ() { void tIntervalYearMonth() { BigDecimal bd = new BigDecimal(3 * 12 + 5); // '3-5' year to month RexLiteral intervalYearMonth = rex.makeIntervalLiteral(bd, YEAR_MONTH_INTERVAL); - var intervalYearMonthExpr = intervalYear(false, 3, 5); + var intervalYearMonthExpr = ExpressionCreator.intervalYear(false, 3, 5); bitest(intervalYearMonthExpr, intervalYearMonth); } @@ -179,7 +178,7 @@ void tIntervalYearMonthWithPrecision() { org.apache.calcite.avatica.util.TimeUnit.MONTH, -1, SqlParserPos.QUOTED_ZERO)); - var intervalYearMonthExpr = intervalYear(false, 123, 5); + var intervalYearMonthExpr = ExpressionCreator.intervalYear(false, 123, 5); // rex --> expression assertEquals(intervalYearMonthExpr, intervalYearMonth.accept(rexExpressionConverter)); @@ -214,7 +213,8 @@ void tIntervalMillisecond() { org.apache.calcite.avatica.util.TimeUnit.SECOND, 3, SqlParserPos.ZERO)); - var intervalDaySecondExpr = intervalDay(false, 3, 5 * 3600 + 7 * 60 + 9, 500_000, 6); + var intervalDaySecondExpr = + ExpressionCreator.intervalDay(false, 3, 5 * 3600 + 7 * 60 + 9, 500_000, 6); bitest(intervalDaySecondExpr, intervalDaySecond); } @@ -227,7 +227,7 @@ void tIntervalDay() { bd, new SqlIntervalQualifier( org.apache.calcite.avatica.util.TimeUnit.DAY, -1, null, -1, SqlParserPos.ZERO)); - var intervalDayExpr = intervalDay(false, 5, 0, 0, 6); + var intervalDayExpr = ExpressionCreator.intervalDay(false, 5, 0, 0, 6); // rex --> expression var convertedExpr = intervalDayLiteral.accept(rexExpressionConverter); @@ -254,7 +254,7 @@ void tIntervalYear() { null, -1, SqlParserPos.QUOTED_ZERO)); - var intervalYearExpr = intervalYear(false, 123, 0); + var intervalYearExpr = ExpressionCreator.intervalYear(false, 123, 0); // rex --> expression assertEquals(intervalYearExpr, intervalYear.accept(rexExpressionConverter)); @@ -280,7 +280,7 @@ void tIntervalMonth() { null, -1, SqlParserPos.QUOTED_ZERO)); - var intervalMonthExpr = intervalYear(false, 123 / 12, 123 % 12); + var intervalMonthExpr = ExpressionCreator.intervalYear(false, 123 / 12, 123 % 12); // rex --> expression assertEquals(intervalMonthExpr, intervalMonth.accept(rexExpressionConverter)); @@ -296,12 +296,12 @@ void tIntervalMonth() { @Test void tFixedChar() { - bitest(fixedChar(false, "hello "), c("hello ", SqlTypeName.CHAR)); + bitest(ExpressionCreator.fixedChar(false, "hello "), c("hello ", SqlTypeName.CHAR)); } @Test void tVarChar() { - bitest(varChar(false, "hello ", 10), c("hello ", SqlTypeName.VARCHAR, 10)); + bitest(ExpressionCreator.varChar(false, "hello ", 10), c("hello ", SqlTypeName.VARCHAR, 10)); } @Test @@ -313,7 +313,7 @@ void tDecimalLiteral() { new BigDecimal("123.450000"), new BigDecimal("-123.450000")); for (BigDecimal bd : decimalList) { - bitest(decimal(false, bd, 32, 6), c(bd, SqlTypeName.DECIMAL, 32, 6)); + bitest(ExpressionCreator.decimal(false, bd, 32, 6), c(bd, SqlTypeName.DECIMAL, 32, 6)); } } @@ -325,7 +325,7 @@ void tDecimalLiteral2() { new BigDecimal("99.123456789123456789123456789123456789") // scale = 36, precision = 38 ); for (BigDecimal bd : decimalList) { - bitest(decimal(false, bd, 38, 36), c(bd, SqlTypeName.DECIMAL, 38, 36)); + bitest(ExpressionCreator.decimal(false, bd, 38, 36), c(bd, SqlTypeName.DECIMAL, 38, 36)); } } @@ -346,20 +346,24 @@ void tDecimalUtil() { void tMap() { var ss = ImmutableMap.of( - string(false, "foo"), i32(false, 4), string(false, "bar"), i32(false, -1)); + ExpressionCreator.string(false, "foo"), + ExpressionCreator.i32(false, 4), + ExpressionCreator.string(false, "bar"), + ExpressionCreator.i32(false, -1)); var calcite = rex.makeLiteral( ImmutableMap.of("foo", 4, "bar", -1), type.createMapType(t(SqlTypeName.VARCHAR), t(SqlTypeName.INTEGER)), true, false); - bitest(map(false, ss), calcite); + bitest(ExpressionCreator.map(false, ss), calcite); } @Test void tList() { bitest( - list(false, i32(false, 4), i32(false, -1)), + ExpressionCreator.list( + false, ExpressionCreator.i32(false, 4), ExpressionCreator.i32(false, -1)), rex.makeLiteral( Arrays.asList(4, -1), type.createArrayType(t(SqlTypeName.INTEGER), -1), false, false)); } @@ -367,7 +371,8 @@ void tList() { @Test void tStruct() { test( - struct(false, i32(false, 4), i32(false, -1)), + ExpressionCreator.struct( + false, ExpressionCreator.i32(false, 4), ExpressionCreator.i32(false, -1)), rex.makeLiteral( Arrays.asList(4, -1), type.createStructType( @@ -381,7 +386,7 @@ void tStruct() { void tFixedBinary() { var val = "my test".getBytes(StandardCharsets.UTF_8); bitest( - fixedBinary(false, val), + ExpressionCreator.fixedBinary(false, val), c(new org.apache.calcite.avatica.util.ByteString(val), SqlTypeName.BINARY)); } diff --git a/isthmus/src/test/java/io/substrait/isthmus/expression/AggregateFunctionConverterTest.java b/isthmus/src/test/java/io/substrait/isthmus/expression/AggregateFunctionConverterTest.java index f72472d11..6cdff89fe 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/expression/AggregateFunctionConverterTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/expression/AggregateFunctionConverterTest.java @@ -1,6 +1,7 @@ package io.substrait.isthmus.expression; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import io.substrait.isthmus.AggregateFunctions; import io.substrait.isthmus.PlanTestBase; From daded9fa4b8d45c0db6606f49d09cfe4f077a5e5 Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Thu, 17 Jul 2025 15:59:25 +0200 Subject: [PATCH 2/2] chore: update spotless and enable removeWildcardImports Signed-off-by: Niels Pardon --- build.gradle.kts | 3 +- core/build.gradle.kts | 4 +- .../io/substrait/expression/FunctionArg.java | 5 +- .../proto/ProtoExpressionConverter.java | 141 ++++++++++-------- .../ExtendedExpressionProtoConverter.java | 3 +- .../main/java/io/substrait/relation/Join.java | 32 ++-- .../substrait/relation/ProtoRelConverter.java | 31 ++-- .../main/java/io/substrait/relation/Set.java | 4 +- .../substrait/relation/physical/HashJoin.java | 8 +- .../relation/physical/MergeJoin.java | 8 +- .../relation/physical/NestedLoopJoin.java | 8 +- .../io/substrait/type/parser/ParseToPojo.java | 5 +- .../type/proto/ProtoTypeConverter.java | 57 +++---- .../proto/TypeExpressionProtoVisitor.java | 6 +- .../type/proto/LocalFilesRoundtripTest.java | 26 ++-- examples/substrait-spark/build.gradle.kts | 2 +- isthmus-cli/build.gradle.kts | 2 +- .../isthmus/cli/IsthmusEntryPoint.java | 4 +- isthmus/build.gradle.kts | 2 +- .../isthmus/SubstraitRelNodeConverter.java | 27 ++-- .../isthmus/SubstraitRelVisitor.java | 26 ++-- .../io/substrait/isthmus/TypeConverter.java | 29 ++-- .../expression/ExpressionRexConverter.java | 10 +- .../isthmus/expression/LiteralConverter.java | 26 ++-- .../expression/RexExpressionConverter.java | 5 +- .../expression/SortFieldConverter.java | 25 ++-- .../isthmus/AggregationFunctionsTest.java | 4 +- .../substrait/isthmus/NameRoundtripTest.java | 7 +- .../io/substrait/isthmus/utils/SetUtils.java | 4 +- spark/.scalafmt.conf | 2 +- spark/build.gradle.kts | 2 +- 31 files changed, 282 insertions(+), 236 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 667ffb892..b38d1c96d 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -8,7 +8,7 @@ plugins { id("java") id("idea") id("com.github.vlsi.gradle-extensions") version "1.74" - id("com.diffplug.spotless") version "6.19.0" + id("com.diffplug.spotless") version "7.1.0" id("org.jreleaser") version "1.18.0" apply false } @@ -67,6 +67,7 @@ allprojects { googleJavaFormat() removeUnusedImports() trimTrailingWhitespace() + removeWildcardImports() } } } diff --git a/core/build.gradle.kts b/core/build.gradle.kts index 97b30a3f1..07232b71f 100644 --- a/core/build.gradle.kts +++ b/core/build.gradle.kts @@ -12,7 +12,7 @@ plugins { id("idea") id("antlr") id("com.google.protobuf") version "0.9.4" - id("com.diffplug.spotless") version "6.19.0" + id("com.diffplug.spotless") version "7.1.0" id("com.gradleup.shadow") version "8.3.6" id("org.jreleaser") } @@ -263,7 +263,7 @@ project.configure { generatedSourceDirs.addAll( listOf( file("build/generated/sources/antlr/main"), - file("build/generated/source/proto/main/java") + file("build/generated/source/proto/main/java"), ) ) } diff --git a/core/src/main/java/io/substrait/expression/FunctionArg.java b/core/src/main/java/io/substrait/expression/FunctionArg.java index 495def8ad..a035fa3fe 100644 --- a/core/src/main/java/io/substrait/expression/FunctionArg.java +++ b/core/src/main/java/io/substrait/expression/FunctionArg.java @@ -91,8 +91,9 @@ public FunctionArg convert( var optionValue = fArg.getEnum(); yield EnumArg.of(enumArgDef, optionValue); } - default -> throw new UnsupportedOperationException( - String.format("Unable to convert FunctionArgument %s.", fArg)); + default -> + throw new UnsupportedOperationException( + String.format("Unable to convert FunctionArgument %s.", fArg)); }; } } diff --git a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java index 4a4d6a4fc..59f7fb402 100644 --- a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java +++ b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java @@ -76,30 +76,34 @@ public FieldReference from(io.substrait.proto.Expression.FieldReference referenc segment = listElement.getChild(); yield FieldReference.ListElement.of(listElement.getOffset()); } - case REFERENCETYPE_NOT_SET -> throw new IllegalArgumentException( - "Unhandled type: " + segment.getReferenceTypeCase()); + case REFERENCETYPE_NOT_SET -> + throw new IllegalArgumentException( + "Unhandled type: " + segment.getReferenceTypeCase()); }); } Collections.reverse(segments); var fieldReference = switch (reference.getRootTypeCase()) { - case EXPRESSION -> FieldReference.ofExpression( - from(reference.getExpression()), segments); + case EXPRESSION -> + FieldReference.ofExpression(from(reference.getExpression()), segments); case ROOT_REFERENCE -> FieldReference.ofRoot(rootType, segments); - case OUTER_REFERENCE -> FieldReference.newRootStructOuterReference( - reference.getDirectReference().getStructField().getField(), - rootType, - reference.getOuterReference().getStepsOut()); - case ROOTTYPE_NOT_SET -> throw new IllegalArgumentException( - "Unhandled type: " + reference.getRootTypeCase()); + case OUTER_REFERENCE -> + FieldReference.newRootStructOuterReference( + reference.getDirectReference().getStructField().getField(), + rootType, + reference.getOuterReference().getStepsOut()); + case ROOTTYPE_NOT_SET -> + throw new IllegalArgumentException( + "Unhandled type: " + reference.getRootTypeCase()); }; return fieldReference; } - case MASKED_REFERENCE -> throw new IllegalArgumentException( - "Unsupported type: " + reference.getReferenceTypeCase()); - default -> throw new IllegalArgumentException( - "Unhandled type: " + reference.getReferenceTypeCase()); + case MASKED_REFERENCE -> + throw new IllegalArgumentException( + "Unsupported type: " + reference.getReferenceTypeCase()); + default -> + throw new IllegalArgumentException("Unhandled type: " + reference.getReferenceTypeCase()); } } @@ -177,10 +181,11 @@ public Expression from(io.substrait.proto.Expression expr) { .collect(java.util.stream.Collectors.toList())) .build(); } - case CAST -> ExpressionCreator.cast( - protoTypeConverter.from(expr.getCast().getType()), - from(expr.getCast().getInput()), - Expression.FailureBehavior.fromProto(expr.getCast().getFailureBehavior())); + case CAST -> + ExpressionCreator.cast( + protoTypeConverter.from(expr.getCast().getType()), + from(expr.getCast().getInput()), + Expression.FailureBehavior.fromProto(expr.getCast().getFailureBehavior())); case SUBQUERY -> { switch (expr.getSubquery().getSubqueryTypeCase()) { case SET_PREDICATE -> { @@ -232,9 +237,9 @@ public Type visit(Type.Struct type) throws RuntimeException { } } - // TODO enum. - case ENUM -> throw new UnsupportedOperationException( - "Unsupported type: " + expr.getRexTypeCase()); + // TODO enum. + case ENUM -> + throw new UnsupportedOperationException("Unsupported type: " + expr.getRexTypeCase()); default -> throw new IllegalArgumentException("Unknown type: " + expr.getRexTypeCase()); }; } @@ -320,9 +325,9 @@ private WindowBound toWindowBound(io.substrait.proto.Expression.WindowFunction.B case CURRENT_ROW -> WindowBound.CURRENT_ROW; case UNBOUNDED -> WindowBound.UNBOUNDED; case KIND_NOT_SET -> - // per the spec, the lower and upper bounds default to the start or end of the partition - // respectively if not set - WindowBound.UNBOUNDED; + // per the spec, the lower and upper bounds default to the start or end of the partition + // respectively if not set + WindowBound.UNBOUNDED; }; } @@ -338,22 +343,25 @@ public Expression.Literal from(io.substrait.proto.Expression.Literal literal) { case STRING -> ExpressionCreator.string(literal.getNullable(), literal.getString()); case BINARY -> ExpressionCreator.binary(literal.getNullable(), literal.getBinary()); case TIMESTAMP -> ExpressionCreator.timestamp(literal.getNullable(), literal.getTimestamp()); - case TIMESTAMP_TZ -> ExpressionCreator.timestampTZ( - literal.getNullable(), literal.getTimestampTz()); - case PRECISION_TIMESTAMP -> ExpressionCreator.precisionTimestamp( - literal.getNullable(), - literal.getPrecisionTimestamp().getValue(), - literal.getPrecisionTimestamp().getPrecision()); - case PRECISION_TIMESTAMP_TZ -> ExpressionCreator.precisionTimestampTZ( - literal.getNullable(), - literal.getPrecisionTimestampTz().getValue(), - literal.getPrecisionTimestampTz().getPrecision()); + case TIMESTAMP_TZ -> + ExpressionCreator.timestampTZ(literal.getNullable(), literal.getTimestampTz()); + case PRECISION_TIMESTAMP -> + ExpressionCreator.precisionTimestamp( + literal.getNullable(), + literal.getPrecisionTimestamp().getValue(), + literal.getPrecisionTimestamp().getPrecision()); + case PRECISION_TIMESTAMP_TZ -> + ExpressionCreator.precisionTimestampTZ( + literal.getNullable(), + literal.getPrecisionTimestampTz().getValue(), + literal.getPrecisionTimestampTz().getPrecision()); case DATE -> ExpressionCreator.date(literal.getNullable(), literal.getDate()); case TIME -> ExpressionCreator.time(literal.getNullable(), literal.getTime()); - case INTERVAL_YEAR_TO_MONTH -> ExpressionCreator.intervalYear( - literal.getNullable(), - literal.getIntervalYearToMonth().getYears(), - literal.getIntervalYearToMonth().getMonths()); + case INTERVAL_YEAR_TO_MONTH -> + ExpressionCreator.intervalYear( + literal.getNullable(), + literal.getIntervalYearToMonth().getYears(), + literal.getIntervalYearToMonth().getMonths()); case INTERVAL_DAY_TO_SECOND -> { // Handle deprecated version that doesn't provide precision and that uses microseconds // instead of subseconds, for backwards compatibility @@ -387,24 +395,30 @@ public Expression.Literal from(io.substrait.proto.Expression.Literal literal) { literal.getIntervalCompound().getIntervalDayToSecond().getPrecision()); } case FIXED_CHAR -> ExpressionCreator.fixedChar(literal.getNullable(), literal.getFixedChar()); - case VAR_CHAR -> ExpressionCreator.varChar( - literal.getNullable(), literal.getVarChar().getValue(), literal.getVarChar().getLength()); - case FIXED_BINARY -> ExpressionCreator.fixedBinary( - literal.getNullable(), literal.getFixedBinary()); - case DECIMAL -> ExpressionCreator.decimal( - literal.getNullable(), - literal.getDecimal().getValue(), - literal.getDecimal().getPrecision(), - literal.getDecimal().getScale()); - case STRUCT -> ExpressionCreator.struct( - literal.getNullable(), - literal.getStruct().getFieldsList().stream() - .map(this::from) - .collect(java.util.stream.Collectors.toList())); - case MAP -> ExpressionCreator.map( - literal.getNullable(), - literal.getMap().getKeyValuesList().stream() - .collect(Collectors.toMap(kv -> from(kv.getKey()), kv -> from(kv.getValue())))); + case VAR_CHAR -> + ExpressionCreator.varChar( + literal.getNullable(), + literal.getVarChar().getValue(), + literal.getVarChar().getLength()); + case FIXED_BINARY -> + ExpressionCreator.fixedBinary(literal.getNullable(), literal.getFixedBinary()); + case DECIMAL -> + ExpressionCreator.decimal( + literal.getNullable(), + literal.getDecimal().getValue(), + literal.getDecimal().getPrecision(), + literal.getDecimal().getScale()); + case STRUCT -> + ExpressionCreator.struct( + literal.getNullable(), + literal.getStruct().getFieldsList().stream() + .map(this::from) + .collect(java.util.stream.Collectors.toList())); + case MAP -> + ExpressionCreator.map( + literal.getNullable(), + literal.getMap().getKeyValuesList().stream() + .collect(Collectors.toMap(kv -> from(kv.getKey()), kv -> from(kv.getValue())))); case EMPTY_MAP -> { // literal.getNullable() is intentionally ignored in favor of the nullability // specified in the literal.getEmptyMap() type. @@ -413,11 +427,12 @@ public Expression.Literal from(io.substrait.proto.Expression.Literal literal) { } case UUID -> ExpressionCreator.uuid(literal.getNullable(), literal.getUuid()); case NULL -> ExpressionCreator.typedNull(protoTypeConverter.from(literal.getNull())); - case LIST -> ExpressionCreator.list( - literal.getNullable(), - literal.getList().getValuesList().stream() - .map(this::from) - .collect(java.util.stream.Collectors.toList())); + case LIST -> + ExpressionCreator.list( + literal.getNullable(), + literal.getList().getValuesList().stream() + .map(this::from) + .collect(java.util.stream.Collectors.toList())); case EMPTY_LIST -> { // literal.getNullable() is intentionally ignored in favor of the nullability // specified in the literal.getEmptyList() type. @@ -430,8 +445,8 @@ public Expression.Literal from(io.substrait.proto.Expression.Literal literal) { yield ExpressionCreator.userDefinedLiteral( literal.getNullable(), type.uri(), type.name(), userDefinedLiteral.getValue()); } - default -> throw new IllegalStateException( - "Unexpected value: " + literal.getLiteralTypeCase()); + default -> + throw new IllegalStateException("Unexpected value: " + literal.getLiteralTypeCase()); }; } diff --git a/core/src/main/java/io/substrait/extendedexpression/ExtendedExpressionProtoConverter.java b/core/src/main/java/io/substrait/extendedexpression/ExtendedExpressionProtoConverter.java index 00c340b53..5189f7dbf 100644 --- a/core/src/main/java/io/substrait/extendedexpression/ExtendedExpressionProtoConverter.java +++ b/core/src/main/java/io/substrait/extendedexpression/ExtendedExpressionProtoConverter.java @@ -36,8 +36,7 @@ public ExtendedExpression toProto( builder.addReferredExpr(expressionReferenceBuilder); } else if (expressionReference instanceof - io.substrait.extendedexpression.ExtendedExpression.AggregateFunctionReference - aft) { + io.substrait.extendedexpression.ExtendedExpression.AggregateFunctionReference aft) { ExpressionReference.Builder expressionReferenceBuilder = ExpressionReference.newBuilder() .setMeasure( diff --git a/core/src/main/java/io/substrait/relation/Join.java b/core/src/main/java/io/substrait/relation/Join.java index 490bd7315..a27c8497a 100644 --- a/core/src/main/java/io/substrait/relation/Join.java +++ b/core/src/main/java/io/substrait/relation/Join.java @@ -65,26 +65,26 @@ public static JoinType fromProto(JoinRel.JoinType proto) { protected Type.Struct deriveRecordType() { Stream leftTypes = switch (getJoinType()) { - case RIGHT, OUTER, RIGHT_SINGLE -> getLeft().getRecordType().fields().stream() - .map(TypeCreator::asNullable); - case RIGHT_SEMI, RIGHT_ANTI -> Stream - .of(); // these are right joins which ignore left side columns - case RIGHT_MARK -> Stream.of( - TypeCreator.REQUIRED - .BOOLEAN); // right mark join keeps all fields from right and adds a boolean mark - // field + case RIGHT, OUTER, RIGHT_SINGLE -> + getLeft().getRecordType().fields().stream().map(TypeCreator::asNullable); + case RIGHT_SEMI, RIGHT_ANTI -> + // these are right joins which ignore left side columns + Stream.of(); + case RIGHT_MARK -> + // right mark join keeps all fields from right and adds a boolean mark field + Stream.of(TypeCreator.REQUIRED.BOOLEAN); default -> getLeft().getRecordType().fields().stream(); }; Stream rightTypes = switch (getJoinType()) { - case LEFT, OUTER, LEFT_SINGLE -> getRight().getRecordType().fields().stream() - .map(TypeCreator::asNullable); - case SEMI, ANTI, LEFT_SEMI, LEFT_ANTI -> Stream - .of(); // these are left joins which ignore right side columns - case LEFT_MARK -> Stream.of( - TypeCreator.REQUIRED - .BOOLEAN); // left mark join keeps all fields from left and adds a boolean mark - // field + case LEFT, OUTER, LEFT_SINGLE -> + getRight().getRecordType().fields().stream().map(TypeCreator::asNullable); + case SEMI, ANTI, LEFT_SEMI, LEFT_ANTI -> + // these are left joins which ignore right side columns + Stream.of(); + case LEFT_MARK -> + // left mark join keeps all fields from left and adds a boolean mark field + Stream.of(TypeCreator.REQUIRED.BOOLEAN); default -> getRight().getRecordType().fields().stream(); }; return TypeCreator.REQUIRED.struct(Stream.concat(leftTypes, rightTypes)); diff --git a/core/src/main/java/io/substrait/relation/ProtoRelConverter.java b/core/src/main/java/io/substrait/relation/ProtoRelConverter.java index c359ea420..04f67df53 100644 --- a/core/src/main/java/io/substrait/relation/ProtoRelConverter.java +++ b/core/src/main/java/io/substrait/relation/ProtoRelConverter.java @@ -269,8 +269,8 @@ protected Rel newUpdate(UpdateRel rel) { case NAMED_TABLE -> { return newNamedUpdate(rel); } - default -> throw new UnsupportedOperationException( - "Unsupported UpdateTypeCase of " + relType); + default -> + throw new UnsupportedOperationException("Unsupported UpdateTypeCase of " + relType); } } @@ -608,17 +608,22 @@ protected Expand newExpand(ExpandRel rel) { .map( expandField -> switch (expandField.getFieldTypeCase()) { - case CONSISTENT_FIELD -> Expand.ConsistentField.builder() - .expression(converter.from(expandField.getConsistentField())) - .build(); - case SWITCHING_FIELD -> Expand.SwitchingField.builder() - .duplicates( - expandField.getSwitchingField().getDuplicatesList().stream() - .map(converter::from) - .collect(java.util.stream.Collectors.toList())) - .build(); - case FIELDTYPE_NOT_SET -> throw new UnsupportedOperationException( - "Expand fields not set"); + case CONSISTENT_FIELD -> + Expand.ConsistentField.builder() + .expression(converter.from(expandField.getConsistentField())) + .build(); + case SWITCHING_FIELD -> + Expand.SwitchingField.builder() + .duplicates( + expandField + .getSwitchingField() + .getDuplicatesList() + .stream() + .map(converter::from) + .collect(java.util.stream.Collectors.toList())) + .build(); + case FIELDTYPE_NOT_SET -> + throw new UnsupportedOperationException("Expand fields not set"); }) .collect(java.util.stream.Collectors.toList())); diff --git a/core/src/main/java/io/substrait/relation/Set.java b/core/src/main/java/io/substrait/relation/Set.java index 697cda1be..e2171ee88 100644 --- a/core/src/main/java/io/substrait/relation/Set.java +++ b/core/src/main/java/io/substrait/relation/Set.java @@ -70,8 +70,8 @@ protected Type.Struct deriveRecordType() { case UNKNOWN -> first; // alternative would be to throw an exception case MINUS_PRIMARY, MINUS_PRIMARY_ALL, MINUS_MULTISET -> first; case INTERSECTION_PRIMARY -> coalesceNullabilityIntersectionPrimary(first, rest); - case INTERSECTION_MULTISET, INTERSECTION_MULTISET_ALL -> coalesceNullabilityIntersection( - first, rest); + case INTERSECTION_MULTISET, INTERSECTION_MULTISET_ALL -> + coalesceNullabilityIntersection(first, rest); case UNION_DISTINCT, UNION_ALL -> coalesceNullabilityUnion(first, rest); }; } diff --git a/core/src/main/java/io/substrait/relation/physical/HashJoin.java b/core/src/main/java/io/substrait/relation/physical/HashJoin.java index 1d9cc3c54..21166ff8f 100644 --- a/core/src/main/java/io/substrait/relation/physical/HashJoin.java +++ b/core/src/main/java/io/substrait/relation/physical/HashJoin.java @@ -60,15 +60,15 @@ public HashJoinRel.JoinType toProto() { protected Type.Struct deriveRecordType() { Stream leftTypes = switch (getJoinType()) { - case RIGHT, OUTER -> getLeft().getRecordType().fields().stream() - .map(TypeCreator::asNullable); + case RIGHT, OUTER -> + getLeft().getRecordType().fields().stream().map(TypeCreator::asNullable); case RIGHT_ANTI, RIGHT_SEMI -> Stream.empty(); default -> getLeft().getRecordType().fields().stream(); }; Stream rightTypes = switch (getJoinType()) { - case LEFT, OUTER -> getRight().getRecordType().fields().stream() - .map(TypeCreator::asNullable); + case LEFT, OUTER -> + getRight().getRecordType().fields().stream().map(TypeCreator::asNullable); case LEFT_ANTI, LEFT_SEMI -> Stream.empty(); default -> getRight().getRecordType().fields().stream(); }; diff --git a/core/src/main/java/io/substrait/relation/physical/MergeJoin.java b/core/src/main/java/io/substrait/relation/physical/MergeJoin.java index 4f7facd32..a5f54480c 100644 --- a/core/src/main/java/io/substrait/relation/physical/MergeJoin.java +++ b/core/src/main/java/io/substrait/relation/physical/MergeJoin.java @@ -60,15 +60,15 @@ public MergeJoinRel.JoinType toProto() { protected Type.Struct deriveRecordType() { Stream leftTypes = switch (getJoinType()) { - case RIGHT, OUTER -> getLeft().getRecordType().fields().stream() - .map(TypeCreator::asNullable); + case RIGHT, OUTER -> + getLeft().getRecordType().fields().stream().map(TypeCreator::asNullable); case RIGHT_ANTI, RIGHT_SEMI -> Stream.empty(); default -> getLeft().getRecordType().fields().stream(); }; Stream rightTypes = switch (getJoinType()) { - case LEFT, OUTER -> getRight().getRecordType().fields().stream() - .map(TypeCreator::asNullable); + case LEFT, OUTER -> + getRight().getRecordType().fields().stream().map(TypeCreator::asNullable); case LEFT_ANTI, LEFT_SEMI -> Stream.empty(); default -> getRight().getRecordType().fields().stream(); }; diff --git a/core/src/main/java/io/substrait/relation/physical/NestedLoopJoin.java b/core/src/main/java/io/substrait/relation/physical/NestedLoopJoin.java index 233aff176..2ce3a6f3c 100644 --- a/core/src/main/java/io/substrait/relation/physical/NestedLoopJoin.java +++ b/core/src/main/java/io/substrait/relation/physical/NestedLoopJoin.java @@ -54,15 +54,15 @@ public static JoinType fromProto(NestedLoopJoinRel.JoinType proto) { protected Type.Struct deriveRecordType() { Stream leftTypes = switch (getJoinType()) { - case RIGHT, OUTER -> getLeft().getRecordType().fields().stream() - .map(TypeCreator::asNullable); + case RIGHT, OUTER -> + getLeft().getRecordType().fields().stream().map(TypeCreator::asNullable); case RIGHT_ANTI, RIGHT_SEMI -> Stream.empty(); default -> getLeft().getRecordType().fields().stream(); }; Stream rightTypes = switch (getJoinType()) { - case LEFT, OUTER -> getRight().getRecordType().fields().stream() - .map(TypeCreator::asNullable); + case LEFT, OUTER -> + getRight().getRecordType().fields().stream().map(TypeCreator::asNullable); case LEFT_ANTI, LEFT_SEMI -> Stream.empty(); default -> getRight().getRecordType().fields().stream(); }; diff --git a/core/src/main/java/io/substrait/type/parser/ParseToPojo.java b/core/src/main/java/io/substrait/type/parser/ParseToPojo.java index d27bd03e2..cc1d3c887 100644 --- a/core/src/main/java/io/substrait/type/parser/ParseToPojo.java +++ b/core/src/main/java/io/substrait/type/parser/ParseToPojo.java @@ -538,8 +538,9 @@ public TypeExpression visitFunctionCall(final SubstraitTypeParser.FunctionCallCo switch (name) { case "MIN" -> TypeExpression.BinaryOperation.OpType.MIN; case "MAX" -> TypeExpression.BinaryOperation.OpType.MAX; - default -> throw new IllegalStateException( - "The following operation was unrecognized: " + name); + default -> + throw new IllegalStateException( + "The following operation was unrecognized: " + name); }; return TypeExpression.BinaryOperation.builder() .opType(type) diff --git a/core/src/main/java/io/substrait/type/proto/ProtoTypeConverter.java b/core/src/main/java/io/substrait/type/proto/ProtoTypeConverter.java index e4e33bacc..c04bfa15e 100644 --- a/core/src/main/java/io/substrait/type/proto/ProtoTypeConverter.java +++ b/core/src/main/java/io/substrait/type/proto/ProtoTypeConverter.java @@ -32,32 +32,39 @@ public Type from(io.substrait.proto.Type type) { case DATE -> n(type.getDate().getNullability()).DATE; case TIME -> n(type.getTime().getNullability()).TIME; case INTERVAL_YEAR -> n(type.getIntervalYear().getNullability()).INTERVAL_YEAR; - case INTERVAL_DAY -> n(type.getIntervalDay().getNullability()) - // precision defaults to 6 (micros) for backwards compatibility, see protobuf - .intervalDay( - type.getIntervalDay().hasPrecision() ? type.getIntervalDay().getPrecision() : 6); - case INTERVAL_COMPOUND -> n(type.getIntervalCompound().getNullability()) - .intervalCompound(type.getIntervalCompound().getPrecision()); + case INTERVAL_DAY -> + n(type.getIntervalDay().getNullability()) + // precision defaults to 6 (micros) for backwards compatibility, see protobuf + .intervalDay( + type.getIntervalDay().hasPrecision() ? type.getIntervalDay().getPrecision() : 6); + case INTERVAL_COMPOUND -> + n(type.getIntervalCompound().getNullability()) + .intervalCompound(type.getIntervalCompound().getPrecision()); case TIMESTAMP_TZ -> n(type.getTimestampTz().getNullability()).TIMESTAMP_TZ; case UUID -> n(type.getUuid().getNullability()).UUID; - case FIXED_CHAR -> n(type.getFixedChar().getNullability()) - .fixedChar(type.getFixedChar().getLength()); + case FIXED_CHAR -> + n(type.getFixedChar().getNullability()).fixedChar(type.getFixedChar().getLength()); case VARCHAR -> n(type.getVarchar().getNullability()).varChar(type.getVarchar().getLength()); - case FIXED_BINARY -> n(type.getFixedBinary().getNullability()) - .fixedBinary(type.getFixedBinary().getLength()); - case DECIMAL -> n(type.getDecimal().getNullability()) - .decimal(type.getDecimal().getPrecision(), type.getDecimal().getScale()); - case PRECISION_TIME -> n(type.getPrecisionTime().getNullability()) - .precisionTime(type.getPrecisionTime().getPrecision()); - case PRECISION_TIMESTAMP -> n(type.getPrecisionTimestamp().getNullability()) - .precisionTimestamp(type.getPrecisionTimestamp().getPrecision()); - case PRECISION_TIMESTAMP_TZ -> n(type.getPrecisionTimestampTz().getNullability()) - .precisionTimestampTZ(type.getPrecisionTimestampTz().getPrecision()); - case STRUCT -> n(type.getStruct().getNullability()) - .struct( - type.getStruct().getTypesList().stream() - .map(this::from) - .collect(java.util.stream.Collectors.toList())); + case FIXED_BINARY -> + n(type.getFixedBinary().getNullability()).fixedBinary(type.getFixedBinary().getLength()); + case DECIMAL -> + n(type.getDecimal().getNullability()) + .decimal(type.getDecimal().getPrecision(), type.getDecimal().getScale()); + case PRECISION_TIME -> + n(type.getPrecisionTime().getNullability()) + .precisionTime(type.getPrecisionTime().getPrecision()); + case PRECISION_TIMESTAMP -> + n(type.getPrecisionTimestamp().getNullability()) + .precisionTimestamp(type.getPrecisionTimestamp().getPrecision()); + case PRECISION_TIMESTAMP_TZ -> + n(type.getPrecisionTimestampTz().getNullability()) + .precisionTimestampTZ(type.getPrecisionTimestampTz().getPrecision()); + case STRUCT -> + n(type.getStruct().getNullability()) + .struct( + type.getStruct().getTypesList().stream() + .map(this::from) + .collect(java.util.stream.Collectors.toList())); case LIST -> fromList(type.getList()); case MAP -> fromMap(type.getMap()); case USER_DEFINED -> { @@ -65,8 +72,8 @@ public Type from(io.substrait.proto.Type type) { var t = lookup.getType(userDefined.getTypeReference(), extensions); yield n(userDefined.getNullability()).userDefined(t.uri(), t.name()); } - case USER_DEFINED_TYPE_REFERENCE -> throw new UnsupportedOperationException( - "Unsupported user defined reference: " + type); + case USER_DEFINED_TYPE_REFERENCE -> + throw new UnsupportedOperationException("Unsupported user defined reference: " + type); case KIND_NOT_SET -> throw new UnsupportedOperationException("Type is not set: " + type); }; } diff --git a/core/src/main/java/io/substrait/type/proto/TypeExpressionProtoVisitor.java b/core/src/main/java/io/substrait/type/proto/TypeExpressionProtoVisitor.java index 1a7dcbc4f..2eed0966f 100644 --- a/core/src/main/java/io/substrait/type/proto/TypeExpressionProtoVisitor.java +++ b/core/src/main/java/io/substrait/type/proto/TypeExpressionProtoVisitor.java @@ -35,10 +35,10 @@ public DerivationExpression visit(final TypeExpression.BinaryOperation expr) { case MIN -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_MIN; case MAX -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_MAX; case LT -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_LESS_THAN; - // case LTE -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_LESS_THAN; + // case LTE -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_LESS_THAN; case GT -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_GREATER_THAN; - // case GTE -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_MINUS; - // case NOT_EQ -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_EQ; + // case GTE -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_MINUS; + // case NOT_EQ -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_EQ; case EQ -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_EQUALS; case COVERS -> DerivationExpression.BinaryOp.BinaryOpType.BINARY_OP_TYPE_COVERS; default -> throw new IllegalStateException("Unexpected value: " + expr.opType()); diff --git a/core/src/test/java/io/substrait/type/proto/LocalFilesRoundtripTest.java b/core/src/test/java/io/substrait/type/proto/LocalFilesRoundtripTest.java index 8489860bb..8905e555a 100644 --- a/core/src/test/java/io/substrait/type/proto/LocalFilesRoundtripTest.java +++ b/core/src/test/java/io/substrait/type/proto/LocalFilesRoundtripTest.java @@ -74,18 +74,20 @@ private ImmutableFileOrFiles.Builder setFileFormat( case ARROW -> builder.fileFormat(FileFormat.ArrowReadOptions.builder().build()); case ORC -> builder.fileFormat(FileFormat.OrcReadOptions.builder().build()); case DWRF -> builder.fileFormat(FileFormat.DwrfReadOptions.builder().build()); - case TEXT -> builder.fileFormat( - FileFormat.DelimiterSeparatedTextReadOptions.builder() - .fieldDelimiter("|") - .maxLineSize(1000) - .quote("\"") - .headerLinesToSkip(1) - .escape("\\") - .build()); - case EXTENSION -> builder.fileFormat( - FileFormat.Extension.builder() - .extension(com.google.protobuf.Any.newBuilder().build()) - .build()); + case TEXT -> + builder.fileFormat( + FileFormat.DelimiterSeparatedTextReadOptions.builder() + .fieldDelimiter("|") + .maxLineSize(1000) + .quote("\"") + .headerLinesToSkip(1) + .escape("\\") + .build()); + case EXTENSION -> + builder.fileFormat( + FileFormat.Extension.builder() + .extension(com.google.protobuf.Any.newBuilder().build()) + .build()); case FILEFORMAT_NOT_SET -> builder; }; } diff --git a/examples/substrait-spark/build.gradle.kts b/examples/substrait-spark/build.gradle.kts index 212f2b11c..7049a8928 100644 --- a/examples/substrait-spark/build.gradle.kts +++ b/examples/substrait-spark/build.gradle.kts @@ -1,7 +1,7 @@ plugins { // Apply the application plugin to add support for building a CLI application in Java. id("java") - id("com.diffplug.spotless") version "6.19.0" + id("com.diffplug.spotless") version "7.1.0" } repositories { diff --git a/isthmus-cli/build.gradle.kts b/isthmus-cli/build.gradle.kts index 693a01f5b..2d7674019 100644 --- a/isthmus-cli/build.gradle.kts +++ b/isthmus-cli/build.gradle.kts @@ -2,7 +2,7 @@ plugins { id("java") id("idea") id("com.palantir.graal") version "0.10.0" - id("com.diffplug.spotless") version "6.19.0" + id("com.diffplug.spotless") version "7.1.0" } java { diff --git a/isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java b/isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java index 96135999f..6acc97626 100644 --- a/isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java +++ b/isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java @@ -103,8 +103,8 @@ public Integer call() throws Exception { private void printMessage(Message message) throws IOException { switch (outputFormat) { - case PROTOJSON -> System.out.println( - JsonFormat.printer().includingDefaultValueFields().print(message)); + case PROTOJSON -> + System.out.println(JsonFormat.printer().includingDefaultValueFields().print(message)); case PROTOTEXT -> TextFormat.printer().print(message, System.out); case BINARY -> message.writeTo(System.out); } diff --git a/isthmus/build.gradle.kts b/isthmus/build.gradle.kts index 01103e023..888d40b0c 100644 --- a/isthmus/build.gradle.kts +++ b/isthmus/build.gradle.kts @@ -5,7 +5,7 @@ plugins { signing id("java-library") id("idea") - id("com.diffplug.spotless") version "6.19.0" + id("com.diffplug.spotless") version "7.1.0" id("com.gradleup.shadow") version "8.3.6" id("com.google.protobuf") version "0.9.4" id("org.jreleaser") diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java index 0f9bd08a6..a7f406969 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java @@ -211,10 +211,11 @@ public RelNode visit(Join join, Context context) throws RuntimeException { case ANTI -> JoinRelType.ANTI; case LEFT_SEMI -> JoinRelType.SEMI; case LEFT_ANTI -> JoinRelType.ANTI; - case UNKNOWN -> throw new UnsupportedOperationException( - "Unknown join type is not supported"); - default -> throw new UnsupportedOperationException( - "Unsupported join type: " + join.getJoinType().name()); + case UNKNOWN -> + throw new UnsupportedOperationException("Unknown join type is not supported"); + default -> + throw new UnsupportedOperationException( + "Unsupported join type: " + join.getJoinType().name()); }; RelNode node = relBuilder.push(left).push(right).join(joinType, condition).build(); return applyRemap(node, join.getRemap()); @@ -236,13 +237,13 @@ public RelNode visit(Set set, Context context) throws RuntimeException { switch (set.getSetOp()) { case MINUS_PRIMARY -> relBuilder.minus(false, numInputs); case MINUS_PRIMARY_ALL, MINUS_MULTISET -> relBuilder.minus(true, numInputs); - case INTERSECTION_PRIMARY, INTERSECTION_MULTISET -> relBuilder.intersect( - false, numInputs); + case INTERSECTION_PRIMARY, INTERSECTION_MULTISET -> + relBuilder.intersect(false, numInputs); case INTERSECTION_MULTISET_ALL -> relBuilder.intersect(true, numInputs); case UNION_DISTINCT -> relBuilder.union(false, numInputs); case UNION_ALL -> relBuilder.union(true, numInputs); - case UNKNOWN -> throw new UnsupportedOperationException( - "Unknown set operation is not supported"); + case UNKNOWN -> + throw new UnsupportedOperationException("Unknown set operation is not supported"); }; RelNode node = builder.build(); return applyRemap(node, set.getRemap()); @@ -371,8 +372,9 @@ private RexNode directedRexNode(Expression.SortField sortField, Context context) case ASC_NULLS_LAST -> relBuilder.nullsLast(rexNode); case DESC_NULLS_FIRST -> relBuilder.nullsFirst(relBuilder.desc(rexNode)); case DESC_NULLS_LAST -> relBuilder.nullsLast(relBuilder.desc(rexNode)); - case CLUSTERED -> throw new RuntimeException( - String.format("Unexpected Expression.SortDirection: Clustered!")); + case CLUSTERED -> + throw new RuntimeException( + String.format("Unexpected Expression.SortDirection: Clustered!")); }; } @@ -413,8 +415,9 @@ private RelFieldCollation toRelFieldCollation(Expression.SortField sortField, Co } case CLUSTERED -> fieldDirection = RelFieldCollation.Direction.CLUSTERED; - default -> throw new RuntimeException( - String.format("Unexpected Expression.SortDirection enum: %s !", sortDirection)); + default -> + throw new RuntimeException( + String.format("Unexpected Expression.SortDirection enum: %s !", sortDirection)); } return new RelFieldCollation(fieldIndex, fieldDirection, nullDirection); } diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java index fdb3f8aec..79ab8175e 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java @@ -178,8 +178,9 @@ public Rel visit(org.apache.calcite.rel.core.Join join) { case FULL -> Join.JoinType.OUTER; case SEMI -> Join.JoinType.LEFT_SEMI; case ANTI -> Join.JoinType.LEFT_ANTI; - default -> throw new UnsupportedOperationException( - "Unsupported join type: " + join.getJoinType()); + default -> + throw new UnsupportedOperationException( + "Unsupported join type: " + join.getJoinType()); }; // An INNER JOIN with a join condition of TRUE can be encoded as a Substrait Cross relation @@ -201,8 +202,9 @@ public Rel visit(org.apache.calcite.rel.core.Correlate correlate) { switch (correlate.getJoinType()) { case INNER -> Join.JoinType.INNER; // corresponds to CROSS APPLY join case LEFT -> Join.JoinType.LEFT; // corresponds to OUTER APPLY join - default -> throw new IllegalArgumentException( - "Invalid correlated join type: " + correlate.getJoinType()); + default -> + throw new IllegalArgumentException( + "Invalid correlated join type: " + correlate.getJoinType()); }; return super.visit(correlate); } @@ -328,14 +330,14 @@ public static Expression.SortField toSortField( RelFieldCollation collation, Type.Struct inputType) { Expression.SortDirection direction = switch (collation.direction) { - case STRICTLY_ASCENDING, ASCENDING -> collation.nullDirection - == RelFieldCollation.NullDirection.LAST - ? Expression.SortDirection.ASC_NULLS_LAST - : Expression.SortDirection.ASC_NULLS_FIRST; - case STRICTLY_DESCENDING, DESCENDING -> collation.nullDirection - == RelFieldCollation.NullDirection.LAST - ? Expression.SortDirection.DESC_NULLS_LAST - : Expression.SortDirection.DESC_NULLS_FIRST; + case STRICTLY_ASCENDING, ASCENDING -> + collation.nullDirection == RelFieldCollation.NullDirection.LAST + ? Expression.SortDirection.ASC_NULLS_LAST + : Expression.SortDirection.ASC_NULLS_FIRST; + case STRICTLY_DESCENDING, DESCENDING -> + collation.nullDirection == RelFieldCollation.NullDirection.LAST + ? Expression.SortDirection.DESC_NULLS_LAST + : Expression.SortDirection.DESC_NULLS_FIRST; case CLUSTERED -> Expression.SortDirection.CLUSTERED; }; diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index c48943ca1..b32f44d19 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -94,15 +94,16 @@ private Type toSubstrait(RelDataType type, List names) { case TIMESTAMP_WITH_LOCAL_TIME_ZONE -> creator.precisionTimestampTZ(type.getPrecision()); case INTERVAL_YEAR, INTERVAL_YEAR_MONTH, INTERVAL_MONTH -> creator.INTERVAL_YEAR; case INTERVAL_DAY, - INTERVAL_DAY_HOUR, - INTERVAL_DAY_MINUTE, - INTERVAL_DAY_SECOND, - INTERVAL_HOUR, - INTERVAL_HOUR_MINUTE, - INTERVAL_HOUR_SECOND, - INTERVAL_MINUTE, - INTERVAL_MINUTE_SECOND, - INTERVAL_SECOND -> creator.intervalDay(type.getScale()); + INTERVAL_DAY_HOUR, + INTERVAL_DAY_MINUTE, + INTERVAL_DAY_SECOND, + INTERVAL_HOUR, + INTERVAL_HOUR_MINUTE, + INTERVAL_HOUR_SECOND, + INTERVAL_MINUTE, + INTERVAL_MINUTE_SECOND, + INTERVAL_SECOND -> + creator.intervalDay(type.getScale()); case VARBINARY -> creator.BINARY; case BINARY -> creator.fixedBinary(type.getPrecision()); case MAP -> { @@ -119,8 +120,9 @@ private Type toSubstrait(RelDataType type, List names) { yield creator.struct(children); } case ARRAY -> creator.list(toSubstrait(type.getComponentType(), names)); - default -> throw new UnsupportedOperationException( - String.format("Unable to convert the type " + type.toString())); + default -> + throw new UnsupportedOperationException( + String.format("Unable to convert the type " + type.toString())); }; } @@ -348,8 +350,9 @@ private RelDataType t(boolean nullable, SqlTypeName typeName, Integer... props) case 0 -> typeFactory.createSqlType(typeName); case 1 -> typeFactory.createSqlType(typeName, props[0]); case 2 -> typeFactory.createSqlType(typeName, props[0], props[1]); - default -> throw new IllegalArgumentException( - "Unexpected properties length: " + Arrays.toString(props)); + default -> + throw new IllegalArgumentException( + "Unexpected properties length: " + Arrays.toString(props)); }; return typeFactory.createTypeWithNullability(baseType, nullable); diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java index bd433ccbe..e4dff992d 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java @@ -430,8 +430,9 @@ public RexNode visit(Expression.WindowFunctionInvocation expr, Context context) case ASC_NULLS_LAST -> Set.of(SqlKind.NULLS_LAST); case DESC_NULLS_FIRST -> Set.of(SqlKind.DESCENDING, SqlKind.NULLS_FIRST); case DESC_NULLS_LAST -> Set.of(SqlKind.DESCENDING, SqlKind.NULLS_LAST); - case CLUSTERED -> throw new IllegalArgumentException( - "SORT_DIRECTION_CLUSTERED is not supported"); + case CLUSTERED -> + throw new IllegalArgumentException( + "SORT_DIRECTION_CLUSTERED is not supported"); }; return new RexFieldCollation(sf.expr().accept(this, context), direction); }) @@ -444,8 +445,9 @@ public RexNode visit(Expression.WindowFunctionInvocation expr, Context context) switch (expr.boundsType()) { case ROWS -> true; case RANGE -> false; - case UNSPECIFIED -> throw new IllegalArgumentException( - "bounds type on window function must be specified"); + case UNSPECIFIED -> + throw new IllegalArgumentException( + "bounds type on window function must be specified"); }; boolean distinct = diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java index cf654b36e..94ab80b74 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java @@ -124,14 +124,15 @@ public Expression.Literal convert(RexLiteral literal) { yield ExpressionCreator.varChar(n, s(literal), literal.getType().getPrecision()); } - case BINARY -> ExpressionCreator.fixedBinary( - n, - ByteString.copyFrom( - padRightIfNeeded( - literal.getValueAs(org.apache.calcite.avatica.util.ByteString.class), - literal.getType().getPrecision()))); - case VARBINARY -> ExpressionCreator.binary( - n, ByteString.copyFrom(literal.getValueAs(byte[].class))); + case BINARY -> + ExpressionCreator.fixedBinary( + n, + ByteString.copyFrom( + padRightIfNeeded( + literal.getValueAs(org.apache.calcite.avatica.util.ByteString.class), + literal.getType().getPrecision()))); + case VARBINARY -> + ExpressionCreator.binary(n, ByteString.copyFrom(literal.getValueAs(byte[].class))); case SYMBOL -> { Object value = literal.getValue(); // case TimeUnitRange tur -> string(n, tur.name()); @@ -203,10 +204,11 @@ public Expression.Literal convert(RexLiteral literal) { n, literals.stream().map(this::convert).collect(Collectors.toList())); } - default -> throw new UnsupportedOperationException( - String.format( - "Unable to convert the value of %s of type %s to a literal.", - literal, literal.getType().getSqlTypeName())); + default -> + throw new UnsupportedOperationException( + String.format( + "Unable to convert the value of %s of type %s to a literal.", + literal, literal.getType().getSqlTypeName())); }; } diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/RexExpressionConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/RexExpressionConverter.java index 2bc7ec534..e60c94156 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/RexExpressionConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/RexExpressionConverter.java @@ -146,8 +146,9 @@ public Expression visitFieldAccess(RexFieldAccess fieldAccess) { return FieldReference.newStructReference(fieldAccess.getField().getIndex(), expression); } } - default -> throw new UnsupportedOperationException( - String.format("RexFieldAccess for SqlKind %s not supported", kind)); + default -> + throw new UnsupportedOperationException( + String.format("RexFieldAccess for SqlKind %s not supported", kind)); } } diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/SortFieldConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/SortFieldConverter.java index 9cfbf9db5..de56718cd 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/SortFieldConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/SortFieldConverter.java @@ -13,18 +13,19 @@ public static Expression.SortField toSortField( var rexDirection = rexFieldCollation.getDirection(); Expression.SortDirection direction = switch (rexDirection) { - case ASCENDING -> rexFieldCollation.getNullDirection() - == RelFieldCollation.NullDirection.LAST - ? Expression.SortDirection.ASC_NULLS_LAST - : Expression.SortDirection.ASC_NULLS_FIRST; - case DESCENDING -> rexFieldCollation.getNullDirection() - == RelFieldCollation.NullDirection.LAST - ? Expression.SortDirection.DESC_NULLS_LAST - : Expression.SortDirection.DESC_NULLS_FIRST; - default -> throw new IllegalArgumentException( - String.format( - "Unexpected RelFieldCollation.Direction:%s enum at the RexFieldCollation!", - rexDirection)); + case ASCENDING -> + rexFieldCollation.getNullDirection() == RelFieldCollation.NullDirection.LAST + ? Expression.SortDirection.ASC_NULLS_LAST + : Expression.SortDirection.ASC_NULLS_FIRST; + case DESCENDING -> + rexFieldCollation.getNullDirection() == RelFieldCollation.NullDirection.LAST + ? Expression.SortDirection.DESC_NULLS_LAST + : Expression.SortDirection.DESC_NULLS_FIRST; + default -> + throw new IllegalArgumentException( + String.format( + "Unexpected RelFieldCollation.Direction:%s enum at the RexFieldCollation!", + rexDirection)); }; return Expression.SortField.builder().expr(expr).direction(direction).build(); diff --git a/isthmus/src/test/java/io/substrait/isthmus/AggregationFunctionsTest.java b/isthmus/src/test/java/io/substrait/isthmus/AggregationFunctionsTest.java index 62357aa3d..ce8315d97 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/AggregationFunctionsTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/AggregationFunctionsTest.java @@ -47,8 +47,8 @@ private Aggregate.Measure functionPicker(Rel input, int field, String fname) { case "sum" -> b.sum(input, field); case "sum0" -> b.sum0(input, field); case "avg" -> b.avg(input, field); - default -> throw new RuntimeException( - String.format("no function is associated with %s", fname)); + default -> + throw new RuntimeException(String.format("no function is associated with %s", fname)); }; } diff --git a/isthmus/src/test/java/io/substrait/isthmus/NameRoundtripTest.java b/isthmus/src/test/java/io/substrait/isthmus/NameRoundtripTest.java index bb4dcd746..79ff3d222 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/NameRoundtripTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/NameRoundtripTest.java @@ -21,9 +21,10 @@ void preserveNamesFromSql() throws Exception { SqlToSubstrait s = new SqlToSubstrait(); var substraitToCalcite = new SubstraitToCalcite(EXTENSION_COLLECTION, typeFactory); - String query = """ - SELECT "a", "B" FROM foo GROUP BY a, b - """; + String query = + """ + SELECT "a", "B" FROM foo GROUP BY a, b + """; List expectedNames = List.of("a", "B"); List calciteRelRoots = s.sqlToRelNode(query, catalogReader); diff --git a/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java b/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java index deac90e87..e7690c5e6 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java +++ b/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java @@ -26,8 +26,8 @@ public static String getSetQuery(Set.SetOp op, boolean multi) { case INTERSECTION_MULTISET_ALL -> "INTERSECT ALL"; case UNION_DISTINCT -> "UNION"; case UNION_ALL -> "UNION ALL"; - default -> throw new UnsupportedOperationException( - "Unknown set operation is not supported"); + default -> + throw new UnsupportedOperationException("Unknown set operation is not supported"); }; StringBuilder query = new StringBuilder(); diff --git a/spark/.scalafmt.conf b/spark/.scalafmt.conf index fc6b14ef6..3606e3439 100644 --- a/spark/.scalafmt.conf +++ b/spark/.scalafmt.conf @@ -1,7 +1,7 @@ runner.dialect = scala212 # Version is required to make sure IntelliJ picks the right version -version = 3.7.3 +version = 3.8.1 preset = default # Max column diff --git a/spark/build.gradle.kts b/spark/build.gradle.kts index 0acec7036..bf82084b8 100644 --- a/spark/build.gradle.kts +++ b/spark/build.gradle.kts @@ -4,7 +4,7 @@ plugins { id("java") id("scala") id("idea") - id("com.diffplug.spotless") version "6.19.0" + id("com.diffplug.spotless") version "7.1.0" id("org.jreleaser") }