isthmus: interval_year conversion wraps on int overflow in both directions

[nielspardon]

ExpressionRexConverter.visit(Expression.IntervalYearLiteral, Context) (isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java:414) flattens years and months with unchecked int arithmetic:

new BigDecimal(expr.years() * 12 + expr.months()), YEAR_MONTH_INTERVAL),

years() and months() are both declared int (core/src/main/java/io/substrait/expression/Expression.java:580, 587) and BigDecimal(int) binds exactly, so the multiply wraps before anything widens. Verified by running the visitor: years = 178956971 yields a RexLiteral of -2147483644, Int.MaxValue yields -12, and Int.MinValue yields exactly 0 — a large positive interval read back as a large negative one, or as a zero-length one.

Nothing downstream rejects it. RexBuilder.makeIntervalLiteral's magnitude guard applies only to DECIMAL, RexLiteral.valueMatchesType for INTERVAL_YEAR_MONTH only checks instanceof BigDecimal, and the qualifier at ExpressionRexConverter.java:71-77 uses the -1/-1 defaults. CalciteLiteralTest covers only years 3 and 123.

The reverse direction narrows unchecked too

LiteralConverter (isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java:317-320) computes in long and then truncates:

long intervalLength = Objects.requireNonNull(literal.getValueAs(Long.class));
long years = intervalLength / 12;
long months = intervalLength - years * 12;
return ExpressionCreator.intervalYear(nullable, (int) years, (int) months);

(int) months is safe — the remainder is bounded to ±11 — but (int) years is not. This is reachable through the supported public entry point SubstraitRelVisitor.convert(RelNode, ExtensionCollection): a host Calcite application under Calcite's own RelDataTypeSystem.DEFAULT may declare INTERVAL YEAR(10), since getMaxPrecision(INTERVAL_YEAR) is 10 there, so 9,999,999,999 years validates. Feeding such a RelNode (119999999988:INTERVAL YEAR(10) TO MONTH) to convert returned IntervalYearLiteral{years=1410065407, months=0} with no exception. It is not reachable through isthmus' own SQL leg, where SubstraitTypeSystem.getMaxPrecision caps INTERVAL_YEAR at 6.

The day-time literal one case below has the same shape: ExpressionCreator.intervalDay(nullable, (int) days, (int) seconds, subseconds, precision) at LiteralConverter.java:355-356, where seconds is bounded by construction but days is not.

Honest severity

Not reachable from a spec-conformant plan. The spec bounds interval_year to [-10,000..10,000] years per component, so overflow needs roughly 17,900x the legal maximum on the Substrait→Calcite side. The Calcite→Substrait side is easier to reach, since Calcite's own default type system permits INTERVAL YEAR(10), but it still needs a caller-built RelNode rather than SQL.

#1129 remains the primary fix for the inbound direction: a per-component check in core would make ExpressionRexConverter.java:414 unreachable rather than merely wrong. It would not cover LiteralConverter, which is where an out-of-spec literal is manufactured.

Fix

Substrait→Calcite needs no exception at all — the target is an unbounded BigDecimal of months, so widening is lossless for every int32 input:

new BigDecimal((long) expr.years() * 12 + expr.months()), YEAR_MONTH_INTERVAL),

Calcite→Substrait has a narrower carrier and does need to report: Math.toIntExact(years) in place of (int) years, and the same for (int) days in the day-time case.

Found while reviewing #1140, which fixes the same wrap in the Spark converter.