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/04/06 14:21:56 UTC
[GitHub] [spark] cloud-fan opened a new pull request #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
cloud-fan opened a new pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This PR makes two changes:
1. always write the datetime values to parquet/avro without rebasing
2. Do not rebase datetime values when reading parquet/avro, if we know the file written version and it's `> "3.0"`.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
It's very easy to have mixed-calendar parquet/avro files: e.g. A user upgrades to Spark 3.0 and writes some parquet files to an existing directory. Then he realizes that the directory contains legacy datetime values before 1582. However, it's too late and he has to find out all the legacy files manually and read them separately.
To support mixed-calendar parquet/avro files, we need to decide to rebase or not based on the file written Spark version. After this, we can simplify the write side and never rebase.
### Does this PR introduce any user-facing change?
<!--
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Updated test
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk edited a comment on issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk edited a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609956319
I haven't looked at the implementation yet but the description looks dangerous for me:
> always write the datetime values to parquet/avro without rebasing
@cloud-fan I think there are at least 2 cases:
1. Spark 3.0 saves parquets that are supposed to be loaded by Spark 3.0 - no need to rebase
2. Spark 3.0 saves parquets that should be loaded by Spark 2.4, 2.3 and etc. - need to rebase.
Do you think the seconds case is not possible?
> Do not rebase datetime values when reading parquet/avro, if we know the file written version and it's > "3.0"
but you don't know the purpose of written files. Maybe an users wants to save parquet files by Spark 3.0 and read them back by Spark 2.4? For example, Spark 3.0 and Spark 2.4 can be used in different clusters that prepares data for each others. And later, if files are loaded by Spark 3.0, how users should read already saved parquet files. For example, when they migrated from 2.4 to 3.0 in a cluster but files were written for 2.4.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404578582
##########
File path: sql/core/benchmarks/DateTimeRebaseBenchmark-jdk11-results.txt
##########
@@ -26,17 +15,6 @@ before 1582, vec off, rebase on 13836 13872
before 1582, vec on, rebase off 3664 3672 7 27.3 36.6 3.5X
before 1582, vec on, rebase on 6049 6078 30 16.5 60.5 2.1X
-OpenJDK 64-Bit Server VM 11.0.6+10-post-Ubuntu-1ubuntu118.04.1 on Linux 4.15.0-1063-aws
-Intel(R) Xeon(R) CPU E5-2670 v2 @ 2.50GHz
-Save timestamps to parquet: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
Review comment:
Yea we can wait for that. IIUC the optimization can also benefit rebasing when writing ORC files, right?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609980240
**[Test build #120876 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120876/testReport)** for PR 28137 at commit [`8295549`](https://github.com/apache/spark/commit/829554925233803c17c72b08ec9096c3a1c41a15).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class AvroDeserializer(rootAvroType: Schema, rootCatalystType: DataType, rebaseDateTime: Boolean) `
* `class ParquetReadSupport(`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404303305
##########
File path: sql/core/benchmarks/DateTimeRebaseBenchmark-jdk11-results.txt
##########
@@ -26,17 +15,6 @@ before 1582, vec off, rebase on 13836 13872
before 1582, vec on, rebase off 3664 3672 7 27.3 36.6 3.5X
before 1582, vec on, rebase on 6049 6078 30 16.5 60.5 2.1X
-OpenJDK 64-Bit Server VM 11.0.6+10-post-Ubuntu-1ubuntu118.04.1 on Linux 4.15.0-1063-aws
-Intel(R) Xeon(R) CPU E5-2670 v2 @ 2.50GHz
-Save timestamps to parquet: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
Review comment:
If you decide to merge this PR, I would ask to do that after the optimizations https://github.com/apache/spark/pull/28119 otherwise I will not have good base line for end-to-end performance in write.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
URL: https://github.com/apache/spark/pull/28137#issuecomment-614197411
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121326/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-614048539
**[Test build #121326 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121326/testReport)** for PR 28137 at commit [`2e908c5`](https://github.com/apache/spark/commit/2e908c51be999f54d97a11ce3261518d1a2f75c3).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609828157
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404654496
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceUtils.scala
##########
@@ -64,4 +66,10 @@ object DataSourceUtils {
private[sql] def isDataFile(fileName: String) =
!(fileName.startsWith("_") || fileName.startsWith("."))
+
+ def needRebaseDateTime(lookupFileMeta: String => String): Option[Boolean] = {
+ // If there is no version, we return None and let the caller side to decide.
+ // Files written by Spark 3.0 and later follow the new calendar and don't need to rebase.
+ Option(lookupFileMeta(SPARK_VERSION_METADATA_KEY)).map(_ < "3.0")
Review comment:
I don't think we guarantee forward compatibility - things from Spark 2.4 should work in Spark 3.0 but I don't think the opposite case is guaranteed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404302533
##########
File path: sql/core/benchmarks/DateTimeRebaseBenchmark-jdk11-results.txt
##########
@@ -26,17 +15,6 @@ before 1582, vec off, rebase on 13836 13872
before 1582, vec on, rebase off 3664 3672 7 27.3 36.6 3.5X
before 1582, vec on, rebase on 6049 6078 30 16.5 60.5 2.1X
-OpenJDK 64-Bit Server VM 11.0.6+10-post-Ubuntu-1ubuntu118.04.1 on Linux 4.15.0-1063-aws
-Intel(R) Xeon(R) CPU E5-2670 v2 @ 2.50GHz
-Save timestamps to parquet: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
Review comment:
Why do you delete it? How should I know the performance in end-to-end case in write?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404299889
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceUtils.scala
##########
@@ -64,4 +66,10 @@ object DataSourceUtils {
private[sql] def isDataFile(fileName: String) =
!(fileName.startsWith("_") || fileName.startsWith("."))
+
+ def needRebaseDateTime(lookupFileMeta: String => String): Option[Boolean] = {
+ // If there is no version, we return None and let the caller side to decide.
+ // Files written by Spark 3.0 and later follow the new calendar and don't need to rebase.
+ Option(lookupFileMeta(SPARK_VERSION_METADATA_KEY)).map(_ < "3.0")
Review comment:
> Files written by Spark 3.0 and later follow the new calendar and don't need to rebase.
This is an unexpected solution for me which doesn't cover some use case from my point of view. I do believe versions in parquet/avro are not enough to make decision about rebasing.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609981150
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120876/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609827205
**[Test build #120876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120876/testReport)** for PR 28137 at commit [`8295549`](https://github.com/apache/spark/commit/829554925233803c17c72b08ec9096c3a1c41a15).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-610211543
@MaxGekk It's always an option to ask users to upgrade software if some bugs are not fixed in earlier versions. For example, if Spark 3.0 writes Parquet files and Spark 2.4 may read wrong data, we can ask them to upgrade to Spark 3.0. Forward compatibility is not officially supported.
On the other hand, if Spark 2.4 writes Parquet files and Spark 3.0 may read wrong data, asking users to downgrade is not a good option. This PR tries to improve backward compatibility by rebasing datetime more smartly: don't rely on the config if the file written Spark version is known. I think it's more important than forward compatibility.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-610225607
Partial upgrade is still possible, e.g. write files using 2.4 and read using 3.0. It's not possible to do the opposite though, as forward compatibility is not guaranteed. And that config doesn't really help too much with the forward compatibility: it's hard for users to know when to enable that config.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404582477
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceUtils.scala
##########
@@ -64,4 +66,10 @@ object DataSourceUtils {
private[sql] def isDataFile(fileName: String) =
!(fileName.startsWith("_") || fileName.startsWith("."))
+
+ def needRebaseDateTime(lookupFileMeta: String => String): Option[Boolean] = {
+ // If there is no version, we return None and let the caller side to decide.
+ // Files written by Spark 3.0 and later follow the new calendar and don't need to rebase.
+ Option(lookupFileMeta(SPARK_VERSION_METADATA_KEY)).map(_ < "3.0")
Review comment:
I think the use case you pointed out (write with 3.0 and read with 2.4) is a bit artificial. If someone develops a data pipeline, it's very tricky if some parts of the pipeline use 3.0 and some parts use 2.4. IMO the more common problem is to deal with legacy data files.
BTW we can support this use case by creating a new metadata entry to record the calendar, but I think it's too complicated and not worth it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
URL: https://github.com/apache/spark/pull/28137#issuecomment-614048539
**[Test build #121326 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121326/testReport)** for PR 28137 at commit [`2e908c5`](https://github.com/apache/spark/commit/2e908c51be999f54d97a11ce3261518d1a2f75c3).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609956319
I haven't looked at the implementation yet but the description looks dangerous for me:
> always write the datetime values to parquet/avro without rebasing
@cloud-fan I think there are at least 2 cases:
1. Spark 3.0 saves parquets that are supposed to be loaded by Spark 3.0 - no need to rebase
2. Spark 3.0 saves parquets that should be loaded by Spark 2.4, 2.3 and etc. - need to rebase.
Do you think the seconds case is not possible?
> Do not rebase datetime values when reading parquet/avro, if we know the file written version and it's > "3.0"
but you don't know the purpose of written files. Maybe an users wants to save parquet files by Spark 3.0 and read them back by Spark 2.4? For example, Spark 3.0 and Spark 2.4 can be used in different clusters that prepares data for each others.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-614049250
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609981144
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609827205
**[Test build #120876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/120876/testReport)** for PR 28137 at commit [`8295549`](https://github.com/apache/spark/commit/829554925233803c17c72b08ec9096c3a1c41a15).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609825789
cc @MaxGekk @dongjoon-hyun @HyukjinKwon
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609981144
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-614049257
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26009/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609828165
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25568/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404588184
##########
File path: sql/core/benchmarks/DateTimeRebaseBenchmark-jdk11-results.txt
##########
@@ -26,17 +15,6 @@ before 1582, vec off, rebase on 13836 13872
before 1582, vec on, rebase off 3664 3672 7 27.3 36.6 3.5X
before 1582, vec on, rebase on 6049 6078 30 16.5 60.5 2.1X
-OpenJDK 64-Bit Server VM 11.0.6+10-post-Ubuntu-1ubuntu118.04.1 on Linux 4.15.0-1063-aws
-Intel(R) Xeon(R) CPU E5-2670 v2 @ 2.50GHz
-Save timestamps to parquet: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
Review comment:
days rebasing in ORC will do after the optimization because it uses DaysWritable which is based on functions touched in this PR
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-614049250
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404597843
##########
File path: sql/core/benchmarks/DateTimeRebaseBenchmark-jdk11-results.txt
##########
@@ -26,17 +15,6 @@ before 1582, vec off, rebase on 13836 13872
before 1582, vec on, rebase off 3664 3672 7 27.3 36.6 3.5X
before 1582, vec on, rebase on 6049 6078 30 16.5 60.5 2.1X
-OpenJDK 64-Bit Server VM 11.0.6+10-post-Ubuntu-1ubuntu118.04.1 on Linux 4.15.0-1063-aws
-Intel(R) Xeon(R) CPU E5-2670 v2 @ 2.50GHz
-Save timestamps to parquet: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
Review comment:
No, it does not use rebaseGregorianToJulianMicros, see https://github.com/apache/spark/blob/820bb9985a76a567b79ffb01bbd2d32788e1dba0/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala#L149-L152 . We need to migrate it on the function as I am going to do for days rebasing in https://github.com/apache/spark/pull/28091
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609828165
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25568/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404287181
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceUtils.scala
##########
@@ -64,4 +66,10 @@ object DataSourceUtils {
private[sql] def isDataFile(fileName: String) =
!(fileName.startsWith("_") || fileName.startsWith("."))
+
+ def needRebaseDateTime(lookupFileMeta: String => String): Option[Boolean] = {
+ // If there is no version, we return None and let the caller side to decide.
+ // Files written by Spark 3.0 and later follow the new calendar and don't need to rebase.
+ Option(lookupFileMeta(SPARK_VERSION_METADATA_KEY)).map(_ < "3.0")
Review comment:
Thank you so much, @cloud-fan . This is much better solution.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-610218155
> For example, if Spark 3.0 writes Parquet files and Spark 2.4 may read wrong data, we can ask them to upgrade to Spark 3.0. Forward compatibility is not officially supported.
but before you removed the SQL config `spark.sql.legacy.parquet.rebaseDateTimeInWrite.enabled`, it was possible. After this PR, we force users to upgrade all theirs clusters to Spark 3.0, so, partial migrations won't be possible.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-610228516
> It's not possible to do the opposite though, as forward compatibility is not guaranteed.
@cloud-fan I would propose to write value of `spark.sql.legacy.parquet.rebaseDateTimeInWrite.enabled` to Parquet/Avro meta. So, Spark 3.0 could check the flag in read and decide either do rebasing or skip it. If the flag does not present in meta info (saved by Spark 2.4), just do rebasing.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404798982
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2522,19 +2522,6 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
- val LEGACY_PARQUET_REBASE_DATETIME_IN_WRITE =
- buildConf("spark.sql.legacy.parquet.rebaseDateTimeInWrite.enabled")
Review comment:
Thanks for investigation. SGTM.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file metadata
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
URL: https://github.com/apache/spark/pull/28137#issuecomment-614197411
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121326/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file metadata
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
URL: https://github.com/apache/spark/pull/28137#issuecomment-614196429
**[Test build #121326 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121326/testReport)** for PR 28137 at commit [`2e908c5`](https://github.com/apache/spark/commit/2e908c51be999f54d97a11ce3261518d1a2f75c3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class AvroDeserializer(rootAvroType: Schema, rootCatalystType: DataType, rebaseDateTime: Boolean) `
* `class AvroSerializer(`
* `class ParquetReadSupport(`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-614049257
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26009/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609952212
Could you update the PR description? It should be `>= 3.0` instead of `> 3.0`.
> Do not rebase datetime values when reading parquet/avro, if we know the file written version and it's > "3.0".
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404673165
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2522,19 +2522,6 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
- val LEGACY_PARQUET_REBASE_DATETIME_IN_WRITE =
- buildConf("spark.sql.legacy.parquet.rebaseDateTimeInWrite.enabled")
Review comment:
One thing I am concerned, though, I suspect it won't be compatible with other external projects if we don't rebase? We have `spark.sql.parquet.writeLegacyFormat` in particular for the compatibility with Apache Hive and Apache Impala for example.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609981150
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/120876/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404592294
##########
File path: sql/core/benchmarks/DateTimeRebaseBenchmark-jdk11-results.txt
##########
@@ -26,17 +15,6 @@ before 1582, vec off, rebase on 13836 13872
before 1582, vec on, rebase off 3664 3672 7 27.3 36.6 3.5X
before 1582, vec on, rebase on 6049 6078 30 16.5 60.5 2.1X
-OpenJDK 64-Bit Server VM 11.0.6+10-post-Ubuntu-1ubuntu118.04.1 on Linux 4.15.0-1063-aws
-Intel(R) Xeon(R) CPU E5-2670 v2 @ 2.50GHz
-Save timestamps to parquet: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
Review comment:
can't `toJavaTimestamp` benefit from your optimization?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-609828157
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file metadata
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
URL: https://github.com/apache/spark/pull/28137#issuecomment-614197404
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404590158
##########
File path: sql/core/benchmarks/DateTimeRebaseBenchmark-jdk11-results.txt
##########
@@ -26,17 +15,6 @@ before 1582, vec off, rebase on 13836 13872
before 1582, vec on, rebase off 3664 3672 7 27.3 36.6 3.5X
before 1582, vec on, rebase on 6049 6078 30 16.5 60.5 2.1X
-OpenJDK 64-Bit Server VM 11.0.6+10-post-Ubuntu-1ubuntu118.04.1 on Linux 4.15.0-1063-aws
-Intel(R) Xeon(R) CPU E5-2670 v2 @ 2.50GHz
-Save timestamps to parquet: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
Review comment:
timestamps rebasing in ORC won't get any benefits from the optimization because it uses to/fromJavaTimestamp
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file
written Spark version
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#discussion_r404760772
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2522,19 +2522,6 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
- val LEGACY_PARQUET_REBASE_DATETIME_IN_WRITE =
- buildConf("spark.sql.legacy.parquet.rebaseDateTimeInWrite.enabled")
Review comment:
People would assume the storage systems to use the ISO standard calendar (proleptic Gregorian calendar) by default. The Parquet spec doesn't define the calendar explicitly but it refers to java 8 time API, which kind of implictly requires proleptic Gregorian calendar.
Spark has this issue because we use java 7 time API before 3.0.
Hive switched to the proleptic Gregorian calendar in 3.1, and AFAIK caused some troubles in other systems, which kind of proves that other systems are using the proleptic Gregorian calendar already.
Impala has added a config `convert_legacy_hive_parquet_utc_timestamps` to work around it: https://issues.apache.org/jira/browse/IMPALA-3933
Presto also hit issues when reading Hive tables with timestamp: https://github.com/prestodb/presto/issues/12180
That why I think it's better to write datetime values without rebasing, as that's the correct data.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137:
[SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file metadata
URL: https://github.com/apache/spark/pull/28137#issuecomment-614197404
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #28137: [SPARK-31361][SQL]
Rebase datetime in parquet/avro according to file written Spark version
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #28137: [SPARK-31361][SQL] Rebase datetime in parquet/avro according to file written Spark version
URL: https://github.com/apache/spark/pull/28137#issuecomment-610242318
Please see https://github.com/apache/spark/pull/28137#discussion_r404582477
My point is: it's more of an artificial use case and I'm not sure it worth introducing a new metadata key for it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org