You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/05/09 18:16:20 UTC
[GitHub] [spark] MaxGekk opened a new pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
MaxGekk opened a new pull request #28486:
URL: https://github.com/apache/spark/pull/28486
### What changes were proposed in this pull request?
Shift non-existing dates in Proleptic Gregorian calendar by 1 day. The reason for that is `RowEncoderSuite` generates random dates/timestamps in the hybrid calendar, and some dates/timestamps don't exist in Proleptic Gregorian calendar like 1000-02-29 because 1000 is not leap year in Proleptic Gregorian calendar.
### Why are the changes needed?
This makes RowEncoderSuite much stable.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
By running RowEncoderSuite and set non-existing date manually:
```scala
val date = new java.sql.Date(1000 - 1900, 1, 29)
Try { date.toLocalDate; date }.getOrElse(new Date(date.getTime + MILLIS_PER_DAY))
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626216233
**[Test build #122467 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/122467/testReport)** for PR 28486 at commit [`448699f`](https://github.com/apache/spark/commit/448699f2ceb4cfaf32c3bb4ee0588b5991704434).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626464599
Can we generate `LocalDate`/`Instant`? Spark supports these 2 types when creating DataFrame/Dataset.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626290450
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon edited a comment on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626552528
That would batter happen when we enable `spark.sql.datetime.java8API.enabled` by default. We could let `RandomDataGenerator` take the configuration into account alternatively. Catalyst converter doesn't look supporting `LocalDate` and `Instant` by default.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626216233
**[Test build #122467 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/122467/testReport)** for PR 28486 at commit [`448699f`](https://github.com/apache/spark/commit/448699f2ceb4cfaf32c3bb4ee0588b5991704434).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626593741
Yeah, seems it does. At least I see one test: https://github.com/apache/spark/blob/ffef3d40741b0be321421aa52a6e17a26d89f541/sql/catalyst/src/test/scala/org/apache/spark/sql/RandomDataGeneratorSuite.scala#L39-L50
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626321312
**[Test build #122473 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/122473/testReport)** for PR 28486 at commit [`11218da`](https://github.com/apache/spark/commit/11218da6e3ef323732f5009541b9cc0bb9706eac).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626245241
**[Test build #122467 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/122467/testReport)** for PR 28486 at commit [`448699f`](https://github.com/apache/spark/commit/448699f2ceb4cfaf32c3bb4ee0588b5991704434).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626290315
**[Test build #122473 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/122473/testReport)** for PR 28486 at commit [`11218da`](https://github.com/apache/spark/commit/11218da6e3ef323732f5009541b9cc0bb9706eac).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626245384
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626290450
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626245384
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626216398
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626573034
We always support `LocalDate`/`Instant` as input, the config `spark.sql.datetime.java8API.enabled` means if we should output `LocalDate`/`Instant`.
If the generated data is not only used as input, but also as output the check the result, then this patch LGTM.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626552528
That would batter happen when we enable `spark.sql.datetime.java8API.enabled` by default. We could let `RandomDataGenerator` take the configuration into account as well. Catalyst converter doesn't look supporting `LocalDate` and `Instant` by default.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626321552
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #28486:
URL: https://github.com/apache/spark/pull/28486#discussion_r422580243
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/RandomDataGenerator.scala
##########
@@ -172,7 +173,8 @@ object RandomDataGenerator {
// January 1, 1970, 00:00:00 GMT for "9999-12-31 23:59:59.999999".
milliseconds = rand.nextLong() % 253402329599999L
}
- DateTimeUtils.toJavaDate((milliseconds / MILLIS_PER_DAY).toInt)
+ val date = DateTimeUtils.toJavaDate((milliseconds / MILLIS_PER_DAY).toInt)
+ Try { date.toLocalDate; date }.getOrElse(new Date(date.getTime + MILLIS_PER_DAY))
Review comment:
Shall we add a short comment that it adds one day in case the leap year is not matched? Also, let's add some words that the dates should be both valid in hybrid Gregorian/Julian calendar and Proleptic Gregorian calendar.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on a change in pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #28486:
URL: https://github.com/apache/spark/pull/28486#discussion_r422606120
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/RandomDataGenerator.scala
##########
@@ -172,7 +173,8 @@ object RandomDataGenerator {
// January 1, 1970, 00:00:00 GMT for "9999-12-31 23:59:59.999999".
milliseconds = rand.nextLong() % 253402329599999L
}
- DateTimeUtils.toJavaDate((milliseconds / MILLIS_PER_DAY).toInt)
+ val date = DateTimeUtils.toJavaDate((milliseconds / MILLIS_PER_DAY).toInt)
+ Try { date.toLocalDate; date }.getOrElse(new Date(date.getTime + MILLIS_PER_DAY))
Review comment:
I added comments
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626321552
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626375231
> this should go into 3.0 but not 2.4 right?
@srowen Yes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626216398
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28486: [SPARK-31669][SQL][TESTS] Fix RowEncoderSuite failures on non-existing dates/timestamps
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28486:
URL: https://github.com/apache/spark/pull/28486#issuecomment-626290315
**[Test build #122473 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/122473/testReport)** for PR 28486 at commit [`11218da`](https://github.com/apache/spark/commit/11218da6e3ef323732f5009541b9cc0bb9706eac).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org