[SPARK-29502][SQL] typed interval expression should fail for invalid format #26151
Conversation
| @@ -435,8 +435,7 @@ class ExpressionParserSuite extends AnalysisTest { | |||
| val intervalLiteral = Literal(CalendarInterval.fromString("interval 3 month 1 hour")) | |||
| assertEqual("InterVal 'interval 3 month 1 hour'", intervalLiteral) | |||
| assertEqual("INTERVAL '3 month 1 hour'", intervalLiteral) | |||
| assertEqual("Interval 'interval 3 monthsss 1 hoursss'", | |||
| Literal(null, CalendarIntervalType)) | |||
| intercept("Interval 'interval 3 monthsss 1 hoursss'") | |||
There was a problem hiding this comment.
We can refer to the timestamp/date tests just a few lines above.
|
Test build #112226 has finished for PR 26151 at commit
|
| val ex = new ParseException(e.getMessage, ctx) | ||
| ex.setStackTrace(e.getStackTrace) | ||
| throw ex | ||
| } |
There was a problem hiding this comment.
The error message is still inconsistent. Invalid interval vs Cannot parse the DATE value:
scala> spark.sql("select Interval '10000 secondss'").show(false)
org.apache.spark.sql.catalyst.parser.ParseException:
Invalid interval: 10000 secondss(line 1, pos 7)
scala> spark.sql("select date '10000 secondss'").show(false)
org.apache.spark.sql.catalyst.parser.ParseException:
Cannot parse the DATE value: 10000 secondss(line 1, pos 7)
== SQL ==
select date '10000 secondss'
-------^^^
|
Test build #112254 has finished for PR 26151 at commit
|
|
retest this please |
|
Test build #112258 has finished for PR 26151 at commit
|
| @@ -435,8 +435,7 @@ class ExpressionParserSuite extends AnalysisTest { | |||
| val intervalLiteral = Literal(CalendarInterval.fromString("interval 3 month 1 hour")) | |||
| assertEqual("InterVal 'interval 3 month 1 hour'", intervalLiteral) | |||
| assertEqual("INTERVAL '3 month 1 hour'", intervalLiteral) | |||
| assertEqual("Interval 'interval 3 monthsss 1 hoursss'", | |||
| Literal(null, CalendarIntervalType)) | |||
| intercept("Interval 'interval 3 monthsss 1 hoursss'") | |||
There was a problem hiding this comment.
Can you check the error message?
There was a problem hiding this comment.
This test suite defines the intercept method and uses it in many places. I don't want to break the rule here. It's definitely better to update the intercept method and take a error message parameter. This should be done in a followup.
There was a problem hiding this comment.
damn, the date and timestamp tests don't specify message, and I thought it doesn't have the message arg... Let me update it now.
|
Test build #112283 has finished for PR 26151 at commit
|
|
Merged to master. |
|
Late LGTM. Thank you all! |
What changes were proposed in this pull request?
This is a followup of #25241 .
The typed interval expression should fail for invalid format.
Why are the changes needed?
Te be consistent with the typed timestamp/date expression
Does this PR introduce any user-facing change?
Yes. But this feature is not released yet.
How was this patch tested?
updated test