-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-8176] [SPARK-8197] [SQL] function to_date/ trunc #6988
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
Conversation
|
Test build #35682 has finished for PR 6988 at commit
|
|
Close it because #6782 (comment) for now |
450159c to
0239f33
Compare
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.
Probably only accept the DateType, TimestampType and the StringType, should be Seq(TypeCollection(DateType, TimestampType, StringType)).
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.
TimestampType, StringType should all be convert into DateType.
Actually, I can add a rule in optimizer to delete this node after we have done implicit cast.
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 mean we don't accept the IntegerType, LongType, do we? Hence we use the TypeCollection, and we'd better inherits from the ExpectInputTypes, instead of the ImplicitCastInputTypes.
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.
int and long could not be cast into datetype.
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.
Seems Hive only support date,string or timestamp, but not int, long..
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 the cast does not work, we will get an exception, that's expected.
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.
Yes, seems we can remove this expression in optimization.
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 we removed this expression during optimizing, then we can remove the code gen stuff, too.
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.
Could you add a comment says that this rely on implicit casting (from StringType and TimestampType)?
|
Test build #38034 has finished for PR 6988 at commit
|
|
Test build #38053 has finished for PR 6988 at commit
|
|
retest this please. |
|
Test build #52 has finished for PR 6988 at commit
|
|
Test build #38055 has finished for PR 6988 at commit
|
|
Test build #38057 has finished for PR 6988 at commit
|
|
retest this please. |
|
Test build #58 has finished for PR 6988 at commit
|
|
@rxin something is wrong with Jenkins. |
|
Test build #38069 has finished for PR 6988 at commit
|
|
retest this please. |
|
Test build #38093 has finished for PR 6988 at commit
|
|
Test build #63 has finished for PR 6988 at commit
|
|
Test build #38165 has finished for PR 6988 at commit
|
|
retest this please. |
|
Test build #78 has finished for PR 6988 at commit
|
|
Test build #38214 has finished for PR 6988 at commit
|
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.
Seems this will not trigger the codegen projections, you probably need to use the checkEvaluation instead.
d3faa53 to
ce15567
Compare
|
Test build #38525 has finished for PR 6988 at commit
|
ce15567 to
d51d5f1
Compare
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.
you could override gen to call child.gen()
d51d5f1 to
a476c5a
Compare
|
Test build #38847 has finished for PR 6988 at commit
|
|
Test build #38963 has finished for PR 6988 at commit
|
|
Test build #38987 has finished for PR 6988 at commit
|
|
retest this please. |
|
Test build #156 has finished for PR 6988 at commit
|
|
retest this please. |
|
Test build #38996 has finished for PR 6988 at commit
|
|
Test build #159 has finished for PR 6988 at commit
|
|
@adrian-wang Thanks for working on this, ti's pretty close, I will take over this one. |
No description provided.