Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle TIMESTAMP_MILLIS for rebase check #8687

Merged
merged 6 commits into from
Jul 12, 2023

Conversation

thirtiseven
Copy link
Collaborator

@thirtiseven thirtiseven commented Jul 11, 2023

Fixes #1130

We aren't properly handling spark.sql.legacy.parquet.datetimeRebaseModeInWrite=TIMESTAMP_MILLIS in isDateTimeRebaseNeeded when we run with spark.sql.parquet.outputTimestampType=EXCEPTION. It should throw a SparkUpgradeException in this case.

This PR updated the ColumnarOutputWriter to scan the table before transform. So in the issue's case, rebase check will happen before type casting. Now the plugin will handle TIMESTAMP_MILLIS normally and throw SparkUpgradeException in the issue's case.

@thirtiseven thirtiseven self-assigned this Jul 11, 2023
@sameerz sameerz added the bug Something isn't working label Jul 11, 2023
@thirtiseven
Copy link
Collaborator Author

build

Signed-off-by: Haoyang Li <[email protected]>
@thirtiseven
Copy link
Collaborator Author

thirtiseven commented Jul 11, 2023

I realized that I misunderstood the meaning of note in this issue. Now, I split scanAndWrite into scan and write and check if rebase is needed before type casting.

@thirtiseven
Copy link
Collaborator Author

build

@thirtiseven thirtiseven changed the title Handle TIMESTAMP_MILLIS in isDateTimeRebaseNeeded Handle TIMESTAMP_MILLIS for rebase check Jul 11, 2023
@revans2
Copy link
Collaborator

revans2 commented Jul 12, 2023

build

@abellina
Copy link
Collaborator

abellina commented Jul 12, 2023

I am merging this because I'd like to pull the changed into another PR I have up that is also ready to go in fairly soon after this. Thanks @thirtiseven !

@abellina abellina merged commit 04d9080 into NVIDIA:branch-23.08 Jul 12, 2023
@thirtiseven thirtiseven deleted the millis_rebase branch August 18, 2023 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] TIMESTAMP_MILLIS not handled in isDateTimeRebaseNeeded
4 participants