You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ganeshashree (via GitHub)" <gi...@apache.org> on 2025/03/08 14:12:58 UTC
[PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
ganeshashree opened a new pull request, #50215:
URL: https://github.com/apache/spark/pull/50215
<!--
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'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### 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.
-->
Changes done to set INT64 as the default timestamp type for Parquet files.
### 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.
-->
The INT96 timestamp type has been deprecated as part of [PARQUET-323](https://issues.apache.org/jira/browse/PARQUET-323). However, Apache Spark still uses INT96 as the default outputTimestampType for Parquet files ([code link](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L1157)). This could create incompatibilities when Parquet data written by Spark is read by readers that do not support the INT96 type. We should consider changing the default timestamp type to INT64 in future versions.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as new features, bug fixes, or other behavior changes. Documentation-only updates are not considered user-facing changes.
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 possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
The default timestamp type for Parquet files will be changed to INT64. Older versions of applications that support INT96 should enable the INT96 type by setting `spark.sql.parquet.int96AsTimestamp` to `true` and `spark.sql.parquet.outputTimestampType` to `INT96`.
### 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.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
Updated the unit tests in ParquetSchemaSuite and SQLConfSuite to reflect INT64 as the default timestamp type for Parquet files.
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-3041293230
More links and information
- The JIRA in question: https://issues.apache.org/jira/browse/SPARK-51359
Also, @alkis says on the parquet mailing list: https://lists.apache.org/thread/ybthxfznokkvmm66r412f6m00ywmvvh8
> I also checked internally with the Spark OSS team and the plan for having
> INT64 timestamps in Spark by default is to make the change when Delta v5
> and Iceberg v4 are proposed. This is expected to happen around the first
> half of 2026.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-3476996179
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #50215: [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files
URL: https://github.com/apache/spark/pull/50215
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-2987528379
Thank you for pushing this forward @ganeshashree and thank you for considering the request @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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "ganeshashree (via GitHub)" <gi...@apache.org>.
ganeshashree commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-2721580059
> int96 support in parquet apache/iceberg#1138
@HyukjinKwon Yes, this will be a breaking change for applications that depend on INT96 and use future versions of Spark. We are changing the default timestamp type for Parquet files in a future version of Spark.
Applications using future versions of Spark to read or write Parquet files will need to configure Spark to handle INT96, as it will no longer be supported by default. We will need to document this change and highlight it in our release notes.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-2715147840
my concern is that this is a breaking change ... We will at least have to update the migration guide
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-2995045980
Can we check the ecosystem again? How is parquet int64 timestamp supported by Hive, Presto, DuckDB, Pandas, Iceberg, Delta Lake, etc.?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-3112212892
Stumbled upon this when trying to figure out why `INT96` is still the default today ...
> Can we check the ecosystem again? How is parquet int64 timestamp supported by Hive, Presto, DuckDB, Pandas, Iceberg, Delta Lake, etc.?
- Presto/Trino: fixed via https://github.com/trinodb/trino/issues/5483
- Hive: Supported via https://issues.apache.org/jira/browse/HIVE-21215 since 4.0.0
- DuckDB: I think supported since very early on. https://github.com/duckdb/duckdb/issues/9852 confirms that.
- Pandas: Most definitely (via `pyarrow`)
- Iceberg: Yes!
- Delta Lake: Yes (see https://github.com/delta-io/delta/blob/master/PROTOCOL.md#delta-data-type-to-parquet-type-mappings)
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-3005065634
DataFusion and supported systems work well with Int64 timestamps
@cloud-fan what would you consider "supported" to mean for those systems? You mean can read a parquet file with Int64 timestamps correctly?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-3005121697
As backstory for why this issue has received some more attention recently, there is move by DataBricks to update the statistics in other parquet writers to handle INT96 timestamps --
- see https://github.com/apache/arrow-rs/pull/7687
- https://github.com/apache/parquet-java/issues/3242
- parquet mailing list discussion on this topic: https://lists.apache.org/thread/6fm50b3pmh6mz659jb5wx5vzmvwccz1n
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-51359][CORE][SQL] Set INT64 as the default timestamp type for Parquet files [spark]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #50215:
URL: https://github.com/apache/spark/pull/50215#issuecomment-2991854141
> my concern is that this is a breaking change ... We will at least have to update the migration guide
Note that Spark already did this once: https://github.com/apache/spark/pull/24425
And then reverted it due to concern about other systems (Hive, Preseto) not having good support for non-INT96 timestamps yet: https://github.com/apache/spark/pull/28450
It probably should have been included in Spark 3.x, but has continued to slip through the cracks.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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