You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by 10110346 <gi...@git.apache.org> on 2017/05/16 09:20:23 UTC
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
GitHub user 10110346 opened a pull request:
https://github.com/apache/spark/pull/17997
[SPARK-20763][SQL]The function of `month` and `day` return an error value
## What changes were proposed in this pull request?
spark-sql>select month("1582-09-28");
spark-sql>10
spark-sql>select day("1582-04-18");
spark-sql>28
when the date before "1582-10-04", the function of `month` and `day` return an error value.
## How was this patch tested?
unit tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/10110346/spark wip_lx_0516
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/17997.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 #17997
----
commit 7d6385fdcb83bca1de05ee9bb7fd751082f3fa3c
Author: liuxian <li...@zte.com.cn>
Date: 2017-05-16T09:13:59Z
The function of `month` and `day` return an error value
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/17997
@gatorsmile I have tested like this:
`val dav = Date.valueOf("1582-10-04");
val date = new Date(dav.getTime);
println(date.toString)`
the output is :1582-10-04
`val dav = Date.valueOf("1582-10-05");
val date = new Date(dav.getTime);
println(date.toString)`
the output is :1582-10-15
`val dav = Date.valueOf("1582-10-14");
val date = new Date(dav.getTime);
println(date.toString)`
the output is :1582-10-24
`val dav = Date.valueOf("1582-10-15");
val date = new Date(dav.getTime);
println(date.toString)`
the output is :1582-10-15
Also,i tested in mysql:
`mysql> select dayofyear('1582-10-4');
+------------------------+
| dayofyear('1582-10-4') |
+------------------------+
| 277 |
+------------------------+
1 row in set (0.00 sec)
mysql> select dayofyear('1582-10-5');
+------------------------+
| dayofyear('1582-10-5') |
+------------------------+
| 278 |
+------------------------+
1 row in set (0.00 sec)
mysql> select dayofyear('1582-10-6');
+------------------------+
| dayofyear('1582-10-6') |
+------------------------+
| 279 |
+------------------------+
1 row in set (0.00 sec)
mysql> select dayofyear('1582-10-14');
+-------------------------+
| dayofyear('1582-10-14') |
+-------------------------+
| 287 |
+-------------------------+
1 row in set (0.00 sec)
mysql> select dayofyear('1582-10-15');
+-------------------------+
| dayofyear('1582-10-15') |
+-------------------------+`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117153595
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
In mysql ,the rusult is :
mysql> select dayofyear("1982-10-04");
+-------------------------+
| dayofyear("1982-10-04") |
+-------------------------+
| 277 |
+-------------------------+
1 row in set (0.00 sec)
mysql> select dayofyear("1982-10-015");
+--------------------------+
| dayofyear("1982-10-015") |
+--------------------------+
| 288 |
+--------------------------+
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116888479
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -601,22 +601,32 @@ object DateTimeUtils {
* The calculation uses the fact that the period 1.1.2001 until 31.12.2400 is
* equals to the period 1.1.1601 until 31.12.2000.
*/
- private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
- // add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
- val numOfQuarterCenturies = daysNormalized / daysIn400Years
- val daysInThis400 = daysNormalized % daysIn400Years + 1
- val (years, dayInYear) = numYears(daysInThis400)
- val year: Int = (2001 - 20000) + 400 * numOfQuarterCenturies + years
- (year, dayInYear)
+ private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int, Int) = {
+ val date = new Date(daysToMillis(daysSince1970))
+ val YMD = date.toString.trim.split("-")
--- End diff --
yes,this cause performance worse than before
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116878495
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -601,22 +601,32 @@ object DateTimeUtils {
* The calculation uses the fact that the period 1.1.2001 until 31.12.2400 is
* equals to the period 1.1.1601 until 31.12.2000.
*/
- private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
- // add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
- val numOfQuarterCenturies = daysNormalized / daysIn400Years
- val daysInThis400 = daysNormalized % daysIn400Years + 1
- val (years, dayInYear) = numYears(daysInThis400)
- val year: Int = (2001 - 20000) + 400 * numOfQuarterCenturies + years
- (year, dayInYear)
+ private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int, Int) = {
+ val date = new Date(daysToMillis(daysSince1970))
+ val YMD = date.toString.trim.split("-")
--- End diff --
this would cause massive performance regression.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/17997
I guess this would also affect date-related functions such as `dayofyear`, `year`, `quater` and `weekofyear`, etc.
Could you add tests for them?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/17997
I have tried to changed `getYearAndDayInYear` like this:
`private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int, Int) = {
val date = new Date(daysToMillis(daysSince1970))
val YMD = date.toString.split("-")
(YMD(0).toInt, YMD(1).toInt, YMD(2).toInt)
}`
The result is right, but 'checkConsistencyBetweenInterpretedAndCodegen' executed failed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/17997
@ueshin Yes, I will do it ,thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117137648
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
cc @cloud-fan @gatorsmile
Do you have any ideas?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117178061
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -603,7 +603,13 @@ object DateTimeUtils {
*/
private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
// add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
+ var daysSince1970Tmp = daysSince1970
+ // In history,the period(5.10.1582 ~ 14.10.1582) is not exist
--- End diff --
`In history,the period(5.10.1582 ~ 14.10.1582) is not exist`
->
`Since Julian calendar was replaced with the Gregorian calendar, the 10 days after Oct. 4 were skipped.`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117157765
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
let's follow mysql
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/17997
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116954928
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
I'm not sure too, maybe `278` is better
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116947161
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -116,6 +121,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
}
+
+ checkEvaluation(Quarter(Literal(new Date(sdf.parse("1582-03-28 13:10:15").getTime))), 1)
+ checkEvaluation(Quarter(Literal(new Date(sdf.parse("1582-09-30 13:10:15").getTime))), 3)
--- End diff --
How about using `1582-09-30` and `1582-10-01` for this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/17997
OK，l Will do it, thanks. @gatorsmile
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116926505
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-04-28 13:10:15").getTime))), 118)
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-4 13:10:15").getTime))), 277)
--- End diff --
I believe the dates we need to test here should be the boundaries like `1582-10-04` and `1582-10-15`.
Btw, let's add `0` for 1-digit date, e.g. `1582-10-04` instead of `1582-10-4`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117180644
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -603,7 +603,13 @@ object DateTimeUtils {
*/
private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
// add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
+ var daysSince1970Tmp = daysSince1970
+ // In history,the period(5.10.1582 ~ 14.10.1582) is not exist
--- End diff --
@gatorsmile ok,thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117153106
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
can we check with other databases?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117158817
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -603,7 +603,13 @@ object DateTimeUtils {
*/
private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
// add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
+ var daysSince1970Tmp = daysSince1970
+ // In history,the period(5.10.1582 ~ 14.10.1582) is not exist
--- End diff --
OK, I will do ,thanks @kiszk @cloud-fan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116930879
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -96,6 +99,8 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
}
+ checkEvaluation(Year(Literal(new Date(sdf.parse("1582-01-01 13:10:15").getTime))), 1582)
+ checkEvaluation(Year(Literal(new Date(sdf.parse("1582-12-31 13:10:15").getTime))), 1582)
--- End diff --
Here should be `1581-12-31` and `1582-01-01`, for example, and the rest should be in a similar way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/17997
The current impl is still missing the special handling of that miracle period. See the outputs of Oracle:
```
Result Set 34
TO_CHAR(TO_DATE('1582-10-04','YYYY-MM-DD'),'DDD')
277
Result Set 35
TO_CHAR(TO_DATE('1582-10-05','YYYY-MM-DD'),'DDD')
288
Result Set 36
TO_CHAR(TO_DATE('1582-10-11','YYYY-MM-DD'),'DDD')
288
Result Set 37
TO_CHAR(TO_DATE('1582-10-12','YYYY-MM-DD'),'DDD')
288
Result Set 38
TO_CHAR(TO_DATE('1582-10-13','YYYY-MM-DD'),'DDD')
288
Result Set 39
TO_CHAR(TO_DATE('1582-10-14','YYYY-MM-DD'),'DDD')
288
Result Set 40
TO_CHAR(TO_DATE('1582-10-15','YYYY-MM-DD'),'DDD')
288
Result Set 41
TO_CHAR(TO_DATE('1582-10-16','YYYY-MM-DD'),'DDD')
289
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/17997
After checking DB2, it behaves the same as MySQL.
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17997
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117178267
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -603,7 +603,13 @@ object DateTimeUtils {
*/
private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
// add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
+ var daysSince1970Tmp = daysSince1970
+ // In history,the period(1582-10-05 ~ 1582-10-14) is not exist
--- End diff --
`In history,the period(5.10.1582 ~ 14.10.1582) is not exist`
->
`Since Julian calendar was replaced with the Gregorian calendar, the 10 days after Oct. 4 were skipped.`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/17997
@srowen @rxin So, by contrast, mybe make a hack in one place is a better solution
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117158477
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
OK, thanks @cloud-fan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/17997
Could you submit a backport PR to 2.1? Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116952974
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -603,7 +603,13 @@ object DateTimeUtils {
*/
private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
// add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
+ var daysSince1970Tmp = daysSince1970
+ // In history,the period(5.10.1582 ~ 14.10.1582) is not exist
--- End diff --
I have tested like this:
`val dav = Date.valueOf("1582-10-14")
val date = new Date(dav.getTime);
println(date.toString)`
the output is:`1582-10-24`
`val dav = Date.valueOf("1582-10-05")
val date = new Date(dav.getTime);
println(date.toString)`
the output is:`1582-10-15`
so, i think we can not use `1582-10-5` ~ `1582-10-14`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117181219
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -603,7 +603,13 @@ object DateTimeUtils {
*/
private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
// add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
+ var daysSince1970Tmp = daysSince1970
+ // In history,the period(1582-10-05 ~ 1582-10-14) is not exist
--- End diff --
@gatorsmile ok,thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116950321
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -603,7 +603,13 @@ object DateTimeUtils {
*/
private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
// add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
+ var daysSince1970Tmp = daysSince1970
+ // In history,the period(5.10.1582 ~ 14.10.1582) is not exist
--- End diff --
nit: this representation may be confusing for all of the people in the world. Can we use 1582-10-5 or like Oct. 5, 1582?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r116948312
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
I'm not sure whether this value is correct or not. should be `278`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/17997
Point taken, but do other DBs support dates before 1970? it does highly depend on historical calendars. The right solution isn't a hack in one place here but using Calendar properly, if anything.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117155497
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
@cloud-fan Because in history,the period(5.10.1582 ~ 14.10.1582) is not exist
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/17997
Thanks! Merging to master/2.2.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117153080
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala ---
@@ -603,7 +603,13 @@ object DateTimeUtils {
*/
private[this] def getYearAndDayInYear(daysSince1970: SQLDate): (Int, Int) = {
// add the difference (in days) between 1.1.1970 and the artificial year 0 (-17999)
- val daysNormalized = daysSince1970 + toYearZero
+ var daysSince1970Tmp = daysSince1970
+ // In history,the period(5.10.1582 ~ 14.10.1582) is not exist
--- End diff --
It's only about comment, and I think 1582-10-5 or Oct. 5, 1582 is more human readable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17997: [SPARK-20763][SQL]The function of `month` and `day` retu...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/17997
@srowen I have tested in mysql, it can support dates before 1970.
mysql> select month("1582-09-28");
+---------------------+
| month("1582-09-28") |
+---------------------+
| 9 |
+---------------------+
mysql> select day("1582-04-18");
+-------------------+
| day("1582-04-18") |
+-------------------+
| 18 |
+-------------------+
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/17997
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17997: [SPARK-20763][SQL]The function of `month` and `da...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/17997#discussion_r117155315
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala ---
@@ -76,6 +76,9 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
}
}
checkEvaluation(DayOfYear(Literal.create(null, DateType)), null)
+
+ checkEvaluation(DayOfYear(Literal(new Date(sdf.parse("1582-10-15 13:10:15").getTime))), 288)
--- End diff --
why `278` is better?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org