[WIP][SPARK-35680][SQL] Support units by the year-month interval type #32825
Conversation
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #139499 has finished for PR 32825 at commit
|
| .map(t => t.typeName -> t).toMap | ||
| } | ||
|
|
||
| /** Given the string representation of a type, return its DataType */ | ||
| private def nameToType(name: String): DataType = { | ||
| name match { | ||
| case "decimal" => DecimalType.USER_DEFAULT | ||
| case "interval year to month" => YearMonthIntervalType() |
There was a problem hiding this comment.
shall we follow FIXED_DECIMAL and write a regex to support different units?
There was a problem hiding this comment.
This is just stub to pass compiler errors. I don't think manually parsing is the right approach. I do believe that:
- Having two type parsers is bad idea
- We should use existing SQL parser.
There was a problem hiding this comment.
I'm happy to see a refactor here to use the SQL parser instead.
| case (YEAR, MONTH) => "interval year to month" | ||
| case (YEAR, YEAR) => "interval year" | ||
| case (MONTH, MONTH) => "interval month" | ||
| case _ => throw new AnalysisException( |
There was a problem hiding this comment.
IllegalArgumentException?
There was a problem hiding this comment.
Since it's user-facing, can we put it in QueryCompilationError?
There was a problem hiding this comment.
I followed the existing approach:
| * | ||
| * @since 3.2.0 | ||
| */ | ||
| @Unstable | ||
| case object YearMonthIntervalType extends YearMonthIntervalType | ||
| object YearMonthIntervalType { |
There was a problem hiding this comment.
Shall we follow DecinalType and extends AbstractDataType? This is useful when a function accepts all kinds of year month intervals.
There was a problem hiding this comment.
I didn't understand this. Could you explain me, please. For now, the code works:
test("aaa") {
def f(y: YearMonthIntervalType): Int = {
y.defaultSize
}
f(YearMonthIntervalType(0, 1))
}@cloud-fan or mean a different use case?
There was a problem hiding this comment.
I mean ExpectsInputTypes.inputTypes. The "function" I mentioned is SQL function (expression).
|
where is the SQL parser change to fill the units? |
This is WIP PR. Changes are coming. |
What changes were proposed in this pull request?
Extend YearMonthIntervalType to support:
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?