Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-22894][SQL] DateTimeOperations should accept SQL like string type #20067

Closed
wants to merge 1 commit into from
Closed

[SPARK-22894][SQL] DateTimeOperations should accept SQL like string type #20067

wants to merge 1 commit into from

Conversation

wangyum
Copy link
Member

@wangyum wangyum commented Dec 23, 2017 •
edited
Loading

What changes were proposed in this pull request?

DateTimeOperations accept StringType, but:

spark-sql> SELECT '2017-12-24' + interval 2 months 2 seconds;
Error in query: cannot resolve '(CAST('2017-12-24' AS DOUBLE) + interval 2 months 2 seconds)' due to data type mismatch: differing types in '(CAST('2017-12-24' AS DOUBLE) + interval 2 months 2 seconds)' (double and calendarinterval).; line 1 pos 7;
'Project [unresolvedalias((cast(2017-12-24 as double) + interval 2 months 2 seconds), None)]
+- OneRowRelation
spark-sql>

After this PR:

spark-sql> SELECT '2017-12-24' + interval 2 months 2 seconds;
2018-02-24 00:00:02
Time taken: 0.2 seconds, Fetched 1 row(s)

How was this patch tested?

unit tests

@SparkQA
Copy link

SparkQA commented Dec 23, 2017

Test build #85351 has finished for PR 20067 at commit ae998ec.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gcz2022
Copy link

gcz2022 commented Dec 25, 2017

LGTM

gatorsmile
gatorsmile reviewed Dec 25, 2017
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
@@ -2760,6 +2760,17 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
}
}

test("SPARK-22894: DateTimeOperations should accept SQL like string type") {
Copy link
Member

@gatorsmile gatorsmile Dec 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add it to #20061?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll and it.

gatorsmile
gatorsmile reviewed Dec 25, 2017
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
@@ -2760,6 +2760,17 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
}
}

test("SPARK-22894: DateTimeOperations should accept SQL like string type") {
val date = "2017-12-24"
val str = sql(s"SELECT CAST('$date' as STRING) + interval 2 months 2 seconds")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far, we issue an exception. What is the behavior of Hive?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hive doesn't accept string type:

hive> SELECT cast('2017-12-24' as date) + interval 2 day;
2017-12-26 00:00:00
hive> SELECT cast('2017-12-24' as timestamp) + interval 2 day;
2017-12-26 00:00:00
hive> SELECT cast('2017-12-24' as string) + interval 2 day;
FAILED: SemanticException Line 0:-1 Wrong arguments '2': No matching method for class org.apache.hadoop.hive.ql.udf.generic.GenericUDFOPDTIPlus with (string, interval_day_time)
hive>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But Spark was originally supported:

spark/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionSuite.scala

Line 1083 in bc0848b

ruleTest(dateTimeOperations, Add(str, interval), Cast(TimeAdd(str, interval), StringType))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the original PR https://github.com/apache/spark/pull/7754/files#r35821191

Maybe the SQL API should support it since we do support it in DataFrame APIs.

@gatorsmile
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Dec 26, 2017

Test build #85387 has finished for PR 20067 at commit ae998ec.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

gatorsmile
gatorsmile reviewed Dec 26, 2017
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
checkAnswer(str, Row("2018-02-24 00:00:02") :: Nil)
checkAnswer(dt, Row(Date.valueOf("2018-02-24")) :: Nil)
checkAnswer(ts, Row(Timestamp.valueOf("2018-02-24 00:00:02")) :: Nil)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please get rid of this test case and merge them to #20061

@gatorsmile
Copy link
Member

LGTM

Merged to master.

@asfgit asfgit closed this in 91d1b30 Dec 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gatorsmile gatorsmile gatorsmile left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wangyum @SparkQA @gcz2022 @gatorsmile