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

[SPARK-41962][MINOR][SQL] Update the order of imports in class SpecificParquetRecordReaderBase #39906

Closed
wants to merge 1 commit into from
Closed

[SPARK-41962][MINOR][SQL] Update the order of imports in class SpecificParquetRecordReaderBase #39906

wants to merge 1 commit into from

Conversation

wayneguow
Copy link
Contributor

What changes were proposed in this pull request?

Update the order of imports in class SpecificParquetRecordReaderBase.

Why are the changes needed?

Follow the code style.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Passed GA.

@github-actions github-actions bot added the SQL label Feb 6, 2023
@wayneguow wayneguow changed the title [SPARK-41962][MINOR][CORE] Update the order of imports in class SpecificParquetRecordReaderBase [SPARK-41962][MINOR][SQL] Update the order of imports in class SpecificParquetRecordReaderBase Feb 6, 2023
HyukjinKwon
HyukjinKwon reviewed Feb 6, 2023
View reviewed changes
...java/org/apache/spark/sql/execution/datasources/parquet/SpecificParquetRecordReaderBase.java
import com.google.common.annotations.VisibleForTesting;
import org.apache.parquet.VersionParser;
import org.apache.parquet.VersionParser.ParsedVersion;
import org.apache.parquet.column.page.PageReadStore;
Copy link
Member

Choose a reason for hiding this comment

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

Can we fix dev/checkstyle.xml to enforce this order?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, that's better, and once and for all. I'll make some changes later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After pr #35945 and discussion in pr #35970, the ImportOrder todo has been removed, the conclusion is that it's not worth changing all unordered files.
So shall we just make a little change for closing the related issued opened by uses?

@wayneguow wayneguow marked this pull request as draft February 6, 2023 14:36
@wayneguow wayneguow changed the title [SPARK-41962][MINOR][SQL] Update the order of imports in class SpecificParquetRecordReaderBase [WIP][SPARK-41962][MINOR][SQL] Update the order of imports in class SpecificParquetRecordReaderBase Feb 6, 2023
@wayneguow wayneguow marked this pull request as ready for review February 7, 2023 05:00
@wayneguow wayneguow requested a review from HyukjinKwon February 7, 2023 05:00
@wayneguow wayneguow changed the title [WIP][SPARK-41962][MINOR][SQL] Update the order of imports in class SpecificParquetRecordReaderBase [SPARK-41962][MINOR][SQL] Update the order of imports in class SpecificParquetRecordReaderBase Feb 7, 2023
@HyukjinKwon
Copy link
Member

HyukjinKwon commented Feb 7, 2023 •
edited
Loading

Merged to master, branch-3.4, branch-3.3 and branch-3.2.

HyukjinKwon pushed a commit that referenced this pull request Feb 7, 2023
@wayneguow @HyukjinKwon
[SPARK-41962][MINOR][SQL] Update the order of imports in class Specif…
0495cd8 
…icParquetRecordReaderBase

### What changes were proposed in this pull request?
Update the order of imports in class SpecificParquetRecordReaderBase.

### Why are the changes needed?
Follow the code style.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Passed GA.

Closes #39906 from wayneguow/import.

Authored-by: wayneguow <guow93@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit d6134f7)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
HyukjinKwon pushed a commit that referenced this pull request Feb 7, 2023
@wayneguow @HyukjinKwon
[SPARK-41962][MINOR][SQL] Update the order of imports in class Specif…
51ed6ba 
…icParquetRecordReaderBase

### What changes were proposed in this pull request?
Update the order of imports in class SpecificParquetRecordReaderBase.

### Why are the changes needed?
Follow the code style.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Passed GA.

Closes #39906 from wayneguow/import.

Authored-by: wayneguow <guow93@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit d6134f7)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
HyukjinKwon pushed a commit that referenced this pull request Feb 7, 2023
@wayneguow @HyukjinKwon
[SPARK-41962][MINOR][SQL] Update the order of imports in class Specif…
c8b47ec 
…icParquetRecordReaderBase

### What changes were proposed in this pull request?
Update the order of imports in class SpecificParquetRecordReaderBase.

### Why are the changes needed?
Follow the code style.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Passed GA.

Closes #39906 from wayneguow/import.

Authored-by: wayneguow <guow93@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit d6134f7)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
@cloud-fan cloud-fan reopened this Feb 7, 2023
@cloud-fan cloud-fan closed this Feb 7, 2023
@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Feb 8, 2023 •
edited
Loading

Hi, @wayneguow and @HyukjinKwon This broke branch-3.2.

(imports) UnusedImports: Unused import - com.google.common.annotations.VisibleForTesting.
(imports) UnusedImports: Unused import - org.apache.parquet.VersionParser.
(imports) UnusedImports: Unused import - org.apache.parquet.VersionParser.ParsedVersion.
(imports) UnusedImports: Unused import - org.apache.parquet.column.page.PageReadStore.
Error: Process completed with exit code 1.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Feb 8, 2023 •
edited
Loading

I reverted the commit from branch-3.2 because the import statement is unused in the original commit (on branch-3.2)

Screenshot 2023-02-08 at 1 34 13 PM

@wayneguow
Copy link
Contributor Author

I reverted the commit from branch-3.2 because the import statement is unused.

Screenshot 2023-02-08 at 1 34 13 PM

@dongjoon-hyun Thank you for resolving it.

@dongjoon-hyun
Copy link
Member

You can make a new backporting PR to branch-3.2 if needed, @wayneguow .

@dongjoon-hyun
Copy link
Member

Also, cc @parthchandra for branch-3.2

@wayneguow
Copy link
Contributor Author

You can make a new backporting PR to branch-3.2 if needed, @wayneguow .

image

It seems that it is ok without those unordered imports in branch-3.2, so just reverting this for branch-3.2 is enough.

@dongjoon-hyun
Copy link
Member

Thank you for the confirmation, @wayneguow .

snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
@wayneguow @dongjoon-hyun
[SPARK-41962][MINOR][SQL] Update the order of imports in class Specif…
0fd9c16 
…icParquetRecordReaderBase

### What changes were proposed in this pull request?
Update the order of imports in class SpecificParquetRecordReaderBase.

### Why are the changes needed?
Follow the code style.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Passed GA.

Closes apache#39906 from wayneguow/import.

Authored-by: wayneguow <guow93@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit d6134f7)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HyukjinKwon HyukjinKwon HyukjinKwon approved these changes

Assignees
No one assigned
Labels
SQL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wayneguow @HyukjinKwon @dongjoon-hyun @cloud-fan