-
Notifications
You must be signed in to change notification settings - Fork 28.4k
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.
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
[SPARK-8199][SPARK-8184][SPARK-8183][SPARK-8182][SPARK-8181][SPARK-8180][SPARK-8179][SPARK-8177][SPARK-8178][SPARK-9115][SQL] date functions #6981
Conversation
override def nullable: Boolean = true | ||
|
||
override def eval(input: InternalRow): Any = { | ||
DateFormat(child, Literal("y")).eval(input) match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it okay to call DateFormat(child, Literal("y")).eval(input)
or is there a more elegant way.
Is there a way to call genCode
of DataFormat
?
I think this should wait #6782 , since we are creating a file with same usage. |
fixed test cases dataframe test issues: - dateformat with date string is null - hours, minutes, seconds is zero for timestamp
import org.apache.spark.sql.types._ | ||
import org.apache.spark.unsafe.types.UTF8String | ||
|
||
case class DateFormatClass(left: Expression, right: Expression) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If name is DateFormat
build failes in line
spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
Line 72 in 6b7f2ce
def toString(days: Int): String = Cast.threadLocalDateFormat.get.format(toJavaDate(days)) |
@adrian-wang I guess we can implement the stuff already. A rebase or merge should be trivial. |
Code generation is adopted from spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala Line 191 in bc76a0f
|
UTF8String.fromString(sdf.format(DateTimeUtils.toJavaDate(valueLeft.asInstanceOf[Int]))) | ||
case StringType => | ||
UTF8String.fromString( | ||
sdf.format(DateTimeUtils.stringToTime(valueLeft.asInstanceOf[UTF8String].toString))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just valueLeft.toString
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NAVER - http://www.naver.com/
sujkh@naver.com 님께 보내신 메일 <Re: [spark] [SPARK-8199][SPARK-8184][SPARK-8183][SPARK-8182][SPARK-8181][SPARK-8180][SPARK-8179][SPARK-8177][SPARK-8178][SQL] date functions (#6981)> 이 다음과 같은 이유로 전송 실패했습니다.
받는 사람이 회원님의 메일을 수신차단 하였습니다.
@YijieSHEN Thanks for all your feedback. I changed the things you mentioned. Have you an idea to improve |
ok to test |
Test build #35952 has finished for PR 6981 at commit
|
Test build #37680 has finished for PR 6981 at commit
|
Test build #37717 has finished for PR 6981 at commit
|
Test build #37718 has finished for PR 6981 at commit
|
@rxin / @davies First of all thanks for all your feedback so far. I removed the manual binary search. I guess we all agree that the if/else structure is much more readable. @davies I removed @cloud-fan Could you have a look on this PR, if you have time? Would be great. |
Test build #37725 has finished for PR 6981 at commit
|
specialized implementation. | ||
|
||
>>> df0 = sqlContext.createDataFrame([('2015-04-08',)], ['a']) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: Why do we have this blank line?
@tarekauel This is pretty close now, one thing left is that the |
Test build #37729 has finished for PR 6981 at commit
|
Test build #37742 has finished for PR 6981 at commit
|
Test build #37746 has finished for PR 6981 at commit
|
(0 to 5).foreach { i => | ||
val c = Calendar.getInstance() | ||
c.set(y, m, 28, 0, 0, 0) | ||
c.add(Calendar.DATE, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 -> i
Thanks @tarekauel. I'm going to merge this and push a commit to fix that two problems @davies pointed out. |
(0 to 5).foreach { i => | ||
val c = Calendar.getInstance() | ||
c.set(y, m, 28, 0, 0, 0) | ||
c.add(Calendar.DATE, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 -> i
@tarekauel congratulations. this is a big pr and a major step for date functions. |
(0 to 5).foreach { i => | ||
val c = Calendar.getInstance() | ||
c.set(y, m, 28, 0, 0, 0) | ||
c.add(Calendar.DATE, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 -> i
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this when I looked for the last bug. I'm going to create a follow PR
…stems. This pull request fixes some of the problems in #6981. - Added date functions to `__all__` so they get exposed - Rename day_of_month -> dayofmonth - Rename day_in_year -> dayofyear - Rename week_of_year -> weekofyear - Removed "day" from Scala/Python API since it is ambiguous. Only leaving the alias in SQL. Author: Reynold Xin <rxin@databricks.com> This patch had conflicts when merged, resolved by Committer: Reynold Xin <rxin@databricks.com> Closes #7506 from rxin/datetime and squashes the following commits: 0cb24d9 [Reynold Xin] Export all functions in Python. e44a4a0 [Reynold Xin] Removed day function from Scala and Python. 9c08fdc [Reynold Xin] [SQL] Make date/time functions more consistent with other database systems.
Jira:
https://issues.apache.org/jira/browse/SPARK-8199
https://issues.apache.org/jira/browse/SPARK-8184
https://issues.apache.org/jira/browse/SPARK-8183
https://issues.apache.org/jira/browse/SPARK-8182
https://issues.apache.org/jira/browse/SPARK-8181
https://issues.apache.org/jira/browse/SPARK-8180
https://issues.apache.org/jira/browse/SPARK-8179
https://issues.apache.org/jira/browse/SPARK-8177
https://issues.apache.org/jira/browse/SPARK-8179
https://issues.apache.org/jira/browse/SPARK-9115
Regarding
day
anddayofmonth
are both necessary?I am going to addDone.Quarter
to this PR as well.As soon as the Scala coding is reviewed and discussed, I'll add the python api.Done