You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mgaido91 <gi...@git.apache.org> on 2017/09/17 12:15:49 UTC
[GitHub] spark pull request #19261: [SPARK-22040] Add current_date function with time...
GitHub user mgaido91 opened a pull request:
https://github.com/apache/spark/pull/19261
[SPARK-22040] Add current_date function with timezone id
## What changes were proposed in this pull request?
Add current_date function with timezone id.
## How was this patch tested?
Manual test on spark-shell and pyspark.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mgaido91/spark SPARK-22040
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19261.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #19261
----
commit 09a3ed93edf6aa44fb71b14c1194edac76f5b098
Author: Marco Gaido <ma...@gmail.com>
Date: 2017-09-17T12:11:20Z
[SPARK-22040] Add current_date(timezone) function
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19261: [SPARK-22040] Add current_date function with timezone id
Posted by jaceklaskowski <gi...@git.apache.org>.
Github user jaceklaskowski commented on the issue:
https://github.com/apache/spark/pull/19261
@rxin @gatorsmile Let me ask you a very similar question then, why does `CurrentDate` operator has the optional timezone parameter? What's the purpose? Wouldn't that answer your questions?
I don't mind not having the change, but am curious what is the reason for the "mismatch"?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19261: [SPARK-22040] Add current_date function with time...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 closed the pull request at:
https://github.com/apache/spark/pull/19261
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19261: [SPARK-22040] Add current_date function with time...
Posted by jaceklaskowski <gi...@git.apache.org>.
Github user jaceklaskowski commented on a diff in the pull request:
https://github.com/apache/spark/pull/19261#discussion_r139309261
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -2508,6 +2508,14 @@ object functions {
def current_date(): Column = withExpr { CurrentDate() }
/**
+ * Returns the current date in the given timezone as a date column.
--- End diff --
`@return`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19261: [SPARK-22040] Add current_date function with timezone id
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19261
I think we should not do it, because no DB vendor does it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19261: [SPARK-22040] Add current_date function with time...
Posted by jaceklaskowski <gi...@git.apache.org>.
Github user jaceklaskowski commented on a diff in the pull request:
https://github.com/apache/spark/pull/19261#discussion_r139309246
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -2508,6 +2508,14 @@ object functions {
def current_date(): Column = withExpr { CurrentDate() }
/**
+ * Returns the current date in the given timezone as a date column.
+ *
+ * @group datetime_funcs
+ * @since 2.3.0
+ */
+ def current_date(timeZone: String): Column = withExpr { CurrentDate(Option(timeZone)) }
--- End diff --
s/`Option`/`Some`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19261: [SPARK-22040] Add current_date function with time...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/19261#discussion_r139309394
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -2508,6 +2508,14 @@ object functions {
def current_date(): Column = withExpr { CurrentDate() }
/**
+ * Returns the current date in the given timezone as a date column.
+ *
+ * @group datetime_funcs
+ * @since 2.3.0
+ */
+ def current_date(timeZone: String): Column = withExpr { CurrentDate(Option(timeZone)) }
--- End diff --
What if someone pass a `null` as value? `Option` is safer in this case...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19261: [SPARK-22040] Add current_date function with time...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/19261#discussion_r139309376
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -2508,6 +2508,14 @@ object functions {
def current_date(): Column = withExpr { CurrentDate() }
/**
+ * Returns the current date in the given timezone as a date column.
--- End diff --
this is consistent with all the other functions in this file.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19261: [SPARK-22040] Add current_date function with timezone id
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/19261
What does this even mean?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19261: [SPARK-22040] Add current_date function with timezone id
Posted by jaceklaskowski <gi...@git.apache.org>.
Github user jaceklaskowski commented on the issue:
https://github.com/apache/spark/pull/19261
OK I feel convinced that you feel convinced Spark SQL should not offer this as part of the public API. Thanks for being with me for so long and patient to explain the things. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19261: [SPARK-22040] Add current_date function with timezone id
Posted by jaceklaskowski <gi...@git.apache.org>.
Github user jaceklaskowski commented on the issue:
https://github.com/apache/spark/pull/19261
@gatorsmile Dunno, but the logical operator does.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19261: [SPARK-22040] Add current_date function with time...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/19261#discussion_r139309404
--- Diff: python/pyspark/sql/functions.py ---
@@ -793,12 +793,12 @@ def ntile(n):
# ---------------------- Date/Timestamp functions ------------------------------
@since(1.5)
-def current_date():
+def current_date(timeZone=None):
--- End diff --
Honestly I have no idea too.. Do you know who can we ask to? Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19261: [SPARK-22040] Add current_date function with timezone id
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19261
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19261: [SPARK-22040] Add current_date function with time...
Posted by jaceklaskowski <gi...@git.apache.org>.
Github user jaceklaskowski commented on a diff in the pull request:
https://github.com/apache/spark/pull/19261#discussion_r139309272
--- Diff: python/pyspark/sql/functions.py ---
@@ -793,12 +793,12 @@ def ntile(n):
# ---------------------- Date/Timestamp functions ------------------------------
@since(1.5)
-def current_date():
+def current_date(timeZone=None):
--- End diff --
Would the change beg a different `@since`? It's no longer true that it existed `since(1.5)`, is it? Just asking...no idea how it really should be.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19261: [SPARK-22040] Add current_date function with timezone id
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19261
Any other database has such an interface?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org