Skip to content

Conversation

@panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Aug 16, 2022

What changes were proposed in this pull request?

This PR aim to remove redundant ColumnIOUtil.

Why are the changes needed?

Complete todo.
image

reason: from parquet version 1.12.3, methods below are public

1.ColumnIO.getDefinitionLevel: https://github.com/apache/parquet-mr/blob/apache-parquet-1.12.3/parquet-column/src/main/java/org/apache/parquet/io/ColumnIO.java#L84-L86
2.ColumnIO.getRepetitionLevel: https://github.com/apache/parquet-mr/blob/apache-parquet-1.12.3/parquet-column/src/main/java/org/apache/parquet/io/ColumnIO.java#L77-L79
3.ColumnIO.getFieldPath: https://github.com/apache/parquet-mr/blob/apache-parquet-1.12.3/parquet-column/src/main/java/org/apache/parquet/io/ColumnIO.java#L50-L52

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

@github-actions github-actions bot added the SQL label Aug 16, 2022
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@srowen
Copy link
Member

srowen commented Aug 16, 2022

Merged to master

@srowen srowen closed this in 3e8975b Aug 16, 2022
/**
* This is a workaround since methods below are not public in {@link ColumnIO}.
*
* TODO(SPARK-36511): we should remove this once PARQUET-2050 and PARQUET-2083 are released with
Copy link
Member

Choose a reason for hiding this comment

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

This is not a minor. This should be removed with JIRA issue, SPARK-36511 .

Copy link
Member

Choose a reason for hiding this comment

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

To @panbingkun , please respect the code when you remove.
Apache Spark community uses IDed TODO like this for trace-ability and we had better respect them.

@dongjoon-hyun
Copy link
Member

Also, cc @sunchao since he is the issue reporter of SPARK-36511 .

@srowen
Copy link
Member

srowen commented Aug 16, 2022

OK fair enough can you file a JIRA? I can connect the dots

@dongjoon-hyun
Copy link
Member

@srowen , the JIRA, SPARK-36511, was already filed and hard-coded in the removed part. Here, @panbingkun ignored it mistakenly.

@srowen
Copy link
Member

srowen commented Aug 16, 2022

Ah ok I'll connect it

@srowen srowen changed the title [MINOR][SQL] Remove ColumnIOUtil [SPARK-36511][MINOR][SQL] Remove ColumnIOUtil Aug 16, 2022
@dongjoon-hyun
Copy link
Member

Thank you, @srowen .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants