Skip to content

HIVE-28625: Upgrade Apache Parquet version to 1.14.4 - #5552

Merged
zhangbutao merged 5 commits into
apache:masterfrom
zhangbutao:HIVE-28625
Dec 2, 2024
Merged

HIVE-28625: Upgrade Apache Parquet version to 1.14.4#5552
zhangbutao merged 5 commits into
apache:masterfrom
zhangbutao:HIVE-28625

Conversation

@zhangbutao

@zhangbutao zhangbutao commented Nov 20, 2024

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Why are the changes needed?

https://github.com/apache/parquet-java/pull/3041/files fixed a bug related to data loss. We should upgrade the latest parquet version to avoid this issue.
btw, Iceberg also revert the low version 1.13.1 & upgrade to 1.14.4 to fix the issue. https://github.com/apache/iceberg/pull/11462/files

BTW, Parquet 1.14 has added the Hadoop Vectored IO feature in https://issues.apache.org/jira/browse/PARQUET-2171, so i think we can use this good feature after the upgrading. :)

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

see the file mvndep.log

How was this patch tested?

The existing tests

@okumin okumin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally, looks good to me

'current-schema'='{"type":"struct","schema-id":0,"fields":[{"id":1,"name":"i","required":false,"type":"int"},{"id":2,"name":"s","required":false,"type":"string"}]}',
'current-snapshot-id'='#SnapshotId#',
'current-snapshot-summary'='{"added-data-files":"1","added-records":"3","added-files-size":"628","changed-partition-count":"1","total-records":"3","total-files-size":"628","total-data-files":"1","total-delete-files":"0","total-position-deletes":"0","total-equality-deletes":"0","iceberg-version":"#Masked#"}',
'current-snapshot-summary'='{"added-data-files":"1","added-records":"3","added-files-size":"#Masked#","changed-partition-count":"1","total-records":"3","total-files-size":"671","total-data-files":"1","total-delete-files":"0","total-position-deletes":"0","total-equality-deletes":"0","iceberg-version":"#Masked#"}',

@okumin okumin Nov 30, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should total-files-size be masked?

@zhangbutao zhangbutao Nov 30, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catach. Masked it. 7adde55

IMO, all files size & Data size related properties should be masked.

@okumin okumin Nov 30, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agree. We may finally mask all at the level of Iceberg driver or somewhere else someday

@okumin okumin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Waiting for CI to be completed

@sonarqubecloud

Copy link
Copy Markdown

@okumin okumin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@zhangbutao
zhangbutao merged commit ad5708c into apache:master Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants