Skip to content

spark: a narrowed YearMonthIntervalType or DayTimeIntervalType is rejected although the value maps losslessly #1238

Description

@nielspardon

ToSubstraitType.convert (spark/src/main/scala/io/substrait/spark/ToSubstraitType.scala:158-159) and ToSubstraitLiteral (spark/src/main/scala/io/substrait/spark/expression/ToSubstraitLiteral.scala:106-109) match the default interval types only:

case DayTimeIntervalType.DEFAULT => Some(creator.intervalDay(Util.MICROSECOND_PRECISION))
case YearMonthIntervalType.DEFAULT => Some(creator.INTERVAL_YEAR)

YearMonthIntervalType.DEFAULT is YearMonthIntervalType(YEAR, MONTH), so a narrowed type — YearMonthIntervalType(YEAR, YEAR), which Spark's DDL and parser produce for INTERVAL YEAR — falls through to case _ => None / case _ => null and the conversion fails with Unable to convert the type interval year. The same holds for DayTimeIntervalType(HOUR, HOUR) and the rest of the day-time field combinations.

Why the mapping would be lossless

Substrait has no notion of interval start/end fields. interval_year is a years/months pair and interval_day<P> is days/seconds/subseconds; a narrowed Spark type carries the same physical value (a months Int, a micros Long) and only restricts which fields Spark will render. So converting YearMonthIntervalType(YEAR, YEAR) to interval_year loses only the rendering hint, not the value.

The asymmetry only bites Spark-authored plans: ToSparkType always emits .DEFAULT on the way in, so a round trip through Substrait normalizes the fields and never produces a narrowed type of its own.

Open question

Whether normalizing to the default on the way out is acceptable, or whether the field restriction should be preserved somehow. Substrait has nowhere to put it, so the realistic choice is between converting and normalizing (a Spark-visible behaviour change on round trip: INTERVAL YEAR comes back as INTERVAL YEAR TO MONTH) and continuing to reject. Rejecting is arguably the honest option, in which case the fix is a clearer message than the generic one — the current failure gives no hint that the fields are the problem rather than the type.

Found while reviewing #1140.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions