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

Replace ParquetFileReader.readFooter with open() and getFooter [databricks] #4976

Merged
merged 5 commits into from
Mar 23, 2022
Merged

Replace ParquetFileReader.readFooter with open() and getFooter [databricks] #4976

merged 5 commits into from
Mar 23, 2022

Conversation

razajafri
Copy link
Collaborator

@razajafri razajafri commented Mar 17, 2022 •
edited
Loading

This PR replaces the use of deprecated method readFooter with open() and getFooter

DB was failing with the following exception unless I import the parquet-common dependency for tests submodule

Class org.apache.parquet.io.InputFile not found - continuing with a stub.
[ERROR]         val file = ParquetFileReader.open(inputFile)

fixes #2082

Signed-off-by: Raza Jafri rjafri@nvidia.com

@razajafri
replace readFooter with open() and getFootter
56232e3 
Signed-off-by: Raza Jafri <rjafri@nvidia.com>
@razajafri razajafri changed the title Replace readFooter with open() and getFootter Replace readFooter with open() and getFootter [databricks] Mar 17, 2022
@razajafri
Copy link
Collaborator Author

build

@razajafri
add dependency for db failure
2a31775 
Signed-off-by: Raza Jafri <rjafri@nvidia.com>
@razajafri
Copy link
Collaborator Author

build

1 similar comment
@razajafri
Copy link
Collaborator Author

build

@razajafri
Copy link
Collaborator Author

There is something weird going on with CI as this test passes locally

@razajafri
Copy link
Collaborator Author

build

@razajafri razajafri changed the title Replace readFooter with open() and getFootter [databricks] Replace ParquetFileReader.readFooter with open() and getFootter [databricks] Mar 18, 2022
@razajafri
Copy link
Collaborator Author

build

@razajafri razajafri changed the title Replace ParquetFileReader.readFooter with open() and getFootter [databricks] Replace ParquetFileReader.readFooter with open() and getFootter Mar 18, 2022
@razajafri
Copy link
Collaborator Author

Removing the databricks and kicking CI off again to see if anything changes 🤞

@razajafri
Copy link
Collaborator Author

build

@razajafri
close the file
2496538 
Signed-off-by: Raza Jafri <rjafri@nvidia.com>
@razajafri
Copy link
Collaborator Author

build

@sameerz sameerz added this to the Mar 21 - Apr 1 milestone Mar 18, 2022
@razajafri razajafri changed the title Replace ParquetFileReader.readFooter with open() and getFootter Replace ParquetFileReader.readFooter with open() and getFootter [databricks] Mar 19, 2022
@razajafri
Copy link
Collaborator Author

Adding stains databricks back

@razajafri
Copy link
Collaborator Author

build

1 similar comment
@razajafri
Copy link
Collaborator Author

build

@razajafri
Merge remote-tracking branch 'origin/branch-22.04' into parquet_readf…
f2d8f6c 
…ooter

Signed-off-by: Raza Jafri <rjafri@nvidia.com>
@razajafri
Copy link
Collaborator Author

build

jlowe
jlowe previously approved these changes Mar 22, 2022
View reviewed changes
@jlowe jlowe changed the title Replace ParquetFileReader.readFooter with open() and getFootter [databricks] Replace ParquetFileReader.readFooter with open() and getFooter [databricks] Mar 22, 2022
gerashegalov
gerashegalov reviewed Mar 22, 2022
View reviewed changes
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuParquetScanBase.scala Outdated
Comment on lines 345 to 350
val parquetFile = ParquetFileReader.open(inputFile)
val footer = try {
parquetFile.getFooter
} finally {
parquetFile.close()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Suggested change
val parquetFile = ParquetFileReader.open(inputFile)
val footer = try {
parquetFile.getFooter
} finally {
parquetFile.close()
}
val footer = withResource(ParquetFileReader.open(inputFile))(_.getFooter())

tests/pom.xml
@@ -224,6 +224,12 @@
<version>${spark.version}</version>
<scope>provided</scope>
</dependency>
<dependency>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you comment in the PR description/code as to why new dependency?

@razajafri
addressed review comments
8b7b380 
Signed-off-by: Raza Jafri <rjafri@nvidia.com>
@razajafri
Copy link
Collaborator Author

build

@razajafri razajafri requested a review from gerashegalov March 23, 2022 16:31
@razajafri
Copy link
Collaborator Author

@gerashegalov have I addressed all your concerns?

gerashegalov
gerashegalov approved these changes Mar 23, 2022
View reviewed changes
Copy link
Collaborator

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM

@razajafri razajafri merged commit 1297916 into NVIDIA:branch-22.04 Mar 23, 2022
@razajafri razajafri deleted the SR-2082-replace-readFooter branch March 23, 2022 18:34
@abellina abellina mentioned this pull request Mar 25, 2022
Closed
abellina added a commit that referenced this pull request Mar 25, 2022
@abellina
Revert "Replace ParquetFileReader.readFooter with open() and getFoote…
8af9b2d 
…r [databricks] (#4976)"

This reverts commit 1297916.
@abellina abellina mentioned this pull request Mar 25, 2022
Merged
abellina added a commit that referenced this pull request Mar 25, 2022
@abellina
Revert "Replace ParquetFileReader.readFooter with open() and getFoote…
4eeadd7 
…r [databricks] (#4976)"

This reverts commit 1297916.

Signed-off-by: Alessandro Bellina <abellina@nvidia.com>
abellina added a commit that referenced this pull request Mar 25, 2022
@abellina
Revert "Replace ParquetFileReader.readFooter with open() and getFoote…
75c4e87 
…r [databricks] (#4976)" (#5051)

This reverts commit 1297916.

Signed-off-by: Alessandro Bellina <abellina@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gerashegalov gerashegalov gerashegalov approved these changes

@jlowe jlowe jlowe approved these changes

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

@tgravescs tgravescs Awaiting requested review from tgravescs tgravescs is a code owner

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

@NvTimLiu NvTimLiu Awaiting requested review from NvTimLiu NvTimLiu is a code owner

Assignees
No one assigned
Labels
audit_3.2.0
Projects
None yet
Milestone
Mar 21 - Apr 1
Development

Successfully merging this pull request may close these issues.

[FEA] Replace ParquetFileReader.readFooter with `ParquetFileReader.open and getFooter
4 participants
@razajafri @jlowe @gerashegalov @sameerz