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.

Sign up for GitHub

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

Jump to bottom

Support spark.sql.parquet.int96RebaseModeInWrite=LEGACY [databricks] #9658

Merged
merged 61 commits into from
Nov 14, 2023
Merged

Support spark.sql.parquet.int96RebaseModeInWrite=LEGACY [databricks] #9658

merged 61 commits into from
Nov 14, 2023

Conversation

ttnghia
Copy link
Collaborator

@ttnghia ttnghia commented Nov 7, 2023 •
edited
Loading

This adds support for LEGACY mode in spark.sql.parquet.int96RebaseModeInWrite, which allows writing files containing ancient times before 1582-10-15 with rebasing from Proleptic Gregorian calendar times to Julian calendar times.

Closes:

ttnghia and others added 30 commits November 2, 2023 10:52
@ttnghia
Add check for nested types
e368aa6
@ttnghia
Recursively check for rebasing
7da416b
@ttnghia
Extract common code
df8f861
@ttnghia
Allow nested type in rebase check
95d19ee
@ttnghia
Enable nested timestamp in roundtrip test
b426610
@ttnghia
Fix another test
7343b17 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Merge branch 'check_rebase_nested' into rebase_datatime
0d48f57
@ttnghia
Enable LEGACY rebase in read
024e6c9
@ttnghia
Remove comment
9a39628
@ttnghia
Change function/class signatures
e686bb0
@ttnghia
Merge branch 'branch-23.12' into rebase_datatime
b49963e 
# Conflicts:
#	sql-plugin/src/main/scala/com/nvidia/spark/RebaseHelper.scala
#	sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuParquetScan.scala
@ttnghia
Complete modification
2c232f8
@ttnghia
Misc
ac0f3e4 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Add explicit type
c773794 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Rename file and add some stuff in DateTimeRebaseHelpers.scala
29df7cd
@ttnghia
Move file and rename class
1b5112d
@ttnghia
Adopt new enum type
63342a9 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Add name for the enum classes
6b2d795
@ttnghia
Change exception messages
37aa40b
@ttnghia
Merge branch 'branch-23.12' into refactor_parquet_scan
d4cdc1b
@ttnghia
Does not yet support legacy rebase in read
03f681e 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Change legacy to corrected mode
14f230f 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Extract common code
1b464ec 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Rename functions
0d26d97 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Reformat
c2504fd 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Make classes serializable
edb6c81 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Revert "Support rebase checking for nested dates and timestamps (NVID…
ea86e8f 
…IA#9617)"

This reverts commit 401d0d8.

Signed-off-by: Nghia Truong <[email protected]>

# Conflicts:
#	sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuParquetScan.scala
@ttnghia
Merge branch 'refactor_parquet_scan' into rebase_datatime
b14463f 
# Conflicts:
#	sql-plugin/src/main/scala/com/nvidia/spark/RebaseHelper.scala
#	sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuParquetScan.scala
@ttnghia
Implement date time rebase
adc8ae2
@ttnghia
Optimize rebase op
791573c
@ttnghia ttnghia added the task Work required that improves the product but is not user facing label Nov 7, 2023
@ttnghia ttnghia self-assigned this Nov 7, 2023
@ttnghia
Revert unrelated changes
bda59ef 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Copy link
Collaborator Author

ttnghia commented Nov 7, 2023

build

@ttnghia
Copy link
Collaborator Author

ttnghia commented Nov 7, 2023

build

@ttnghia ttnghia mentioned this pull request Nov 8, 2023
Merged
@ttnghia
Copy link
Collaborator Author

ttnghia commented Nov 8, 2023

build

revans2
revans2 previously approved these changes Nov 8, 2023
View reviewed changes
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the tests are updated and the code says that we now support rebase for in96 writes. But I don't see anywhere that the code was updated for it. I am assuming that the existing code just covered it and we are now enabling it after testing.

integration_tests/src/main/python/parquet_write_test.py Show resolved Hide resolved
@ttnghia
Change date/time generators
27967e4 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Copy link
Collaborator Author

ttnghia commented Nov 8, 2023

build

@ttnghia
Copy link
Collaborator Author

ttnghia commented Nov 8, 2023

I see the tests are updated and the code says that we now support rebase for in96 writes. But I don't see anywhere that the code was updated for it. I am assuming that the existing code just covered it and we are now enabling it after testing.

Right, the existing code already handles the rebase computation. Now we just enable the corresponding code path and update tests.

@ttnghia
Fix test
c67a53a 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Copy link
Collaborator Author

ttnghia commented Nov 8, 2023

build

@ttnghia ttnghia requested a review from revans2 November 9, 2023 14:33
revans2
revans2 reviewed Nov 9, 2023
View reviewed changes
integration_tests/src/main/python/hive_write_test.py
@@ -85,8 +85,6 @@ def do_write(spark, table_name):
@pytest.mark.skipif(not is_hive_available(), reason="Hive is missing")
@pytest.mark.parametrize("gens", [_basic_gens], ids=idfn)
@pytest.mark.parametrize("storage_with_confs", [
("PARQUET", {"spark.sql.legacy.parquet.datetimeRebaseModeInWrite": "LEGACY",
"spark.sql.legacy.parquet.int96RebaseModeInWrite": "LEGACY"}),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why drop these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fallback test. We now have full support for LEGACY in write thus we don't fallback anymore.

Copy link
Collaborator Author

@ttnghia ttnghia Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deleted test is similar: https://github.com/NVIDIA/spark-rapids/pull/9658/files#diff-33b5f03f5e0d5aa2b19d7666f2db31cb30b3c8d9b67271017ce960cc6ff40c78L789

@ttnghia ttnghia linked an issue Nov 14, 2023 that may be closed by this pull request
[FEA] Support spark.sql.parquet.int96RebaseModeInWrite= LEGACY #9037
Closed
@revans2 revans2 merged commit 4fdd7bd into NVIDIA:branch-23.12 Nov 14, 2023
37 checks passed
@ttnghia ttnghia deleted the int96_rebase_write branch November 14, 2023 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@revans2 revans2 revans2 approved these changes

Assignees

@ttnghia ttnghia

Labels
task Work required that improves the product but is not user facing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Support spark.sql.parquet.int96RebaseModeInWrite= LEGACY
2 participants
@ttnghia @revans2