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.

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-8192] [SPARK-8193] [SQL] udf current_date, current_timestamp #6985

Closed
wants to merge 7 commits into from

Conversation

adrian-wang
Copy link
Contributor

No description provided.

@SparkQA
Copy link

SparkQA commented Jun 24, 2015

Test build #35674 has finished for PR 6985 at commit 9e4bca5.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class CurrentTimestamp() extends Expression
    • case class CurrentDate() extends Expression

@adrian-wang
Copy link
Contributor Author

Close it because #6782 (comment) for now

/**
* Adds a number of days to startdate: date_add('2008-12-31', 1) = '2009-01-01'.
*/
case class CurrentTimestamp() extends Expression {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this one extend LeafExpression?

@rxin
Copy link
Contributor

rxin commented Jul 2, 2015

@adrian-wang can you do this one first? This one is simple enough that there is no debate. Update it to address my comment with test cases, and then we can merge it.

@adrian-wang adrian-wang reopened this Jul 2, 2015
@SparkQA
Copy link

SparkQA commented Jul 2, 2015

Test build #36384 has finished for PR 6985 at commit 427d9dc.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class CurrentDate() extends LeafExpression
    • case class CurrentTimestamp() extends LeafExpression

@SparkQA
Copy link

SparkQA commented Jul 2, 2015

Test build #36385 has finished for PR 6985 at commit 98e8550.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class CurrentDate() extends LeafExpression
    • case class CurrentTimestamp() extends LeafExpression

@adrian-wang adrian-wang changed the title [SPARK-8192] [SPARK-8193] [SQL] [WIP] udf current_date, current_timestamp [SPARK-8192] [SPARK-8193] [SQL] udf current_date, current_timestamp Jul 2, 2015
@adrian-wang
Copy link
Contributor Author

I think it again, and since System.currentTimeMillis is not so close to what's inside, I designed to put it back.

test("datetime function current_date") {
checkEvaluation(
CurrentDate(),
new Date(System.currentTimeMillis), EmptyRow)
Copy link
Contributor

Choose a reason for hiding this comment

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

this test would be flaky -- I think you want to add some tolerance, e.g. checking System.currentTimeMillis +- 100ms

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I thought that was rare to have. I'll modify the code here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@adrian-wang just fyi this one is still flaky here ...

@SparkQA
Copy link

SparkQA commented Jul 3, 2015

Test build #36472 has finished for PR 6985 at commit e11ae75.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class CurrentDate() extends LeafExpression
    • case class CurrentTimestamp() extends LeafExpression


import ctx.implicits._

val df1 = Seq((1, 2), (3, 1)).toDF("a", "b")
Copy link
Contributor

Choose a reason for hiding this comment

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

don't do this here. if there is an error with the df creation, it will make all the test cases disappear since it wasn't able to construct the object.

you can add a lazy in front of val here.

@rxin
Copy link
Contributor

rxin commented Jul 3, 2015

lgtm

@SparkQA
Copy link

SparkQA commented Jul 3, 2015

Test build #36478 has finished for PR 6985 at commit 27c9f95.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class CurrentDate() extends LeafExpression
    • case class CurrentTimestamp() extends LeafExpression

@SparkQA
Copy link

SparkQA commented Jul 3, 2015

Test build #36483 has finished for PR 6985 at commit 6a20b64.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class CurrentDate() extends LeafExpression
    • case class CurrentTimestamp() extends LeafExpression

@rxin
Copy link
Contributor

rxin commented Jul 4, 2015

Thanks - I've merged this.

@asfgit asfgit closed this in 9fb6b83 Jul 4, 2015
tarekbecker added a commit to tarekbecker/spark that referenced this pull request Jul 4, 2015
asfgit pushed a commit that referenced this pull request Jul 19, 2015
…][SPARK-8179][SPARK-8177][SPARK-8178][SPARK-9115][SQL] date functions

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`and `dayofmonth` are both necessary?

~~I am going to add `Quarter` to this PR as well.~~ Done.

~~As soon as the Scala coding is reviewed and discussed, I'll add the python api.~~ Done

Author: Tarek Auel <tarek.auel@googlemail.com>
Author: Tarek Auel <tarek.auel@gmail.com>

Closes #6981 from tarekauel/SPARK-8199 and squashes the following commits:

f7b4c8c [Tarek Auel] [SPARK-8199] fixed bug in tests
bb567b6 [Tarek Auel] [SPARK-8199] fixed test
3e095ba [Tarek Auel] [SPARK-8199] style and timezone fix
256c357 [Tarek Auel] [SPARK-8199] code cleanup
5983dcc [Tarek Auel] [SPARK-8199] whitespace fix
6e0c78f [Tarek Auel] [SPARK-8199] removed setTimeZone in tests, according to cloud-fans comment in #7488
4afc09c [Tarek Auel] [SPARK-8199] concise leap year handling
ea6c110 [Tarek Auel] [SPARK-8199] fix after merging master
70238e0 [Tarek Auel] Merge branch 'master' into SPARK-8199
3c6ae2e [Tarek Auel] [SPARK-8199] removed binary search
fb98ba0 [Tarek Auel] [SPARK-8199] python docstring fix
cdfae27 [Tarek Auel] [SPARK-8199] cleanup & python docstring fix
746b80a [Tarek Auel] [SPARK-8199] build fix
0ad6db8 [Tarek Auel] [SPARK-8199] minor fix
523542d [Tarek Auel] [SPARK-8199] address comments
2259299 [Tarek Auel] [SPARK-8199] day_of_month alias
d01b977 [Tarek Auel] [SPARK-8199] python underscore
56c4a92 [Tarek Auel] [SPARK-8199] update python docu
e223bc0 [Tarek Auel] [SPARK-8199] refactoring
d6aa14e [Tarek Auel] [SPARK-8199] fixed Hive compatibility
b382267 [Tarek Auel] [SPARK-8199] fixed bug in day calculation; removed set TimeZone in HiveCompatibilitySuite for test purposes; removed Hive tests for second and minute, because we can cast '2015-03-18' to a timestamp and extract a minute/second from it
1b2e540 [Tarek Auel] [SPARK-8119] style fix
0852655 [Tarek Auel] [SPARK-8119] changed from ExpectsInputTypes to implicit casts
ec87c69 [Tarek Auel] [SPARK-8119] bug fixing and refactoring
1358cdc [Tarek Auel] Merge remote-tracking branch 'origin/master' into SPARK-8199
740af0e [Tarek Auel] implement date function using a calculation based on days
4fb66da [Tarek Auel] WIP: date functions on calculation only
1a436c9 [Tarek Auel] wip
f775f39 [Tarek Auel] fixed return type
ad17e96 [Tarek Auel] improved implementation
c42b444 [Tarek Auel] Removed merge conflict file
ccb723c [Tarek Auel] [SPARK-8199] style and fixed merge issues
10e4ad1 [Tarek Auel] Merge branch 'master' into date-functions-fast
7d9f0eb [Tarek Auel] [SPARK-8199] git renaming issue
f3e7a9f [Tarek Auel] [SPARK-8199] revert change in DataFrameFunctionsSuite
6f5d95c [Tarek Auel] [SPARK-8199] fixed year interval
d9f8ac3 [Tarek Auel] [SPARK-8199] implement fast track
7bc9d93 [Tarek Auel] Merge branch 'master' into SPARK-8199
5a105d9 [Tarek Auel] [SPARK-8199] rebase after #6985 got merged
eb6760d [Tarek Auel] Merge branch 'master' into SPARK-8199
f120415 [Tarek Auel] improved runtime
a8edebd [Tarek Auel] use Calendar instead of SimpleDateFormat
5fe74e1 [Tarek Auel] fixed python style
3bfac90 [Tarek Auel] fixed style
356df78 [Tarek Auel] rely on cast mechanism of Spark. Simplified implementation
02efc5d [Tarek Auel] removed doubled code
a5ea120 [Tarek Auel] added python api; changed test to be more meaningful
b680db6 [Tarek Auel] added codegeneration to all functions
c739788 [Tarek Auel] added support for quarter SPARK-8178
849fb41 [Tarek Auel] fixed stupid test
638596f [Tarek Auel] improved codegen
4d8049b [Tarek Auel] fixed tests and added type check
5ebb235 [Tarek Auel] resolved naming conflict
d0e2f99 [Tarek Auel] date functions
@adrian-wang adrian-wang deleted the udfcurrent branch July 21, 2015 00:37
test("function current_timestamp") {
checkAnswer(df1.select(countDistinct(current_timestamp())), Row(1))
// Execution in one query should return the same value
checkAnswer(ctx.sql("""SELECT CURRENT_TIMESTAMP() = CURRENT_TIMESTAMP()"""),
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is a known issue as SPARK-9196 . we have to use a different way to evaluate this function. I think we'd better disable this function test for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants