PARQUET-2343: Fixes NPE when rewriting file with multiple rowgroups #1136

ConeyLiu · 2023-08-31T10:09:16Z

Currently, the ParquetRewiter creates the ColumnReadStoreImpl crStore and reuses it for all the blocks rewriting. This should be incorrect and we should create the crStore for each block that needs to be rewritten. Otherwise, we will fail as the following:

java.lang.NullPointerException
at org.apache.parquet.column.impl.ColumnReaderBase.readPage(ColumnReaderBase.java:620)
at org.apache.parquet.column.impl.ColumnReaderBase.checkRead(ColumnReaderBase.java:594)
at org.apache.parquet.column.impl.ColumnReaderBase.consume(ColumnReaderBase.java:735)
at org.apache.parquet.column.impl.ColumnReaderImpl.consume(ColumnReaderImpl.java:30)
at org.apache.parquet.column.impl.ColumnReaderImpl.<init>(ColumnReaderImpl.java:47)
at org.apache.parquet.column.impl.ColumnReadStoreImpl.getColumnReader(ColumnReadStoreImpl.java:82)
at org.apache.parquet.hadoop.rewrite.ParquetRewriter.processBlocksFromReader(ParquetRewriter.java:316)
at org.apache.parquet.hadoop.rewrite.ParquetRewriter.processBlocks(ParquetRewriter.java:250)

Jira

My PR addresses the following Parquet Jira issues and references them in the PR title. For example, "PARQUET-1234: My Parquet PR"
- https://issues.apache.org/jira/browse/PARQUET-XXX
- In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Tests

My PR adds the following unit tests OR does not need testing for this extremely good reason:

Adds testRewriteFileWithMultipleBlocks in ParquetRewriterTest.

Commits

My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters (not including Jira issue reference)
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

Documentation

In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain Javadoc that explain what it does

ConeyLiu · 2023-08-31T10:12:18Z

Hi @wgtmac, could you help to review this? Thanks a lot.

wgtmac

Thanks for the fix! @ConeyLiu

…pache#1136)

ConeyLiu · 2023-09-04T08:48:16Z

Thanks @wgtmac for merging this.

changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release.

* Only recompress the parquet rows when the compression mode changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release. * Fix ETLTimeTest to handle src made up of ZIP files. * Fix NamedFlagETLTest to use a single configservice so as to not conflict test with each other. * Fix ETLPostProcessorTest to use a single configservice so as to not conflict test with each other. * Run workflows on all pull requests

Add support for using parquet files Adds support in the plainpbstorage plugin for storing pb messages in parquet files. Parquet fix recompress (#20) * Only recompress the parquet rows when the compression mode changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release. * Fix ETLTimeTest to handle src made up of ZIP files. * Fix NamedFlagETLTest to use a single configservice so as to not conflict test with each other. * Fix ETLPostProcessorTest to use a single configservice so as to not conflict test with each other. * Run workflows on all pull requests Parquet more test (#19) * Create more test cases Adds the parquet backend to more tests Creates more variations in test setup Removes repeated tests removes need for single forks tests tag Removes need for slow tests tag * Test different compression options in the timetest Parquet multi file (#18) * Tidy up pb/parquet packages * Combine parquet and pb compression mode Adds a new wrapper class for CompressionCodec from parquet and CompressionMode in pb * Improve efficiency of using Parquet library * Make the parquet file stream able to work with multiple files.

Add support for using parquet files Adds support in the plainpbstorage plugin for storing pb messages in parquet files. Parquet fix recompress (#20) * Only recompress the parquet rows when the compression mode changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release. * Fix ETLTimeTest to handle src made up of ZIP files. * Fix NamedFlagETLTest to use a single configservice so as to not conflict test with each other. * Fix ETLPostProcessorTest to use a single configservice so as to not conflict test with each other. * Run workflows on all pull requests Parquet more test (#19) * Create more test cases Adds the parquet backend to more tests Creates more variations in test setup Removes repeated tests removes need for single forks tests tag Removes need for slow tests tag * Test different compression options in the timetest Parquet multi file (#18) * Tidy up pb/parquet packages * Combine parquet and pb compression mode Adds a new wrapper class for CompressionCodec from parquet and CompressionMode in pb * Improve efficiency of using Parquet library * Make the parquet file stream able to work with multiple files. Add support for parquet storage. Add support for using parquet files Adds support in the plainpbstorage plugin for storing pb messages in parquet files. Parquet fix recompress (#20) * Only recompress the parquet rows when the compression mode changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release. * Fix ETLTimeTest to handle src made up of ZIP files. * Fix NamedFlagETLTest to use a single configservice so as to not conflict test with each other. * Fix ETLPostProcessorTest to use a single configservice so as to not conflict test with each other. * Run workflows on all pull requests Parquet more test (#19) * Create more test cases Adds the parquet backend to more tests Creates more variations in test setup Removes repeated tests removes need for single forks tests tag Removes need for slow tests tag * Test different compression options in the timetest Parquet multi file (#18) * Tidy up pb/parquet packages * Combine parquet and pb compression mode Adds a new wrapper class for CompressionCodec from parquet and CompressionMode in pb * Improve efficiency of using Parquet library * Make the parquet file stream able to work with multiple files. Replace Timestamp with Instant Also reduces length of ETL Tests by decreasing amount of data created. Upgrade junit to junit 5.

Use constants for mgmt and other urls Add parquet file extension to integration tests. Add support for parquet storage. Add support for using parquet files Adds support in the plainpbstorage plugin for storing pb messages in parquet files. Parquet fix recompress (#20) * Only recompress the parquet rows when the compression mode changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release. * Fix ETLTimeTest to handle src made up of ZIP files. * Fix NamedFlagETLTest to use a single configservice so as to not conflict test with each other. * Fix ETLPostProcessorTest to use a single configservice so as to not conflict test with each other. * Run workflows on all pull requests Parquet more test (#19) * Create more test cases Adds the parquet backend to more tests Creates more variations in test setup Removes repeated tests removes need for single forks tests tag Removes need for slow tests tag * Test different compression options in the timetest Parquet multi file (#18) * Tidy up pb/parquet packages * Combine parquet and pb compression mode Adds a new wrapper class for CompressionCodec from parquet and CompressionMode in pb * Improve efficiency of using Parquet library * Make the parquet file stream able to work with multiple files. Add support for parquet storage. Add support for using parquet files Adds support in the plainpbstorage plugin for storing pb messages in parquet files. Parquet fix recompress (#20) * Only recompress the parquet rows when the compression mode changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release. * Fix ETLTimeTest to handle src made up of ZIP files. * Fix NamedFlagETLTest to use a single configservice so as to not conflict test with each other. * Fix ETLPostProcessorTest to use a single configservice so as to not conflict test with each other. * Run workflows on all pull requests Parquet more test (#19) * Create more test cases Adds the parquet backend to more tests Creates more variations in test setup Removes repeated tests removes need for single forks tests tag Removes need for slow tests tag * Test different compression options in the timetest Parquet multi file (#18) * Tidy up pb/parquet packages * Combine parquet and pb compression mode Adds a new wrapper class for CompressionCodec from parquet and CompressionMode in pb * Improve efficiency of using Parquet library * Make the parquet file stream able to work with multiple files. Replace Timestamp with Instant Also reduces length of ETL Tests by decreasing amount of data created. Upgrade junit to junit 5.

Adds support in the plainstorage plugin for storing pb messages in parquet files. * Only recompress the parquet rows when the compression mode changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release. * Fix ETLTimeTest to handle src made up of ZIP files. * Fix NamedFlagETLTest to use a single configservice so as to not conflict test with each other. * Fix ETLPostProcessorTest to use a single configservice so as to not conflict test with each other. removes need for single forks tests tag Removes need for slow tests tag * Combine parquet and pb compression mode Adds a new wrapper class for CompressionCodec from parquet and CompressionMode in pb

ConeyLiu added 2 commits August 31, 2023 17:30

Fixes rewrite file with multiple blocks

084a6f0

update

be30444

wgtmac approved these changes Sep 4, 2023

View reviewed changes

wgtmac merged commit 9b5a962 into apache:master Sep 4, 2023

wgtmac pushed a commit to wgtmac/parquet-mr that referenced this pull request Sep 4, 2023

PARQUET-2343: Fixes NPE when rewriting file with multiple rowgroups (a…

2cdbd2c

…pache#1136)

ConeyLiu deleted the fixes-rewrite branch September 4, 2023 08:48

jacomago added a commit to jacomago/epicsarchiverap that referenced this pull request Nov 6, 2023

Only recompress the parquet rows when the compression mode

dea3a2a

changes. This is unnecessary when PR apache/parquet-java#1136 is in the parquet release.

jacomago mentioned this pull request Jan 11, 2024

Add support for parquet storage. jacomago/epicsarchiverap#23

Closed

jacomago mentioned this pull request Feb 1, 2024

Add support for using parquet files jacomago/epicsarchiverap#25

Closed

This was referenced Feb 7, 2024

Add support for using parquet files archiver-appliance/epicsarchiverap#208

Closed

Add support for using parquet files jacomago/epicsarchiverap#26

Closed

This was referenced Feb 21, 2024

Add support for using parquet files jacomago/epicsarchiverap#28

Closed

Add support for using parquet files jacomago/epicsarchiverap#30

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PARQUET-2343: Fixes NPE when rewriting file with multiple rowgroups #1136

PARQUET-2343: Fixes NPE when rewriting file with multiple rowgroups #1136

ConeyLiu commented Aug 31, 2023 •

edited

Loading

ConeyLiu commented Aug 31, 2023

wgtmac left a comment

ConeyLiu commented Sep 4, 2023

PARQUET-2343: Fixes NPE when rewriting file with multiple rowgroups #1136

PARQUET-2343: Fixes NPE when rewriting file with multiple rowgroups #1136

Conversation

ConeyLiu commented Aug 31, 2023 • edited Loading

Jira

Tests

Commits

Documentation

ConeyLiu commented Aug 31, 2023

wgtmac left a comment

Choose a reason for hiding this comment

ConeyLiu commented Sep 4, 2023

ConeyLiu commented Aug 31, 2023 •

edited

Loading