diff --git a/buildSrc/src/main/resources/substrait-pmd.xml b/buildSrc/src/main/resources/substrait-pmd.xml index 65ef55578..829996045 100644 --- a/buildSrc/src/main/resources/substrait-pmd.xml +++ b/buildSrc/src/main/resources/substrait-pmd.xml @@ -12,4 +12,5 @@ + diff --git a/core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java b/core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java index 0e7593d27..60cba7165 100644 --- a/core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java +++ b/core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java @@ -372,7 +372,7 @@ public Expression visit( .setValue(Any.parseFrom(expr.value()))) .build(); } catch (InvalidProtocolBufferException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } }); } 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 db47cb278..859ddfca5 100644 --- a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java +++ b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java @@ -426,7 +426,7 @@ public Expression.Literal from(io.substrait.proto.Expression.Literal literal) { case INTERVAL_COMPOUND: { if (!literal.getIntervalCompound().getIntervalDayToSecond().hasPrecision()) { - throw new RuntimeException( + throw new UnsupportedOperationException( "Interval compound with deprecated version of interval day (ie. no precision) is not supported"); } return ExpressionCreator.intervalCompound( diff --git a/core/src/main/java/io/substrait/extension/SimpleExtension.java b/core/src/main/java/io/substrait/extension/SimpleExtension.java index 4b4cf1a90..58f46abff 100644 --- a/core/src/main/java/io/substrait/extension/SimpleExtension.java +++ b/core/src/main/java/io/substrait/extension/SimpleExtension.java @@ -21,6 +21,7 @@ import io.substrait.util.Util; import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -736,7 +737,7 @@ public static ExtensionCollection load(List resourcePaths) { try (InputStream stream = ExtensionCollection.class.getResourceAsStream(path)) { return load(path, stream); } catch (IOException e) { - throw new RuntimeException(e); + throw new UncheckedIOException(e); } }) .collect(Collectors.toList()); @@ -752,7 +753,7 @@ public static ExtensionCollection load(String namespace, String str) { ExtensionSignatures doc = objectMapper(namespace).readValue(str, ExtensionSignatures.class); return buildExtensionCollection(namespace, doc); } catch (JsonProcessingException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } @@ -764,7 +765,7 @@ public static ExtensionCollection load(String namespace, InputStream stream) { } catch (RuntimeException ex) { throw ex; } catch (Exception ex) { - throw new RuntimeException("Failure while parsing " + namespace, ex); + throw new IllegalStateException("Failure while parsing " + namespace, ex); } } diff --git a/core/src/main/java/io/substrait/relation/ProtoRelConverter.java b/core/src/main/java/io/substrait/relation/ProtoRelConverter.java index 37b45d9ce..5417ff807 100644 --- a/core/src/main/java/io/substrait/relation/ProtoRelConverter.java +++ b/core/src/main/java/io/substrait/relation/ProtoRelConverter.java @@ -939,7 +939,7 @@ protected Extension.Optimization optimizationFromAdvancedExtension(com.google.pr * io.substrait.proto.AdvancedExtension#getEnhancement()} data */ protected Extension.Enhancement enhancementFromAdvancedExtension(com.google.protobuf.Any any) { - throw new RuntimeException("enhancements cannot be ignored by consumers"); + throw new IllegalStateException("enhancements cannot be ignored by consumers"); } /** Override to provide a custom converter for {@link ExtensionLeafRel#getDetail()} data */ diff --git a/core/src/main/java/io/substrait/relation/RelProtoConverter.java b/core/src/main/java/io/substrait/relation/RelProtoConverter.java index 54fdaa433..da435f451 100644 --- a/core/src/main/java/io/substrait/relation/RelProtoConverter.java +++ b/core/src/main/java/io/substrait/relation/RelProtoConverter.java @@ -296,7 +296,7 @@ public Rel visit(HashJoin hashJoin, EmptyVisitationContext context) throws Runti List rightKeys = hashJoin.getRightKeys(); if (leftKeys.size() != rightKeys.size()) { - throw new RuntimeException("Number of left and right keys must be equal."); + throw new IllegalArgumentException("Number of left and right keys must be equal."); } builder.addAllLeftKeys(leftKeys.stream().map(this::toProto).collect(Collectors.toList())); @@ -321,7 +321,7 @@ public Rel visit(MergeJoin mergeJoin, EmptyVisitationContext context) throws Run List rightKeys = mergeJoin.getRightKeys(); if (leftKeys.size() != rightKeys.size()) { - throw new RuntimeException("Number of left and right keys must be equal."); + throw new IllegalArgumentException("Number of left and right keys must be equal."); } builder.addAllLeftKeys(leftKeys.stream().map(this::toProto).collect(Collectors.toList())); @@ -535,7 +535,7 @@ public Rel visit(Expand expand, EmptyVisitationContext context) throws RuntimeEx .addAllDuplicates(toProto(sf.getDuplicates()))) .build()); } else { - throw new RuntimeException( + throw new IllegalArgumentException( "Consistent or Switching fields must be set for the Expand relation."); } }); diff --git a/core/src/main/java/io/substrait/type/YamlRead.java b/core/src/main/java/io/substrait/type/YamlRead.java index 7a9afe476..23123bd92 100644 --- a/core/src/main/java/io/substrait/type/YamlRead.java +++ b/core/src/main/java/io/substrait/type/YamlRead.java @@ -72,7 +72,7 @@ private static Stream parse(String name) { } catch (RuntimeException ex) { throw ex; } catch (Exception ex) { - throw new RuntimeException("Failure while parsing file " + name, ex); + throw new IllegalStateException("Failure while parsing file " + name, ex); } } } diff --git a/core/src/test/java/io/substrait/relation/utils/StringHolderHandlingProtoRelConverter.java b/core/src/test/java/io/substrait/relation/utils/StringHolderHandlingProtoRelConverter.java index c2ce7c687..2a9f944c1 100644 --- a/core/src/test/java/io/substrait/relation/utils/StringHolderHandlingProtoRelConverter.java +++ b/core/src/test/java/io/substrait/relation/utils/StringHolderHandlingProtoRelConverter.java @@ -24,7 +24,7 @@ StringHolder asStringHolder(Any any) { try { return new StringHolder(any.unpack(StringValue.class).getValue()); } catch (InvalidProtocolBufferException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } diff --git a/examples/substrait-spark/build.gradle.kts b/examples/substrait-spark/build.gradle.kts index 8f4124164..00da24cc0 100644 --- a/examples/substrait-spark/build.gradle.kts +++ b/examples/substrait-spark/build.gradle.kts @@ -2,6 +2,7 @@ plugins { // Apply the application plugin to add support for building a CLI application in Java. id("java") id("com.diffplug.spotless") version "7.1.0" + id("substrait.java-conventions") } repositories { @@ -37,3 +38,5 @@ tasks.named("test") { } java { toolchain { languageVersion.set(JavaLanguageVersion.of(17)) } } + +tasks.pmdMain { dependsOn(":core:shadowJar") } 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 047fe4405..cbdd4bc60 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 @@ -118,7 +118,7 @@ public void beforeAnalysis(BeforeAnalysisAccess access) { if (c.method != null) RuntimeReflection.register(c.method); }); } catch (Exception e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java index 804244c22..4ea6bc19d 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java @@ -1,6 +1,6 @@ package io.substrait.isthmus; -import static io.substrait.isthmus.SqlToSubstrait.EXTENSION_COLLECTION; +import static io.substrait.isthmus.SqlConverterBase.EXTENSION_COLLECTION; import com.google.common.collect.ImmutableList; import io.substrait.expression.Expression; @@ -415,7 +415,8 @@ private RexNode directedRexNode(Expression.SortField sortField, Context context) return relBuilder.nullsLast(relBuilder.desc(rexNode)); } if (sortDirection == Expression.SortDirection.CLUSTERED) { - throw new RuntimeException(String.format("Unexpected Expression.SortDirection: Clustered!")); + throw new UnsupportedOperationException( + String.format("Unexpected Expression.SortDirection: Clustered!")); } throw new IllegalArgumentException("Unsupported sort direction: " + sortDirection); @@ -428,10 +429,12 @@ public RelNode visit(Fetch fetch, Context context) throws RuntimeException { long count = optCount.orElse(-1L); var offset = fetch.getOffset(); if (offset > Integer.MAX_VALUE) { - throw new RuntimeException(String.format("offset is overflowed as an integer: %d", offset)); + throw new IllegalArgumentException( + String.format("offset is overflowed as an integer: %d", offset)); } if (count > Integer.MAX_VALUE) { - throw new RuntimeException(String.format("count is overflowed as an integer: %d", count)); + throw new IllegalArgumentException( + String.format("count is overflowed as an integer: %d", count)); } RelNode node = relBuilder.push(child).limit((int) offset, (int) count).build(); return applyRemap(node, fetch.getRemap()); @@ -463,7 +466,7 @@ private RelFieldCollation toRelFieldCollation(Expression.SortField sortField, Co fieldDirection = RelFieldCollation.Direction.CLUSTERED; nullDirection = RelFieldCollation.NullDirection.UNSPECIFIED; } else { - throw new RuntimeException( + throw new UnsupportedOperationException( String.format("Unexpected Expression.SortDirection enum: %s !", sortDirection)); } diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java index f55975032..9ce5bc39a 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java @@ -132,7 +132,7 @@ public Optional getSqlOperatorFromSubstraitFunc(String key, Type ou if (resolvedOperators.size() == 1) { return Optional.of(resolvedOperators.get(0)); } else if (resolvedOperators.size() > 1) { - throw new RuntimeException( + throw new IllegalStateException( String.format( "Found %d SqlOperators: %s for ScalarFunction %s: ", resolvedOperators.size(), resolvedOperators, key)); diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/ListSqlOperatorFunctions.java b/isthmus/src/main/java/io/substrait/isthmus/expression/ListSqlOperatorFunctions.java index 713b26037..e785537b4 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ListSqlOperatorFunctions.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/ListSqlOperatorFunctions.java @@ -30,7 +30,7 @@ public static void main(String[] args) { SqlOperator op = (SqlOperator) f.get(null); return true; } catch (IllegalAccessException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } }) .filter(f -> Modifier.isStatic(f.getModifiers()) && Modifier.isPublic(f.getModifiers())) @@ -44,7 +44,7 @@ private static SqlOperator toOp(Field f) { try { return (SqlOperator) f.get(null); } catch (IllegalAccessException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } } diff --git a/isthmus/src/test/java/io/substrait/isthmus/AggregationFunctionsTest.java b/isthmus/src/test/java/io/substrait/isthmus/AggregationFunctionsTest.java index 32d4b2553..32cfcdaf0 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/AggregationFunctionsTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/AggregationFunctionsTest.java @@ -53,7 +53,8 @@ private Aggregate.Measure functionPicker(Rel input, int field, String fname) { case "avg": return b.avg(input, field); default: - throw new RuntimeException(String.format("no function is associated with %s", fname)); + throw new UnsupportedOperationException( + String.format("no function is associated with %s", fname)); } } diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java index cb08298b2..fb9ba34d9 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java @@ -1,6 +1,6 @@ package io.substrait.isthmus; -import static io.substrait.isthmus.SqlToSubstrait.EXTENSION_COLLECTION; +import static io.substrait.isthmus.SqlConverterBase.EXTENSION_COLLECTION; import static io.substrait.isthmus.SubstraitTypeSystem.YEAR_MONTH_INTERVAL; import static org.junit.jupiter.api.Assertions.assertEquals; diff --git a/isthmus/src/test/java/io/substrait/isthmus/CustomFunctionTest.java b/isthmus/src/test/java/io/substrait/isthmus/CustomFunctionTest.java index de63c38d1..e0a2399dc 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CustomFunctionTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CustomFunctionTest.java @@ -20,6 +20,7 @@ import io.substrait.type.Type; import io.substrait.type.TypeCreator; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.List; import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.type.RelDataType; @@ -47,7 +48,7 @@ public class CustomFunctionTest extends PlanTestBase { try { FUNCTIONS_CUSTOM = asString("extensions/functions_custom.yaml"); } catch (IOException e) { - throw new RuntimeException(e); + throw new UncheckedIOException(e); } } diff --git a/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java b/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java index 6856c6ba8..3cd4723f2 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java +++ b/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java @@ -61,7 +61,7 @@ public static String asString(String resource) throws IOException { TPCH_CATALOG = SubstraitCreateStatementParser.processCreateStatementsToCatalog(tpchCreateStatements); } catch (IOException | SqlParseException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } diff --git a/isthmus/src/test/java/io/substrait/isthmus/RelCreator.java b/isthmus/src/test/java/io/substrait/isthmus/RelCreator.java index f10ab5fa8..0f96dac44 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/RelCreator.java +++ b/isthmus/src/test/java/io/substrait/isthmus/RelCreator.java @@ -61,7 +61,7 @@ public RelRoot parse(String sql) { RelRoot root = converter.convertQuery(parsed, true, true); return root; } catch (SqlParseException e) { - throw new RuntimeException(e); + throw new IllegalArgumentException(e); } } diff --git a/isthmus/src/test/java/io/substrait/isthmus/RelExtensionRoundtripTest.java b/isthmus/src/test/java/io/substrait/isthmus/RelExtensionRoundtripTest.java index b1ff2d46c..0a33b9ae8 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/RelExtensionRoundtripTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/RelExtensionRoundtripTest.java @@ -165,7 +165,7 @@ ColumnAppendDetail unpack(Any any) { .from(proto.getLiteral())); return new ColumnAppendDetail(literal); } catch (InvalidProtocolBufferException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } @@ -208,7 +208,7 @@ public RelNode visit(ExtensionLeaf extensionLeaf, Context context) { return new ColumnAppenderRel( relBuilder.getCluster(), traits, literal, Collections.emptyList()); } - throw new RuntimeException("detail was not ColumnAppendDetail"); + throw new UnsupportedOperationException("detail was not ColumnAppendDetail"); } @Override @@ -220,7 +220,7 @@ public RelNode visit(ExtensionSingle extensionSingle, Context context) throws Ru return new ColumnAppenderRel( input.getCluster(), input.getTraitSet(), literal, List.of(input)); } - throw new RuntimeException("detail was not ColumnAppendDetail"); + throw new UnsupportedOperationException("detail was not ColumnAppendDetail"); } @Override @@ -235,7 +235,7 @@ public RelNode visit(ExtensionMulti extensionMulti, Context context) throws Runt return new ColumnAppenderRel( inputs.get(0).getCluster(), inputs.get(0).getTraitSet(), literal, inputs); } - throw new RuntimeException("detail was not ColumnAppendDetail"); + throw new UnsupportedOperationException("detail was not ColumnAppendDetail"); } } diff --git a/spark/src/main/scala/io/substrait/spark/logical/ToLogicalPlan.scala b/spark/src/main/scala/io/substrait/spark/logical/ToLogicalPlan.scala index 3630a6e70..5472d94c6 100644 --- a/spark/src/main/scala/io/substrait/spark/logical/ToLogicalPlan.scala +++ b/spark/src/main/scala/io/substrait/spark/logical/ToLogicalPlan.scala @@ -220,7 +220,8 @@ class ToLogicalPlan(spark: SparkSession = SparkSession.builder().getOrCreate()) case SExpression.SortDirection.ASC_NULLS_LAST => (Ascending, NullsLast) case SExpression.SortDirection.DESC_NULLS_LAST => (Descending, NullsLast) case other => - throw new RuntimeException(s"Unexpected Expression.SortDirection enum: $other !") + throw new UnsupportedOperationException( + s"Unexpected Expression.SortDirection enum: $other !") } SortOrder(expression, direction, nullOrdering, Seq.empty) }