diff --git a/spark/src/main/scala/io/substrait/spark/expression/ToSparkExpression.scala b/spark/src/main/scala/io/substrait/spark/expression/ToSparkExpression.scala index 7aaca95bd..749caae6a 100644 --- a/spark/src/main/scala/io/substrait/spark/expression/ToSparkExpression.scala +++ b/spark/src/main/scala/io/substrait/spark/expression/ToSparkExpression.scala @@ -148,8 +148,11 @@ class ToSparkExpression( override def visit( expr: SExpression.IntervalYearLiteral, context: EmptyVisitationContext): Literal = { - // Spark uses a single months Int as the "physical" type for YearMonthInterval - val months = expr.years() * 12 + expr.months() + // Spark uses a single months Int as the "physical" type for YearMonthInterval, and both + // components are int32, so the flattened total can outrun that carrier: a large year count + // used to wrap a positive interval into a negative one. Only the total is significant, so + // the check belongs on it rather than on the intermediate product. + val months = Math.toIntExact(expr.years() * Util.MONTHS_PER_YEAR + expr.months()) Literal(months, ToSparkType.convert(expr.getType)) } diff --git a/spark/src/main/scala/io/substrait/spark/utils/Util.scala b/spark/src/main/scala/io/substrait/spark/utils/Util.scala index 7ffbe5512..7243a7ca7 100644 --- a/spark/src/main/scala/io/substrait/spark/utils/Util.scala +++ b/spark/src/main/scala/io/substrait/spark/utils/Util.scala @@ -23,6 +23,7 @@ object Util { val SECONDS_PER_DAY: Long = 24 * 60 * 60 val MICROS_PER_SECOND: Long = 1000 * 1000 + val MONTHS_PER_YEAR: Long = 12 val MICROSECOND_PRECISION = 6 // for PrecisionTimestamp(TZ) and IntervalDay types /** Indexed by exponent, which [[toMicroseconds]] bounds to 0..MICROSECOND_PRECISION. */ diff --git a/spark/src/test/scala/io/substrait/spark/TypesAndLiteralsSuite.scala b/spark/src/test/scala/io/substrait/spark/TypesAndLiteralsSuite.scala index 8d1b1c491..8a6d2078f 100644 --- a/spark/src/test/scala/io/substrait/spark/TypesAndLiteralsSuite.scala +++ b/spark/src/test/scala/io/substrait/spark/TypesAndLiteralsSuite.scala @@ -307,6 +307,28 @@ class TypesAndLiteralsSuite extends SparkFunSuite { sparkLiteral(ExpressionCreator.intervalDay(false, Int.MaxValue, 0, 0L, 6))) } + test("a year-month interval reports overflow instead of wrapping") { + // years and months are both int32 and Spark's physical type is a months Int, so the flattened + // total can outrun the carrier: 178,956,971 years used to come out as -2,147,483,644 months, + // and Int.MinValue years as exactly 0 — a zero-length interval. This is the carrier's bound, + // not the spec's much tighter 10,000-year one, so it takes a producer already far past that. + intercept[ArithmeticException]( + sparkLiteral(ExpressionCreator.intervalYear(false, 178956971, 0))) + intercept[ArithmeticException]( + sparkLiteral(ExpressionCreator.intervalYear(false, 178956970, 8))) + intercept[ArithmeticException]( + sparkLiteral(ExpressionCreator.intervalYear(false, Int.MinValue, 0))) + + // Only the total is significant, so an intermediate past Int.MaxValue is not itself an error. + val mixedSigns = sparkLiteral(ExpressionCreator.intervalYear(false, 178956971, -12)) + assert(mixedSigns.value === 2147483640) + + // The spec's maximum still converts, and the months component carries through. + val atTheBound = sparkLiteral(ExpressionCreator.intervalYear(false, 9999, 12)) + assert(atTheBound.value === 120000) + assert(atTheBound.dataType === YearMonthIntervalType.DEFAULT) + } + test("a coarser precision on a type is rejected, since a type has no value to rescale") { // A Spark type carries no precision of its own, so mapping precision_timestamp<3> onto // TimestampNTZType would reinterpret millisecond counts as microsecond ones. Only the literal