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

MINOR: ParquetRewriter must close opened reader/stream #3002

Merged
merged 6 commits into from
Aug 31, 2024

Conversation

cetindogu
Copy link
Contributor

if reader not closed, open parquet files in cannot be deleted.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

if reader not closed, open parquet files in cannot be deleted.
@cetindogu
Copy link
Contributor Author

anything you want me to do ? i dont know why pipeline fails !!!

@wgtmac
Copy link
Member

wgtmac commented Aug 28, 2024

Error:  COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
Error:  /home/runner/work/parquet-java/parquet-java/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/rewrite/ParquetRewriter.java:[256,19] unreported exception java.io.IOException; must be caught or declared to be thrown

Calling close() may throw IOException.

@cetindogu
Copy link
Contributor Author

i got it. i will solve (tomorrow date hour) thanks.
it is my fault, i edited from github webpage editor:)

initNextReader() method has 2 usage and both of them already throws IOException 

public ParquetRewriter(RewriteOptions options) throws IOException 
public void processBlocks() throws IOException
@cetindogu
Copy link
Contributor Author

cetindogu commented Aug 28, 2024

fix: close() can throw IOException
initNextReader() method has 2 usages and both of them already throws IOException
so this must solve the problem

usage 1 -> public ParquetRewriter(RewriteOptions options) throws IOException
usage 2 -> public void processBlocks() throws IOException

image

@cetindogu cetindogu requested a review from Fokko August 28, 2024 17:47
@cetindogu
Copy link
Contributor Author

Error:  COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
Error:  /home/runner/work/parquet-java/parquet-java/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/rewrite/ParquetRewriter.java:[256,19] unreported exception java.io.IOException; must be caught or declared to be thrown

Calling close() may throw IOException.

thanks for your fast answer
i can't use intellij in windows 11 for this project. it cant be compiled. so i updated the code with text editor.

@cetindogu
Copy link
Contributor Author

cetindogu commented Aug 29, 2024

Is it indentation and white Space problem?

@cetindogu
Copy link
Contributor Author

i combined "reader.close();" command with "if" clouse (one line)

@cetindogu
Copy link
Contributor Author

fix: white space is added after if
if(reader != null) reader.close(); to
if (reader != null) reader.close();

@cetindogu
Copy link
Contributor Author

fix: mvn spotless:apply command is run.

@cetindogu
Copy link
Contributor Author

Anything else that i should do?

@Fokko @wgtmac

@wgtmac wgtmac changed the title fix: ParquetRewriter must close reader (open stream) MINOR: ParquetRewriter must close opened reader/stream Aug 31, 2024
@wgtmac wgtmac merged commit aec7bc6 into apache:master Aug 31, 2024
9 checks passed
@wgtmac
Copy link
Member

wgtmac commented Aug 31, 2024

I just merged it. Thanks @cetindogu and @Fokko!

@cetindogu
Copy link
Contributor Author

When can i see the new package in maven repo?

@wgtmac
Copy link
Member

wgtmac commented Sep 1, 2024

We will not see it until the next release, e.g. 1.15.0.

@wgtmac wgtmac added this to the 1.15.0 milestone Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants